Code review comment for lp://qastaging/~rvb/maas/maas-improve-tests

Thanks for the review.

> Line-breaking conditionals often makes it harder to grok the code at speed.
> It also raises the "where does the 'and' go" controversy in an awkward place.
> Could you rephrase this so that the "if" is a single line?

Okay, fixed.

> Tip: "too long" * 100

Nice!

> === modified file 'src/maasserver/tests/test_models.py'
> --- src/maasserver/tests/test_models.py 2012-01-23 12:43:28 +0000
> +++ src/maasserver/tests/test_models.py 2012-01-25 11:34:39 +0000
> @@ -34,6 +39,9 @@
> > self.assertEqual(len(self.node.system_id), 41)
> > self.assertTrue(self.node.system_id.startswith('node-'))
> >
> > + def test_display_status(self):
> > + self.assertEqual('New', self.node.display_status())
> > +
>
> Waitwaitwait — where does self.node come from? What are you testing and
> proving exactly when you say its display_status is 'New'? Yes, I can guess,
> but making people guess is a bad habit. Tests should be as self-evident and
> independent as possible.
[...]
> Expect n to grow. In the longer run we'll add other selection criteria, and
> the temptation will be to add more kinds of nodes into setUp, as well as tests
> to the test case. Each node will show up in some of the test outputs but not
> in others. Eventually it becomes an arbitrary soup of sample data that people
> won't want to touch, but making things worse will be slightly easier than
> cleaning things up.

Okay, I was ready to stand firm, but the performance argument "fait mouche".

> I feel strongly about this antipattern. You wouldn't believe the suffering it
> caused Soyuz and Rosetta!

;)

R.