Code review comment for lp:~eric97/bzr/dump-btree-api

On Thu, 2011-02-17 at 09:46 +0000, Vincent Ladeuil wrote:
> Review: Approve
> Nicely done !

Thanks.

> You can just use:
>
> test_raw_output_3nodes = '''Root node:
> B+Tree Graph Index 2
> ...
> '''

That won't match, so it requires a change -- either:
    test_raw_output_0nodes = re.sub("(?m)^ *", "",
        '''Root node:
        B+Tree Graph Index 2
        ...
        ''')
or:
    test_raw_output_0nodes = \
'''Root node:
B+Tree Graph Index 2
...
'''

Is it still an improvement? If so, which is preferable?

> (all rules have exceptions, copy/paste is bad, but copying an expected
result from a failed test is way faster than typing it from scratch).

I don't understand: why is one easier to copy than the other?

As for whether copying is bad, well, those data definitions, and most of
the method that reads them, are already duplicated between the two test
files. I didn't like doing that, but liked even less to reach across
from one to the other:
    self.assertEquals(some_other_module.SomeClass.expected_value,
actual_value)
Plus, for the module that does that, it would put the test data far from
the tests themselves, thus awkward to look up.

  - Eric