Code review comment for lp:~bloodearnest/canonical-identity-provider/sso-dev

Revision history for this message
Simon Davy (bloodearnest) wrote :

>> === modified file 'src/identityprovider/tests/test_command_cleanup.py'
>> --- src/identityprovider/tests/test_command_cleanup.py 2015-04-30 17:20:09 +0000
>> +++ src/identityprovider/tests/test_command_cleanup.py 2015-11-17 13:23:47 +0000
>> @@ -48,8 +48,8 @@
>>
>> def make_test_accounts(self, count=0, date_created=None):
>> for i in xrange(count):
>> - email = self.factory.make_email_address(prefix='isdtest+',
>> - domain='canonical.com')
>> + email = self.factory.make_email_address(prefix='testemail+',
>> + domain='example.com')
>
> What's the rationale for this change? The job needs to clear from the DB all email addressed created by the acceptance tests, which use the isdtest+ prefix, so changing it will not trigger the right cleanup.

So, one of the things I tried to do was "normalise" the dev acceptance
test configuration, so it could just be part default devel
configuration. This would mean that a) you wouldn't need access to the
config branch to run acceptance tests and b) could run the acceptance
tests against the dev server OOTB with no config changes, which
simplified stuff a lot.

The only thing in the *dev* acceptance settings that was at all
sensitive was the isdtest email stuff, as it might leak info about our
production config.

Now, the isdtest email stuff was already in the code base anyway (like
in the above test), but I went ahead an made the default dev config
use <email address hidden>, and updated some tests that were expecting
other config.

I don't *think* this will have broken the clean up job, as that uses
the configured emails, which are unchanged in prod or staging.

But maybe I missed something?

--
Thanks

« Back to merge proposal