Test suite failures when using PYTHONOPTIMIZE or optimization CLI flags

[Keryn Knight]

I can't see any tickets for this, but searching for assert, assertion, -OO, -O2 doesn't necessarily cover all the ways to talk about it, so apologies if this is covered elsewhere!

For reasons entirely unrelated, a while back I added the following to my env:

export PYTHONDONTWRITEBYTECODE=true
export PYTHONUNBUFFERED=true
export PYTHONOPTIMIZE=2
export PYTHONWARNINGS=default
export PYTHONUTF8=1

Most of which have no effect, but to my consternation (because I'd entirely forgotten having done so!), using ./runtests.py suddenly started throwing a lot of errors at me (because of PYTHONOPTIMIZE=2), the list of which follows further below.

Most of the errors fall into either one of:

• using assert x ==y, 'error' being blown away entirely (in one case, this cryptically results an UnboundLocalError instead)
• checking __doc__ values, which are replaced with None

There are a handful of tests which fail at -O, and a bunch more which do so only when -OO is used. A sticky issue is that there's no great answer for CI:

• running the base suite (./runtests.py with none of the optional deps, but with -OO) means some tests/bits won't be covered.
• running each of the supported suite combos under normal + -OO is exhaustive for little benefit and many more suite executions.
• not running -OO at all means at best, any PR will pass without incident (having not broken anything), but without demonstrating things work.

The fix for assert would probably be to either skipIf them or fix the places they're used to raise an exception which is always present, like ValueError or TypeError, though this means changing the exceptions outside of tests in a couple of places :/
The fix for __doc__ is to either skipIf them or selectively test them based on sys.flags.optimize.
I'd personally prefer not to skipIf, because many of the tests assert multiple things, only one of which breaks.

At least one test is also, as far as I can tell, passing incorrectly currently:
FAIL: test_get_version_invalid_version (version.tests.VersionTests) (version=(3, 2, 0, 'gamma', 1, '20210315111111'))
This is actually failing on the length assertion rather than the alpha/beta/... one.

The complete list, then, first at -O:

======================================================================
ERROR: test_file_error_blocking (file_uploads.tests.FileUploadTests)
The server should not block when there are upload errors (bug #8622).
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/tests/file_uploads/tests.py", line 584, in test_file_error_blocking
    self.client.post('/upload_errors/', post_data)
  File "/django/django/test/client.py", line 756, in post
    response = super().post(path, data=data, content_type=content_type, secure=secure, **extra)
  File "/django/django/test/client.py", line 407, in post
    return self.generic('POST', path, post_data, content_type,
  File "/django/django/test/client.py", line 473, in generic
    return self.request(**r)
  File "/django/django/test/client.py", line 721, in request
    self.check_exception(response)
  File "/django/django/test/client.py", line 582, in check_exception
    raise exc_value
  File "/django/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/django/django/core/handlers/base.py", line 181, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/django/tests/file_uploads/views.py", line 139, in file_upload_errors
    return file_upload_echo(request)
  File "/django/tests/file_uploads/views.py", line 76, in file_upload_echo
    r = {k: f.name for k, f in request.FILES.items()}
  File "/django/django/core/handlers/wsgi.py", line 116, in FILES
    self._load_post_and_files()
  File "/django/django/http/request.py", line 328, in _load_post_and_files
    self._post, self._files = self.parse_file_upload(self.META, data)
  File "/django/django/http/request.py", line 288, in parse_file_upload
    return parser.parse()
  File "/django/django/http/multipartparser.py", line 262, in parse
    chunk = handler.receive_data_chunk(chunk, counters[i])
  File "/django/tests/file_uploads/uploadhandler.py", line 47, in receive_data_chunk
    raise CustomUploadError("Oops!")
file_uploads.uploadhandler.CustomUploadError: Oops!

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/django/tests/file_uploads/tests.py", line 585, in test_file_error_blocking
    except reference_error.__class__ as err:
UnboundLocalError: local variable 'reference_error' referenced before assignment

======================================================================
FAIL: test_real_apps_non_set (migrations.test_state.StateTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/tests/migrations/test_state.py", line 929, in test_real_apps_non_set
    ProjectState(real_apps=['contenttypes'])
AssertionError: AssertionError not raised

======================================================================
FAIL: test_flags_with_pre_compiled_regex (utils_tests.test_regex_helper.LazyReCompileTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/tests/utils_tests/test_regex_helper.py", line 54, in test_flags_with_pre_compiled_regex
    lazy_test_pattern.match('TEST')
  File "/python/3.9.5/lib/python3.9/contextlib.py", line 124, in __exit__
    next(self.gen)
  File "/django/django/test/testcases.py", line 684, in _assert_raises_or_warns_cm
    yield cm
AssertionError: AssertionError not raised

======================================================================
FAIL: test_get_version_invalid_version (version.tests.VersionTests) (version=(3, 2, 0, 'alpha', 1, '20210315111111'))
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/tests/version/tests.py", line 61, in test_get_version_invalid_version
    get_complete_version(version)
AssertionError: AssertionError not raised

======================================================================
FAIL: test_get_version_invalid_version (version.tests.VersionTests) (version=(3, 2, 0, 'gamma', 1, '20210315111111'))
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/tests/version/tests.py", line 61, in test_get_version_invalid_version
    get_complete_version(version)
AssertionError: AssertionError not raised

----------------------------------------------------------------------
Ran 15290 tests in 508.250s

FAILED (failures=4, errors=1,...

At -OO (ignoring those that occurred at -O):

======================================================================
FAIL: test_testcase_ordering (test_runner.test_discover_runner.DiscoverRunnerTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/tests/test_runner/test_discover_runner.py", line 301, in test_testcase_ordering
    self.assertIn('DocTestCase', [t.__class__.__name__ for t in suite._tests[2:]])
AssertionError: 'DocTestCase' not found in ['Test', 'TestVanillaUnittest', 'SkipDocTestCase']

======================================================================
FAIL: test_migrate_check_plan (migrations.test_commands.MigrateTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/django/test/utils.py", line 437, in inner
    return func(*args, **kwargs)
  File "/django/tests/migrations/test_commands.py", line 293, in test_migrate_check_plan
    self.assertEqual(
AssertionError: 'Plan[60 chars]alamander\n    Raw Python operation -> Grow salamander tail.\n' != 'Plan[60 chars]alamander\n    Raw Python operation\n'
  Planned operations:
  migrations.0001_initial
      Create model Salamander
-     Raw Python operation -> Grow salamander tail.
+     Raw Python operation


======================================================================
FAIL: test_migrate_plan (migrations.test_commands.MigrateTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/django/test/utils.py", line 437, in inner
    return func(*args, **kwargs)
  File "/django/tests/migrations/test_commands.py", line 430, in test_migrate_plan
    self.assertEqual(
AssertionError: "Plan[90 chars]ation -> Grow salamander tail.\nmigrations.000[199 chars]']\n" != "Plan[90 chars]ation\nmigrations.0002_second\n    Create mode[174 chars]']\n"
  Planned operations:
  migrations.0001_initial
      Create model Salamander
-     Raw Python operation -> Grow salamander tail.
+     Raw Python operation
  migrations.0002_second
      Create model Book
      Raw SQL operation -> ['SELECT * FROM migrations_book']
  migrations.0003_third
      Create model Author
      Raw SQL operation -> ['SELECT * FROM migrations_author']


======================================================================
FAIL: test_inclusion_tag_registration (template_tests.test_custom.InclusionTagTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/tests/template_tests/test_custom.py", line 316, in test_inclusion_tag_registration
    self.verify_tag(inclusion.inclusion_no_params, 'inclusion_no_params')
  File "/django/tests/template_tests/test_custom.py", line 43, in verify_tag
    self.assertEqual(tag.__doc__, 'Expected %s __doc__' % name)
AssertionError: None != 'Expected inclusion_no_params __doc__'

======================================================================
FAIL: test_simple_tag_registration (template_tests.test_custom.SimpleTagTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/tests/template_tests/test_custom.py", line 155, in test_simple_tag_registration
    self.verify_tag(custom.no_params, 'no_params')
  File "/django/tests/template_tests/test_custom.py", line 43, in verify_tag
    self.assertEqual(tag.__doc__, 'Expected %s __doc__' % name)
AssertionError: None != 'Expected no_params __doc__'

======================================================================
FAIL: test_preserve_attributes (decorators.tests.MethodDecoratorTests) (Test=<class 'decorators.tests.MethodDecoratorTests.test_preserve_attributes.<locals>.TestPlain'>)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/tests/decorators/tests.py", line 271, in test_preserve_attributes
    self.assertEqual(Test.method.__doc__, 'A method')
AssertionError: None != 'A method'

======================================================================
FAIL: test_preserve_attributes (decorators.tests.MethodDecoratorTests) (Test=<class 'decorators.tests.MethodDecoratorTests.test_preserve_attributes.<locals>.TestMethodAndClass'>)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/tests/decorators/tests.py", line 271, in test_preserve_attributes
    self.assertEqual(Test.method.__doc__, 'A method')
AssertionError: None != 'A method'

======================================================================
FAIL: test_preserve_attributes (decorators.tests.MethodDecoratorTests) (Test=<class 'decorators.tests.MethodDecoratorTests.test_preserve_attributes.<locals>.TestFunctionIterable'>)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/tests/decorators/tests.py", line 271, in test_preserve_attributes
    self.assertEqual(Test.method.__doc__, 'A method')
AssertionError: None != 'A method'

======================================================================
FAIL: test_preserve_attributes (decorators.tests.MethodDecoratorTests) (Test=<class 'decorators.tests.MethodDecoratorTests.test_preserve_attributes.<locals>.TestMethodIterable'>)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/tests/decorators/tests.py", line 271, in test_preserve_attributes
    self.assertEqual(Test.method.__doc__, 'A method')
AssertionError: None != 'A method'

======================================================================
FAIL: test_cached_property (utils_tests.test_functional.FunctionalTests) (attr='value')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/tests/utils_tests/test_functional.py", line 63, in assertCachedPropertyWorks
    self.assertEqual(get(Class).__doc__, 'Here is the docstring...')
AssertionError: None != 'Here is the docstring...'

======================================================================
FAIL: test_cached_property (utils_tests.test_functional.FunctionalTests) (attr='other')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/tests/utils_tests/test_functional.py", line 63, in assertCachedPropertyWorks
    self.assertEqual(get(Class).__doc__, 'Here is the docstring...')
AssertionError: None != 'Here is the docstring...'

======================================================================
FAIL: test_cached_property (utils_tests.test_functional.FunctionalTests) (attr='__foo__')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/tests/utils_tests/test_functional.py", line 63, in assertCachedPropertyWorks
    self.assertEqual(get(Class).__doc__, 'Here is the docstring...')
AssertionError: None != 'Here is the docstring...'

======================================================================
FAIL: test_cached_property_auto_name (utils_tests.test_functional.FunctionalTests) (attr='_Class__value')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/tests/utils_tests/test_functional.py", line 63, in assertCachedPropertyWorks
    self.assertEqual(get(Class).__doc__, 'Here is the docstring...')
AssertionError: None != 'Here is the docstring...'

======================================================================
FAIL: test_cached_property_auto_name (utils_tests.test_functional.FunctionalTests) (attr='other')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/tests/utils_tests/test_functional.py", line 63, in assertCachedPropertyWorks
    self.assertEqual(get(Class).__doc__, 'Here is the docstring...')
AssertionError: None != 'Here is the docstring...'

======================================================================
FAIL: test_attributes (decorators.tests.DecoratorsTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/django/tests/decorators/tests.py", line 91, in test_attributes
    self.assertEqual(fully_decorated.__doc__, 'Expected __doc__')
AssertionError: None != 'Expected __doc__'

----------------------------------------------------------------------
Ran 15290 tests in 480.762s

FAILED (failures=15,...

If we want to go ahead and attempt to fix/resolve these, I do have a draft branch fixing the above errors, though it obviously won't do anything on CI, may not catch everything, and may not be the desired way to do it (having excised all the assert statements entirely, for example).

[Mariusz Felisiak]

Raising specific errors instead of using assert was already discussed in #32508. We decided that remaining cases are valid and should remain as assert. I don't think it's worth changing other tests that fail with -O.

[Jacob Walls]

Keryn, your point about the second subtest of test_get_version_invalid_version() passing without testing what it claims to test looks valid to me. I would hazard that would be a welcome cleanup PR (and could ref this ticket number, even, maybe, to suggest how it was found).

[Mariusz Felisiak]

Replying to Jacob Walls:

Keryn, your point about the second subtest of test_get_version_invalid_version() passing without testing what it claims to test looks valid to me. I would hazard that would be a welcome cleanup PR (and could ref this ticket number, even, maybe, to suggest how it was found).

get_version() is an internal Django utility, so I'd not change this (see ​comment).

[Adam Johnson]

I would drop PYTHONOPTIMIZE from your environment, basically no one knows about its existence so most code assumes assert always works.