[Django] #35402: DatabaseFeatures.django_test_skips crashes when it references a class in another test module

[Django]

#35402: DatabaseFeatures.django_test_skips crashes when it references a class in
another test module
-------------------------------------+-------------------------------------
Reporter: Tim | Owner: nobody
Graham |
Type: Bug | Status: new
Component: Database | Version: dev
layer (models, ORM) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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
{{{#!diff
diff --git a/django/db/backends/sqlite3/features.py
b/django/db/backends/sqlite3/features.py
index d95c6fb2d1..51229fe970 100644
--- a/django/db/backends/sqlite3/features.py
+++ b/django/db/backends/sqlite3/features.py
@@ -68,8 +68,7 @@ class DatabaseFeatures(BaseDatabaseFeatures):
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:
{{{#!shell
$ ./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:
{{{#!diff
diff --git a/django/db/backends/base/creation.py
b/django/db/backends/base/creation.py
index 6856fdb596..f6cc270b16 100644
--- a/django/db/backends/base/creation.py
+++ b/django/db/backends/base/creation.py
@@ -350,6 +350,9 @@ class BaseDatabaseCreation:
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))
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35402>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35402: DatabaseFeatures.django_test_skips crashes when it references a class in
another test module
-------------------------------------+-------------------------------------

Reporter: Tim Graham | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* stage: Unreviewed => Accepted

Comment:

Thank you for the report!
--
Ticket URL: <https://code.djangoproject.com/ticket/35402#comment:1>

[Django]

#35402: DatabaseFeatures.django_test_skips crashes when it references a class in
another test module
-------------------------------------+-------------------------------------

Reporter: Tim Graham | Owner: jon-mcfee
Type: Bug | Status: assigned

Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by jon-mcfee):

* owner: nobody => jon-mcfee
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35402#comment:2>

[Django]

#35402: DatabaseFeatures.django_test_skips crashes when it references a class in
another test module
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: jon-mcfee
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by 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?
--
Ticket URL: <https://code.djangoproject.com/ticket/35402#comment:3>

[Django]

#35402: DatabaseFeatures.django_test_skips crashes when it references a class in
another test module
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: jon-mcfee
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/35402#comment:4>

[Django]

#35402: DatabaseFeatures.django_test_skips crashes when it references a class in
another test module
-------------------------------------+-------------------------------------

Reporter: Tim Graham | Owner: (none)
Type: Bug | Status: new

Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by jon-mcfee):

* owner: jon-mcfee => (none)
* status: assigned => new

--
Ticket URL: <https://code.djangoproject.com/ticket/35402#comment:5>

[Django]

#35402: DatabaseFeatures.django_test_skips crashes when it references a class in
another test module
-------------------------------------+-------------------------------------

Reporter: Tim Graham | Owner: jon-mcfee
Type: Bug | Status: assigned

Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by jon-mcfee):

* owner: (none) => jon-mcfee

* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35402#comment:6>

[Django]

#35402: DatabaseFeatures.django_test_skips crashes when it references a class in
another test module
-------------------------------------+-------------------------------------
Reporter: Tim Graham | Owner: jon-mcfee
Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/35402#comment:7>

[Django]

#35402: DatabaseFeatures.django_test_skips crashes when it references a class in
another test module
-------------------------------------+-------------------------------------

Reporter: Tim Graham | Owner: Jacob
| Walls

Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* has_patch: 0 => 1
* owner: (none) => Jacob Walls

Comment:

[https://github.com/django/django/pull/18203 Alternative PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/35402#comment:8>

[Django]

#35402: DatabaseFeatures.django_test_skips crashes when it references a class in
another test module
-------------------------------------+-------------------------------------

Reporter: Tim Graham | Owner: jon-mcfee

Type: Bug | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 1

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* needs_docs: 0 => 1
* owner: Jacob Walls => jon-mcfee

Comment:

Un-commandeering the issue ;)

Needs an update to the docs for `import_string()`.
--
Ticket URL: <https://code.djangoproject.com/ticket/35402#comment:9>