Code review comment for lp://staging/~jtv/launchpad/bug-711077

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Okay, I extended PersonSet.getPrecachedPersonsFromIDS (horrible name!) and used that instead. The fetchAuthorsForDisplay method grew out of something that started fetching at RevisionAuthor, which turned out to be subobtimal.

Jon: moving things into setUp doesn't save anything, since each test method gets its own TestCase object to play with. Each needs to be set up anyway. Luckily creating objects usually isn't very expensive, and we generally prefer it over hidden dependencies across arbitrary reuse. In practice, object creation in setUp can even slow things down as not all tests may make use of all the setup!

Also, we've learned that creating implicit state from setUp easily obscures what the test does, so it's generally better to create helper methods for setup than it is to chuck everything into the TestCase from setUp.

« Back to merge proposal