DatabaseFeatures.django_test_skips crashes when it references a class in another test module

[Tim Graham]

Since the introduction of DatabaseFeatures.django_test_skips (#32178), I've used it to skip test classes, however, this can crash if there are multiple test modules in a package.

To reproduce, first make this modification to skip a class

django/db/backends/sqlite3/features.py

@@ -68 +68 @@
     def django_test_skips(self):
         skips = {
             "SQLite stores values rounded to 15 significant digits.": {
-                "model_fields.test_decimalfield.DecimalFieldTests."
-                "test_fetch_from_db_without_float_rounding",
+                "model_fields.test_decimalfield.DecimalFieldTests",
             },
             "SQLite naively remakes the table on field alteration.": {

Then try to execute a test module in model_fields besides the one with the skip:

$ ./tests/runtests.py model_fields.test_autofield
Testing against Django installed in '/home/tim/code/django/django' with up to 3 processes
Found 62 test(s).
Creating test database for alias 'default'...
Traceback (most recent call last):
  File "/home/tim/code/django/django/utils/module_loading.py", line 30, in import_string
    return cached_import(module_path, class_name)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tim/code/django/django/utils/module_loading.py", line 16, in cached_import
    return getattr(module, class_name)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: module 'model_fields' has no attribute 'test_decimalfield'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/tim/code/django/./tests/runtests.py", line 784, in <module>
    failures = django_tests(
               ^^^^^^^^^^^^^
  File "/home/tim/code/django/./tests/runtests.py", line 422, in django_tests
    failures = test_runner.run_tests(test_labels)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tim/code/django/django/test/runner.py", line 1066, in run_tests
    old_config = self.setup_databases(
                 ^^^^^^^^^^^^^^^^^^^^^
  File "/home/tim/code/django/django/test/runner.py", line 964, in setup_databases
    return _setup_databases(
           ^^^^^^^^^^^^^^^^^
  File "/home/tim/code/django/django/test/utils.py", line 206, in setup_databases
    connection.creation.create_test_db(
  File "/home/tim/code/django/django/db/backends/base/creation.py", line 102, in create_test_db
    self.mark_expected_failures_and_skips()
  File "/home/tim/code/django/django/db/backends/base/creation.py", line 356, in mark_expected_failures_and_skips
    test_case = import_string(test_case_name)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/tim/code/django/django/utils/module_loading.py", line 32, in import_string
    raise ImportError(
ImportError: Module "model_fields" does not define a "test_decimalfield" attribute/class

A naive fix:

django/db/backends/base/creation.py

@@ -350 +350 @@
                 test_app = test_name.split(".")[0]
                 # Importing a test app that isn't installed raises RuntimeError.
                 if test_app in settings.INSTALLED_APPS:
+                    # If this is a a test class, it may need to be imported.
+                    if test_name.count(".") == 2:
+                        import_string(test_name)
                     test_case = import_string(test_case_name)
                     test_method = getattr(test_case, test_method_name)
                     setattr(test_case, test_method_name, skip(reason)(test_method))

[Sarah Boyce]

Thank you for the report!

[jon-mcfee]

In the backends.base.test_creation.TestMarkTests, it seems like there is a test for skipping a class.

"skip test class": {
       "backends.base.test_creation.SkipTestClass",
}

Interestingly, this passes, yet the ImportError is still thrown for seemingly the same behavior in django/db/backends/base/features.py. I can't really understand why import_string accepts "backends.base.test_creation", but fails on "model_fields.test_decimalfield". Can anyone offer any advice?

[Tim Graham]

I believe the problem has something to do with module loading. When running only model_fields.test_autofield, model_fields.test_decimalfield is not loaded, thus the import error. If I understood the problem completely, I would have offered the fix.

[jon-mcfee]

Okay, I made a PR. I'm not sure if it's naive to assume that tests must start with "test", but I passed all local tests and automated testing, so please let me know if that should work.

[Jacob Walls]

​Alternative PR

[Jacob Walls]

Un-commandeering the issue ;)

Needs an update to the docs for import_string().

[Jacob Walls]

​PR to cover uncovered lines related to the proposed fix. May help inform decision making about the eventual solution.

[Sarah Boyce <42296566+sarahboyce@…>]

In b99c608e:

Refs #35402 -- Added tests for invalid usage of submodules in some settings.