Opened 7 years ago

Closed 5 years ago

#12658 closed Bug (fixed)

Import Errors in test files cause tests to be skipped if both the tests file and the models file have been split into folders

Reported by: Matthew Schinckel Owned by: nobody
Component: Testing framework Version: 1.1
Severity: Normal Keywords:
Cc: Malcolm Box Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

If you have an import error (or an error in a file that is trying to be imported) in a test file, then the tests from that file (possible even the whole app) are skipped.

This does not seem to present unless your tests have been split into seperate files inside a tests package/folder in your app.

This is discussed in the following thread http://groups.google.com/group/django-users/browse_thread/thread/70385d35ec43a6e3/

Note: this is not limited to recent trunk versions - I am running 1.1.1 and it manifests there too.

Attachments (2)

12658.diff (814 bytes) - added by Matthew Schinckel 7 years ago.
Fix for syntax/import errors causing tests to be skipped.
patch+tests.diff (5.6 KB) - added by Malcolm Box 6 years ago.
Patch and testcase for issue, against 1.3

Download all attachments as: .zip

Change History (17)

comment:1 Changed 7 years ago by Matthew Schinckel

More information: it only occurs if both the tests file and the models file have been split into folders. This is due to the line in django.test.simple.get_tests that tries again to find the module:

mod = find_module(TEST_MODULE, [os.path.dirname(app_module.__file__)])

At this point, if the models file has been split, then app_module.__file__ is the __init__.py file in the models directory, so os.path.dirname() of this gets the model directory, not the app directory. Thus, no tests module can be found (unless there was one in the models directory).

A really hacky solution is to capture the internal exception, and compare the arguments of the two exceptions. If they match, then it is the tests module that is missing. If it isn't then the problem is an import. I'm sure there is a much nicer solution, though. I'm not really even game to submit a patch for mine!

Last edited 6 years ago by Ramiro Morales (previous) (diff)

comment:2 Changed 7 years ago by Matthew Schinckel

Summary: Import Errors in test files cause tests to be split.Import Errors in test files cause tests to be skipped.

comment:3 Changed 7 years ago by Russell Keith-Magee

Resolution: duplicate
Status: newclosed

I think this is a duplicate of #12574. Please reopen if I've misunderstood the problem you are describing.

comment:4 Changed 7 years ago by Karen Tracey

They sound similar but I don't think they are the same problem. #12574 reports a problem running Django's own tests, this one is reporting a problem running an application's tests. Unless the patch for #12574 is wrong (it changes only tests/runtests.py and does not touch the code in test/simple.py mentioned above) I don't see how this one can be a dupe of that one.

comment:5 Changed 7 years ago by Matthew Schinckel

Resolution: duplicate
Status: closedreopened

Yeah, since the patch in #12574 does not affect anything inside the django/ directory, only tests/, then it cannot fix my issue.

Which, as kmtracey stated, is running tests on my own apps, not on the django framework.

Changed 7 years ago by Matthew Schinckel

Attachment: 12658.diff added

Fix for syntax/import errors causing tests to be skipped.

comment:6 Changed 7 years ago by Matthew Schinckel

Has patch: set

comment:7 Changed 7 years ago by Eric Holscher

Needs tests: set
Triage Stage: UnreviewedAccepted

Seems like this should have a test case to prove that it is indeed working.

comment:8 Changed 6 years ago by Russell Keith-Magee

I agree with Eric that this needs a test case. The test case doesn't need to be of the full test suite; just that the get_tests() method works. The model_package modeltest provides a set of test models that could be used.

I would also say that the "check that the module exists" logic should be replace with django.utils.module_loading.module_has_submodule(); this utility method was introduced in 1.2 to perform exactly the check that is being performed here, but in a more rigorous fashion.

Changed 6 years ago by Malcolm Box

Attachment: patch+tests.diff added

Patch and testcase for issue, against 1.3

comment:9 Changed 6 years ago by Malcolm Box

Needs tests: unset

I have created a testcase and updated the patch as requested to use module_has_submodule(). This patch was tested on 1.3 but should apply back to 1.2.

The error is also present in 1.1, but the module_has_submodule utility function isn't.

comment:10 Changed 6 years ago by Malcolm Box

Cc: Malcolm Box added

comment:11 Changed 6 years ago by Matt McClanahan

Severity: Normal
Type: Bug

comment:12 Changed 6 years ago by patchhammer

Easy pickings: unset
Patch needs improvement: set

patch%2Btests.diff fails to apply cleanly on to trunk

comment:13 Changed 6 years ago by Ramiro Morales

Summary: Import Errors in test files cause tests to be skipped.Import Errors in test files cause tests to be skipped if both the tests file and the models file have been split into folders
UI/UX: unset

comment:14 Changed 6 years ago by Ramiro Morales

#16217 was a duplicate.

comment:15 Changed 5 years ago by Ramiro Morales

Resolution: fixed
Status: reopenedclosed

In [16382]:

Fixed #12658 -- Fixed test discovery so ImportErrors aren't ignored when both tests and models are packages. Thanks schinckel for the report and boxm for a patch for the issue.

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