Code review comment for lp:~stevenk/launchpad/switch-ifp-to-unittests

Hi Steve,

So, your new tests look okay, though I'd like to get a second opinion on whether they completely replace the existing doctests since I find the doctests quite torturous to read.

However, the new tests are sorely in need of comments. Firstly, each test_ method should have a comment or docstring (I prefer comments for tests, FTR) explaining what's being tested, in the form:

    def test_frobnob_goes_boing(self):
        # The frobnob will go boing if pushed.

Secondly, I can't tell (because there are no comments to tell me) whether test_initialise() is huge for a reason or whether it can be split into individual tests. I'm guessing that it's huge for a reason, because all the asserts seem to be of the nature of

    foo = bar.foo
    self.assertEqual(expected_foo, foo)

Which is fine, but given the length of the method they should have some explanation with them:

    # The bar's foo will have been updated.
    foo = bar.foo
    self.assertEqual(expected_foo, foo)

As I said, I don't have the context for knowing if the test conversion is accurate, since the old test seems to rely a lot on the output from the script. I'd appreciate it if you could find someone else (Jools maybe) who can confirm that (I'm aware that this is overkill but I'm also aware that this is a Soyuz branch).

I'm marking the branch as needs fixing pending the changes above.