[PATCH V2] New factory class to create arbitrary model objects, to be used in tests

[Jeremy Kerr]

Hi Guilherme,

> > > +# Patchwork - automated patch tracking system
> > > +# Copyright (C) 2011 Jeremy Kerr <jk at ozlabs.org>
> > 
> > Probably best to add your name here :)
> 
> Should I add myself as the author and keep you as the copyright holder?

Since I haven't written anything in that file, best just to put your
name there.

> > 
> > > +
> > > +    def __init__(self):
> > > +        # Initialise the unique identifier.
> > > +        self.integer = count(randint(0, 1000000))
> > 
> > As a general rule, I'm averse to using random data in test cases. We may
> > see non-repeatable failures here, if we create multiple ObjectFactories
> > that happen to have colliding unique IDs.
> > 
> > So, perhaps we should make ObjectFactory a singleton (referenced though
> > an exported module variable, something like 'factory'), and just use:
> > 
> >     self.integer = count()
> > 
> > How does that sound?
> 
> I can't think of any reason for not having ObjectFactory as a singleton,
> and I think your concern is valid, so consider this done. :)

OK, great.

> Indeed. I've added 'content' as an optional argument and when it's not
> passed I set it to a minimal patch.
> 
> I'll submit a third version with these changes as soon as I have an
> answer from you about the copyright/authorship question. Oh, and I'll
> also submit new versions of the patches that use this as they now use
> the factory singleton.

Sounds good, thanks. When you re-roll this patch, could you avoid
breaking lines right after the opening bracket? So instead of:

        person = Person(
            email=self.getUniqueEmailAddress(), name=self.getUniqueString())

We get:

        person = Person(email=self.getUniqueEmailAddress(),
                        name=self.getUniqueString())

Also, most of the codebase has spaces around the '=' in kwargs:

        person = Person(email = self.getUniqueEmailAddress(),
                        name = self.getUniqueString())

- I'm aware that this is not what PEP-8 says, but I'd prefer to keep it
consistent, and perhaps do a global change at some point.

Also, the initial state is defined as one with ordering = 0:

+        patch = Patch(
+            project=project, msgid=msgid, name=self.getUniqueString(),
+            submitter=submitter, date=date,
+            state=State.objects.get(name='New'))

This should be:

	state = State.objects.get(ordering = 0)

- as there may not be a 'New' state in a particular patchwork setup.
Even better, you can leave 'state' unset, and the Patch.save() will set
it to this default.

Cheers,


Jeremy