Code review comment for lp:~jamalta/launchpad/bug-127489

Revision history for this message
Jamal Fanaian (jamalta) wrote :

Sorry forgot to mention this on my last comment. I will try to ping someone in #launchpad-reviews come Monday to go over my branch, that way we don't have to go back and forth over email.

Thanks!

> I have added a test that makes sure the categories returned by
> PersonView.contributed_categories are sorted correctly. Instead of running the
> script to update Karma I am using the test factories and creating the
> KarmaCache records based on how the Karma update script does it.
>
> I may not be doing this the best way, so I am requesting a review and
> hopefully I can chat with someone about it come Monday to figure out what
> needs to be improved or changed.
>
> Thanks again for all your help and sorry this took so long!
>
> > On Fri, 2009-08-07 at 09:19 +0000, Gavin Panella wrote:
> > > Have a look at using lp.testing.factory.LaunchpadObjectFactory. You can
> use
> > this to quickly create, say, branches, so that there are code artifacts for
> > your user. That will probably be enough to test this code.
> >
> > I don't think so. That list of categories is generated based on the kind
> > of karma the person has on the KarmaCache table, and that table is
> > populated by a script. Even though the person would get karma when
> > factory.makeBranch() is called, it wouldn't end up in the KarmaCache
> > table until we ran the script. We could run the script in the test, but
> > that'd make the test *a lot* slower.
> >
> > That's why I suggested a new method on LPObjectFactory to create the
> > KarmaCache entries directly.
> >
> > Jamal, I remember you were able to write the new factory method, so you
> > should be quite close to getting a test for your fix. If you need any
> > help, just drop by #launchpad-reviewers and I'll help you.
> >
> > Cheers,
> >
> > --
> > Guilherme Salgado <email address hidden>

« Back to merge proposal