On Mon, 02 Aug 2010 15:58:40 -0000, Jonathan Lange <[email protected]> wrote: > What's the reason for this change? Where are the corresponding tests?
Sorry, I forgot to mention it in the cover letter. When you register a Collection as a utility it is instantiated by the zcml. However, as the store is queried in the constructor this can fail if there is not a utility registered for the store yet. How this manifested was that as soon as I added the utility I could no longer run any tests as there was an exception during zcml processing. I'm not sure how I would test this directly? A test that created a Collection and tested that .store was None? It does make me realise that I should probably add a test that the utility returns something sensible? > In IBranchCollection, we have a *args method for something similar. Do > you think that would be nicer here? The implementation is *purposes, so the interface needs correcting. > Maybe it's my crappy email client, but the indentation on this looks messed > up. It's non-standard as it was copy-paste code, I will fix. > > + # FIXME: Include private PPA's if user is an uploader > > + return self.refine( > > + Or(public_archive, > > Archive.ownerID.is_in(user_teams_subselect))) > > > > Are you going to fix this in this branch? Won't this be buggy if > someone uses it? This is copy-paste code, so we apparently have survived this far without needing it. I can look at fixing it though. > These tests are good, thanks. > > One thing I've done with similar tests is have a setUp() that removes > all of the data in question (e.g. removing all branches). That way you > can do actual equality testing rather than testing to if if objects > are members in tests. > > You don't have to though. That's a good idea, I'll look for your code to learn from it. Thanks, James -- https://code.launchpad.net/~james-w/launchpad/archive-collection/+merge/31499 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~james-w/launchpad/archive-collection into lp:launchpad/devel. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
