[PATCH] New factory which creates arbitrary model objects to be used in tests.

[Dirk Wallenstein]

On Mon, Feb 28, 2011 at 09:32:33AM -0300, Guilherme Salgado wrote:
> On Sat, 2011-02-26 at 10:05 +0100, Dirk Wallenstein wrote:
> > On Fri, Feb 25, 2011 at 04:35:51PM -0300, Guilherme Salgado wrote:
> > > 
> > > ---
> > > If there's interest in this I'd be happy to move the stuff from
> > > patchwork/tests/utils.py to here and change tests to use the factory.
> > > 
> [...]
> > > +    def makePerson(self, is_user=True):
> > > +        if is_user:
> > > +            user = self.makeUser()
> > > +        person = Person(
> > > +            email=self.getUniqueEmailAddress(), name=self.getUniqueString(),
> > > +            user=user)
> >                      ^^^ might not exist
> 
> Oops, good catch, I'll fix it.
> 
> > 
> > 
> > > +        person.save()
> > > +        return person
> > > +
> > > +    def makePatch(self, project=None, submitter=None):
> > > +        if project is None:
> > > +            project = self.makeProject()
> > > +        if submitter is None:
> > > +            submitter = self.makePerson()
> > > +        patch = Patch(
> > > +            project=project, msgid=self.getUniqueString(),
> > The email package has a helper to format msgids in 
> >     email.utils.make_msgid()
> 
> Oh, cool.  I'll use that and feed a unique string to it to "strengthen
> the uniqueness of the message id". :)
> 
> > 
> > > +            name=self.getUniqueString(), submitter=submitter,
> > > +            state=State.objects.get(name='New'))
> > > +        patch.save()
> > > +        return patch
> > 
> > I guess you want to solve the problem of creating an initial db state.
> > I personally would prefer a fixture that creates a state with more
> > reasonable names like:
> >     TestProjectA
> >     TestProjectB
> >     TestUserA
> >     TestUserB
> >     TestMaintainer
> >     etc and/or similar
> > That would make it much easier to inspect than arbitrary numbers (eg in
> > test mails).
> > Maybe have a TestFixtureMixIn class that has a 'fixtures' attribute and
> > that makes those models available as properties (wrap the lookup).
> > I assume that would cover most of the testing needs and clients would
> > not have to create it themselves.
> 
> There are a few reasons why I didn't go down that route:
> 
> First, having a fixture definition separated from the tests themselves
> make the test less readable as you have to lookup the fixture to see
> what data is being made available to the test.
> 
> Second, sharing a single fixture between multiple tests, although
> probably a time-saver in the short term, will lead to less maintainable
> tests in the long term. That's because most tests would certainly depend
> on just a subset of the fixture and it's very hard to tell what's that
> subset and whether or not the rest of the fixture affects the test in
> some way. The most common annoyance you'll see when you have a shared
> fixture is tests breaking when you add new stuff to the fixture.
> http://xunitpatterns.com/Shared%20Fixture.html has more info about
> shared fixtures and when to use them.  I think shared fixtures work fine
> if you have tests that need lots of data in the DB and don't share the
> fixture between too many tests, but that doesn't seem to be the case
> here.
> 
> Recently I've worked on a project which had a shared fixture and it was
> very painful to maintain the tests that relied on it, so we stopped
> using that and started having our tests define their own fixture. It
> made our tests more verbose but a lot more maintainable/readable.
> That's why I avoided the shared fixture pattern this time.

Well, in short, the idea was to provide a foundation of things one usually
doesn't care about what the specific settings are -- when you just need
a project, a user, a maintainer.  If you need specific settings you can
still make incremental changes to the settings you want to test.
Changes will be undone after each test.

-- 
Greetings,
Dirk