Opened 4 years ago

Closed 4 years ago

#17194 closed Bug (fixed)

Auth tests failing on non-US locales

Reported by: hcarvalhoalves Owned by: nobody
Component: contrib.auth Version: master
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 claudep 4 years ago.
Fix language isolation of contrib.auth tests
17194-2.diff (7.6 KB) - added by claudep 4 years ago.
Add middleware isolation for test_session_not_accessed
17194-3.diff (30.7 KB) - added by claudep 4 years ago.
Alternate approach with DefaultSettingsTestCase
auth-tests-localization.diff (11.9 KB) - added by rabio 4 years ago.
Another approach: localize strings in tests instead of setting fixed locale
django-17194-localise-auth-tests.patch (15.8 KB) - added by bpeschier 4 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 4 years ago by julien

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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 overriding the locale. Would you like to provide a patch?

Last edited 4 years ago by julien (previous) (diff)

Changed 4 years ago by claudep

Fix language isolation of contrib.auth tests

comment:2 Changed 4 years ago by claudep

  • 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.

Changed 4 years ago by claudep

Add middleware isolation for test_session_not_accessed

Changed 4 years ago by claudep

Alternate approach with DefaultSettingsTestCase

comment:3 Changed 4 years ago by claudep

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 Changed 4 years ago by claudep

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

comment:5 follow-up: Changed 4 years ago by hcarvalhoalves

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

comment:6 in reply to: ↑ 5 Changed 4 years ago by claudep

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 Changed 4 years ago by jezdez

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 Changed 4 years ago by claudep

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 Changed 4 years ago by rabio

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 4 years ago by rabio (previous) (diff)

Changed 4 years ago by rabio

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

comment:10 Changed 4 years ago by bpeschier

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.

Changed 4 years ago by bpeschier

comment:11 Changed 4 years ago by jezdez

  • Resolution set to fixed
  • Status changed from new to closed

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