Opened 2 years ago

Closed 22 months ago

Last modified 22 months ago

#34180 closed Bug (fixed)

Document that setting language in tests affects other tests

Reported by: Václav Řehák Owned by: Faris Naimi
Component: Documentation Version: 4.1
Severity: Normal Keywords: documentation i18n tests
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The testing documentation https://docs.djangoproject.com/en/4.1/topics/testing/tools/#setting-the-language suggests to set language in tests using a cookie or http header. But it is not obvious to the reader that doing so will switch the Django active language for all other subsequent tests.

In our case we had random test failures in parallel run depending on the actual order of tests executed. This is easily reproducible with the following test file

from django.contrib.auth.forms import AuthenticationForm
from django.test import TestCase

class ViewTestCase(TestCase):
    def test_czech_request(self):
        self.client.get("/", HTTP_ACCEPT_LANGUAGE="cs-cz")


class FormTestCase(TestCase):
    def test_form(self):
        f = AuthenticationForm(data={})
        self.assertEqual(f.errors['username'], ['This field is required.'])

This testsuite passes in normal run (form is tested before the view) but fails when running tests with --reverse as the view test switches Django to Czech language and the validation errors of the form are translated.

I'm not sure if this can be fixed (should the test runner activate the default language before each test?) but I suggest to at least document it and probably recommend the user to activate the default language in tearDown method.

    def tearDown(self):
        translation.activate(settings.LANGUAGE_CODE)

Change History (14)

comment:1 by Carlton Gibson, 2 years ago

Triage Stage: UnreviewedAccepted

Yes, this reproduces. Thanks!

Expected behaviour was doc'd in 3c447b108ac70757001171f7a4791f493880bf5b.

I wonder if this is a regression somewhere? (Update: but it's the same behaviour on stable/3.2.x)

It would be good to resolve as not merely a docs fix but with a quick glance there's no obvious way forward here, so suggesting a translation.deactivate() at the end of the test may be the best option. 🤔

Last edited 2 years ago by Carlton Gibson (previous) (diff)

comment:2 by Carlton Gibson, 2 years ago

#34181 was a duplicate which found source of this behaviour:

Django's LocaleMiddleware only activates a translation, but doesn't deactivate it since #5241 / ​
https://github.com/django/django/commit/aa089b106b6cfc9a47cd54a0f9eb44bd44811ed9.

Given that this was for Django 1.6, ≈9 years ago now, I think we can consider it established behaviour.
(Again, I think a note in the docs is likely option 1.)

comment:3 by Raphaël Barrois, 2 years ago

I'll add the results of the extra investigation I did in #34181 (sorry for not seeing this ticket, it wasn't here when I started writing mine 😅):

However, the pattern shown here is very close to typical code used in tests, where strings marked for translation are tested against the value in the default locale.

I feel that a documentation fix stating "Every test asserting on a string marked for translation should wrap said assertion with translation.override" wouldn't actually help users, as that would add a significant amount of boilerplate to test cases.

If the recommended solution is to call translation.deactivate() in each test case' tearDown, I'd suggest including that in Django's source code.
Another option could be for the test client to deactivate translations post-request when it detects that the LocaleMiddleware is enabled.
Otherwise, we could also explore the possibility to restore the locale context in some post-request handler, server-side.

comment:4 by Václav Řehák, 2 years ago

To fix this at the moment, we will be adding translation.deactivate() to the base class of our tests. I wonder if there is any downside of including this in Django's TestCase tearDown. I'm not fan of keeping the current behavior as it can lead to difficult-to-debug random failures of tests.

comment:5 by Faris Naimi, 2 years ago

Owner: changed from nobody to Faris Naimi
Status: newassigned

comment:6 by Faris Naimi, 2 years ago

The documentation was modified to explain the issue and the possibility of activating the default language in the tearDown method.

This is now available from the branch "ticket_34180" on the repo:

https://github.com/fnaimi66/django.git

comment:7 by Faris Naimi, 2 years ago

Resolution: fixed
Status: assignedclosed

comment:8 by Václav Řehák, 2 years ago

Resolution: fixed
Status: closednew

How can you close ticket just because there is a commit in a Github fork? The issue is not closed until is it merged into Django source code (or wontfixed).

comment:9 by Tim Graham, 2 years ago

Has patch: set

comment:10 by Carlton Gibson, 23 months ago

Patch needs improvement: set

PR looks OK. Just needs tweaks.

comment:11 by Mariusz Felisiak, 22 months ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:12 by Mariusz Felisiak <felisiak.mariusz@…>, 22 months ago

Resolution: fixed
Status: newclosed

In 40217d1:

Fixed #34180 -- Added note about resetting language in test tear-downs.

Co-authored-by: Faris Naimi <farisfaris66@…>

comment:13 by Mariusz Felisiak <felisiak.mariusz@…>, 22 months ago

In af396ce:

[4.2.x] Fixed #34180 -- Added note about resetting language in test tear-downs.

Co-authored-by: Faris Naimi <farisfaris66@…>

Backport of 40217d1a82b0c16cddba377325d12b2c253f402a from main

comment:14 by Mariusz Felisiak <felisiak.mariusz@…>, 22 months ago

In f586c12f:

[4.1.x] Fixed #34180 -- Added note about resetting language in test tear-downs.

Co-authored-by: Faris Naimi <farisfaris66@…>

Backport of 40217d1a82b0c16cddba377325d12b2c253f402a from main

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