Code review comment for lp:~jml/canonical-identity-provider/reliable-tests

Revision history for this message
Jonathan Lange (jml) wrote :

On 29 November 2012 20:19, Ricardo Kirkner
<email address hidden> wrote:
> Review: Approve
>
> Wow, the work you've done here is impressive. Thanks a lot for digging into it.
>
> I am quite annoyed at finding about this cache in gargoyle this way but I can only blame myself for it (it doesn't seem to be documented anywhere in gargoyle) even though it's part of it's public api and clearly affects how you work with tests involving flags.
>
> The simplest change we could do (though we might be affected by the caching issue later as you mention) is to decorate the failing tests with a decorator that makes sure the TWOFACTOR flags is disabled while running the tests. This would look like
>
> @switches(TWOFACTOR=False)
> def test_decoration_{no,with}_params(self):
> ...
>
> If you still think we should do the caching work to prevent other possible future issues, I'm ok with you exploring that route.

I do. At the moment I think the "right" thing to do is wrap Django's
cache layer, record cache entries & clear them out at the end of
tests. I'll chase that up in https://launchpad.net/djangofixture,
since it's got little to do w/ openid & will probably be useful for
other things.

In the mean time, will add the switches decorator (a variant of option
4) so we can get some momentum on pre-merge testing.

jml

« Back to merge proposal