Skip to content

Added regression test for #17967. #22

Merged
merged 2 commits into from Apr 29, 2012

2 participants

@aviraldg

@aaugustin Here's a regression test for #17967. Hope it's okay.

@alex alex commented on an outdated diff Apr 29, 2012
tests/regressiontests/admin_views/tests.py
@@ -572,6 +572,24 @@ def test_allowed_filtering_15103(self):
except SuspiciousOperation:
self.fail("Filters should be allowed if they are defined on a ForeignKey pointing to this model")
+ def test_hide_change_password(self):
+ """
+ Tests if the "change password" link in the admin is hidden if the User
+ does not have a usable password set.
+ (against 9bea85795705d015cdadc82c68b99196a8554f5c)
+ """
+ user = User.objects.get(username='super')
+ password = user.password
+ user.set_unusable_password()
+ user.save()
+
+ response = self.client.get('/test_admin/admin/')
+ if reverse('admin:password_change') in response.content:
+ self.fail('The "change password" link should not be displayed if a user does not have a usable password.')
@alex
Django member
alex added a note Apr 29, 2012

It's generally the style of the Django tests to do:

self.assertFalse(..., msg="foo")

rather than have the if statement (there may even be an assertNotContains or similar, that'd be even more optimal)..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@alex alex and 1 other commented on an outdated diff Apr 29, 2012
tests/regressiontests/admin_views/tests.py
+ """
+ Tests if the "change password" link in the admin is hidden if the User
+ does not have a usable password set.
+ (against 9bea85795705d015cdadc82c68b99196a8554f5c)
+ """
+ user = User.objects.get(username='super')
+ password = user.password
+ user.set_unusable_password()
+ user.save()
+
+ response = self.client.get('/test_admin/admin/')
+ if reverse('admin:password_change') in response.content:
+ self.fail('The "change password" link should not be displayed if a user does not have a usable password.')
+
+ user.password = password
+ user.save()
@alex
Django member
alex added a note Apr 29, 2012

No need to save the user here, we only needed to test the one assert.

@aviraldg
aviraldg added a note Apr 29, 2012

What about test cases that might come after this one?

@alex
Django member
alex added a note Apr 29, 2012

Test cases are isolated, objects created in one aren't visible to another.

@aviraldg
aviraldg added a note Apr 29, 2012

The user object here is actually the one created from the fixtures that we're modifying.

@alex
Django member
alex added a note Apr 29, 2012

It doesn't matter, the semantics are that the changes you make aren't visible to other tests. Remove these lines and try running all the tests, you'll see!

@aviraldg
aviraldg added a note Apr 29, 2012

Oh, I didn't know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@alex alex merged commit e75bd7e into django:master Apr 29, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.