Merge lp://staging/~bigkevmcd/django-openid-auth/fix-bug-897651 into lp://staging/~django-openid-auth/django-openid-auth/trunk

Proposed by Kevin McDermott
Status: Superseded
Proposed branch: lp://staging/~bigkevmcd/django-openid-auth/fix-bug-897651
Merge into: lp://staging/~django-openid-auth/django-openid-auth/trunk
Diff against target: 134 lines (+94/-6)
3 files modified
django_openid_auth/admin.py (+3/-5)
django_openid_auth/tests/__init__.py (+2/-1)
django_openid_auth/tests/test_admin.py (+89/-0)
To merge this branch: bzr merge lp://staging/~bigkevmcd/django-openid-auth/fix-bug-897651
Reviewer Review Type Date Requested Status
James Henstridge Approve
Review via email: mp+84232@code.staging.launchpad.net

This proposal has been superseded by a proposal from 2012-04-25.

Description of the change

This adds some tests for the admin view override, and fixes this bug, the view being called no-longer exists in the views file.

To post a comment you must log in.
92. By Kevin McDermott

Assert properly.

Revision history for this message
James Henstridge (jamesh) wrote :

Looks good, although the copyright date in the new file should be updated.

review: Approve
93. By Kevin McDermott

Update the copyright date in the newly added test_admin.py

Revision history for this message
Kevin McDermott (bigkevmcd) wrote :

> Looks good, although the copyright date in the new file should be updated.

I've fixed it to be 2012...is there a plan to generally update all files?

Revision history for this message
James Henstridge (jamesh) wrote :

Yeah. I guess all files should be updated for the next release.

Revision history for this message
James Henstridge (jamesh) wrote :

I just tried merging this, and received the following errors:

======================================================================
FAIL: test_admin_site_with_openid_login_authenticated_non_staff (django_openid_auth.tests.test_admin.SiteAdminTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/james/src/ubunet/django-openid-auth/django_openid_auth/tests/test_admin.py", line 74, in test_admin_site_with_openid_login_authenticated_non_staff
    response.content, 'Missing error message in response')
AssertionError: False is not True : Missing error message in response

======================================================================
FAIL: test_admin_site_with_openid_login_non_authenticated_user (django_openid_auth.tests.test_admin.SiteAdminTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/james/src/ubunet/django-openid-auth/django_openid_auth/tests/test_admin.py", line 82, in test_admin_site_with_openid_login_non_authenticated_user
    self.assertEqual(302, response.status_code)
AssertionError: 302 != 200

----------------------------------------------------------------------

Looking at the tests, the problem is that the behaviour being tested reads the settings variable at import time, and while the test is probably updating the setting after django_openid_auth.admin has already been imported by the django.contrib.admin code.

94. By Kevin McDermott

Fix to work with Django 1.3.0 and 1.3.1.

95. By Kevin McDermott

Tested with Django 1.3.0, 1.3.1, 1.2.5

96. By Kevin McDermott

Merge with parent.

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches