Opened 13 years ago

Closed 13 years ago

#17194 closed Bug (fixed)

Auth tests failing on non-US locales

Reported by: Henrique C. Alves Owned by: nobody
Component: contrib.auth Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Auth tests are failing on current trunk (1.4). An error message is being compared as a test condition, but this fails since the error message will return a translated string (in this particular case, pt_br).

Destroying old test database 'default'...
..........F
======================================================================
FAIL: test_inactive_user (django.contrib.auth.tests.forms.AuthenticationFormTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/hcalves/.virtualenvs/omelete.v2/src/django/django/contrib/auth/tests/forms.py", line 105, in test_inactive_user
    [u'This account is inactive.'])
AssertionError: [u'Esta conta est\xe1 inativa.'] != [u'This account is inactive.']

Attachments (5)

17194-1.diff (6.1 KB ) - added by Claude Paroz 13 years ago.
Fix language isolation of contrib.auth tests
17194-2.diff (7.6 KB ) - added by Claude Paroz 13 years ago.
Add middleware isolation for test_session_not_accessed
17194-3.diff (30.7 KB ) - added by Claude Paroz 13 years ago.
Alternate approach with DefaultSettingsTestCase
auth-tests-localization.diff (11.9 KB ) - added by rabio 13 years ago.
Another approach: localize strings in tests instead of setting fixed locale
django-17194-localise-auth-tests.patch (15.8 KB ) - added by Bas Peschier 13 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 by Julien Phalip, 13 years ago

Triage Stage: UnreviewedAccepted

That's right, multiple tests are failing when the project's settings.LANGUAGE_CODE isn't English. Those tests need to be made more isolated by temporarily overring the locale. Would you like to provide a patch?

Version 0, edited 13 years ago by Julien Phalip (next)

by Claude Paroz, 13 years ago

Attachment: 17194-1.diff added

Fix language isolation of contrib.auth tests

comment:2 by Claude Paroz, 13 years ago

Has patch: set

Here's my proposal to fix it. Just setting LANGUAGE_CODE isn't sufficient to change current language, that's why I changed override_settings to special-case LANGUAGE_CODE setting change. Might be useful in other tests, too.

The other solution would be to enclose each string comparison test with translation.override, but it seems not very DRY to me.

by Claude Paroz, 13 years ago

Attachment: 17194-2.diff added

Add middleware isolation for test_session_not_accessed

by Claude Paroz, 13 years ago

Attachment: 17194-3.diff added

Alternate approach with DefaultSettingsTestCase

comment:3 by Claude Paroz, 13 years ago

After discussion with Julien on irc, I've come with a radically different approach, with a new DefaultSettingsTestCase class that strives to completely isolate the contrib.auth tests from custom project settings.

comment:4 by Claude Paroz, 13 years ago

Note also that the latest patch might also fix #13394 and #16366. (see also #9156)

comment:5 by Henrique C. Alves, 13 years ago

The patch only overrides the locale for Django's own tests, correct? User-provided tests will still run on the settings.py locale?

in reply to:  5 comment:6 by Claude Paroz, 13 years ago

Replying to hcarvalhoalves:

The patch only overrides the locale for Django's own tests, correct? User-provided tests will still run on the settings.py locale?

Yes, of course.

comment:7 by Jannis Leidel, 13 years ago

Hm, I'm not sure why another test case class is needed, mind elaborating? I always thought @override_settings and self.settings() would be enough, isolating test classes has worked elsewhere, so why shouldn't it here?

comment:8 by Claude Paroz, 13 years ago

With @override_settings and self.settings(), the logic is "Current settings except this one and this one". You never really no if suddenly a new different setting has a new side effect.

With this class, the logic is reversed. Default settings are the base (sort of a sandbox), and you only specify the particular settings (if any) which you want to specifically test.

There are pros and cons, as usual. The DefaultSettingsTestCase approach does address the specific issue of being able to test with a known and stable set of settings. Whether it is wanted or not, that's a design decision.

comment:9 by rabio, 13 years ago

Let me propose another approach. Instead of setting fixed locale for auth tests, use localized strings in tests instead. Patch attached below does exactly that.

I'm using there a kind of a poor man escaping filter. It probably should be substituted with one from template framework, though.

Last edited 13 years ago by rabio (previous) (diff)

by rabio, 13 years ago

Another approach: localize strings in tests instead of setting fixed locale

comment:10 by Bas Peschier, 13 years ago

If you go that approach, it might be a good idea to actually place the error messages on the relevant forms and fields so you can reuse them in the tests.

It's also better to reuse the html-escape from django (in django.utils.html) instead of creating a new one.

Attached a patch.

by Bas Peschier, 13 years ago

comment:11 by Jannis Leidel, 13 years ago

Resolution: fixed
Status: newclosed

In [17204]:

Fixed #17194 -- Made sure the auth form tests work if a language other than English is activated by moving the error message translation strings into class level dictionaries. Many thanks to Claude Paroz, rabio and Bas Peschier for their initial work on this.

Note: See TracTickets for help on using tickets.
Back to Top