[PATCH V2] New factory class to create arbitrary model objects, to be used in tests
jk at ozlabs.org
Fri Apr 29 11:51:24 EST 2011
> > > +# 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
> > > +
> > > + 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. :)
> 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(
person = Person(email=self.getUniqueEmailAddress(),
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,
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.
More information about the Patchwork