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

[Guilherme Salgado]

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.

Cheers,

-- 
Guilherme Salgado <https://launchpad.net/~salgado>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/patchwork/attachments/20110228/b2dc6cb3/attachment.pgp>