Code review comment for lp:~lifeless/launchpad/bug-227494

Revision history for this message
Robert Collins (lifeless) wrote :

18:47 < lifeless> wgrant: follow up on https://code.launchpad.net/~lifeless/launchpad/bug-227494/+merge/62943 ?
18:49 < wgrant> lifeless: You are arbitrarily changing the user to an admin half-way through because the owner has no privs.
18:49 < wgrant> That may be invalidating subsequent tests.
18:49 < wgrant> Even if it isn't, it's rather confusing to change it partially globally when only that one block needs it.
18:50 < lifeless> wgrant: have you looked through that doctest ?
18:50 < lifeless> hint: it does this a lot already
18:50 < wgrant> It's a doctest. No.
18:50 < lifeless> and the previous user was one it thought was an admin.
18:50 < wgrant> Ah, I see.
18:50 < wgrant> OK.
18:50 < wgrant> An admin, but not an Admin.
18:50 < wgrant> Not an ~admin, sorry.
18:50 < lifeless> yes, a ~admin
18:51 < wgrant> How did it stop being an admin?
18:51 < lifeless> because it was mark
18:51 < wgrant> Because it was the owner of ~admins?
18:51 < lifeless> yes
18:51 < wgrant> But the test removed that membership?
18:51 < lifeless> yes
18:51 < wgrant> ahhhhhhhh
18:51 < lifeless> it was BROKEN
18:51 < wgrant> Carry on, then.
18:51 < wgrant> Preferably beating that test to death on the way.

« Back to merge proposal