Code review comment for lp:~mbp/launchpad/flags-gui

> > OK. Can I make it just browser.py, or should I have a submodule with
> > a named file within it?
>
> If a nontrivial __init__.py is already a normal thing to write, then there's
> no reason to worry about it here after all. So it's fine to leave it as it
> is.

I already moved it, and perhaps it's better that way as it leaves room for growth.

> > Hm, so perhaps (if it's possible) it would be clearer to write something
> like:
> >
> > browser = getUserBrowserAsTeamMember(teams)
> > self.assertRaises(Unauthorized, browser.open(url))
>
> Yes, exactly. And AFAICS that should work.

Yep, and that is better.

> I think ignoring whitespace would be a fine thing to do, assuming that a
> separate test verifies that the whitespace conforms to expectations.
>
> A Matcher would be very nice here.

Done, and reformatted to not be specially indented.

>

> > Actually I think for most testing, we want to just change the
> > featurecontroller, not actually change the database. And perhaps it
> > should be a test fixture not on that monolith class?
>
> I could imagine a need for both "create a rule in the database" and "create a
> rule in whatever rules source I'm currently using." Which you want this
> function for is up to you, and I defer to your judgment.

There's some other existing code that's directly poking things in to the db. I'd like to reconsider the way it sets things up, but separately from this.