Code

Opened 4 years ago

Closed 3 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: schinckel Owned by: nobody
Component: Testing framework Version: 1.1
Severity: Normal Keywords:
Cc: boxm 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 schinckel 4 years ago.
Fix for syntax/import errors causing tests to be skipped.
patch+tests.diff (5.6 KB) - added by boxm 3 years ago.
Patch and testcase for issue, against 1.3

Download all attachments as: .zip

Change History (17)

comment:1 Changed 4 years ago by schinckel

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 3 years ago by ramiro (previous) (diff)

comment:2 Changed 4 years ago by schinckel

  • Summary changed from Import Errors in test files cause tests to be split. to Import Errors in test files cause tests to be skipped.

comment:3 Changed 4 years ago by russellm

  • Resolution set to duplicate
  • Status changed from new to closed

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

comment:4 Changed 4 years ago by kmtracey

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 4 years ago by schinckel

  • Resolution duplicate deleted
  • Status changed from closed to reopened

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 4 years ago by schinckel

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

comment:6 Changed 4 years ago by schinckel

  • Has patch set

comment:7 Changed 4 years ago by ericholscher

  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted

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

comment:8 Changed 4 years ago by russellm

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 3 years ago by boxm

Patch and testcase for issue, against 1.3

comment:9 Changed 3 years ago by boxm

  • 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 3 years ago by boxm

  • Cc boxm added

comment:11 Changed 3 years ago by mattmcc

  • Severity set to Normal
  • Type set to Bug

comment:12 Changed 3 years ago by patchhammer

  • Easy pickings unset
  • Patch needs improvement set

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

comment:13 Changed 3 years ago by ramiro

  • Summary changed from Import Errors in test files cause tests to be skipped. to 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 3 years ago by ramiro

#16217 was a duplicate.

comment:15 Changed 3 years ago by ramiro

  • Resolution set to fixed
  • Status changed from reopened to closed

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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.