#22448 closed Bug (fixed)

django test command runs wrong tests if test module has no tests

Reported by: cjerdonek Owned by: nobody
Component: Testing framework Version: 1.6
Severity: Normal Keywords: test discovery, test runner, test command, test label
Cc: chris.jerdonek@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The Django test command seems to behave incorrectly if passed a test module that has no tests.

For example, if passed test label foo.bar corresponding to module foo.bar and foo.bar has no tests, the command seems to discover and run all tests in foo.*, which is more than it should.

This behavior made it much harder to troubleshoot the fact that one of my test modules mistakenly had no tests. If given a module with no tests, Django should report back with a message like "0 tests found in ---" or simply run no tests.

Change History (11)

comment:1 Changed 11 months ago by cjerdonek

  • Cc chris.jerdonek@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 11 months ago by prestontimmons

Hi Chris,

The behavior you describe is a bug in how Python's unittest discovery works, but it isn't an issue with Django's test runner than I'm aware of.

There's even a test to ensure that:

https://github.com/django/django/blob/master/tests/test_runner/test_discover_runner.py#L92

Are you using a custom test runner by any chance?

I can't reproduce the problem in my own testing. If you can give me a case where it discovers incorrectly, I'll take another look.

comment:3 Changed 11 months ago by cjerdonek

Thanks for the response. No, I'm not using a custom test runner. But it looks like this was fixed in 1.7? I'm using 1.6 as the bug report indicates. And indeed, I don't have the fix when I manually checked the Django code I'm using.

comment:4 Changed 11 months ago by prestontimmons

Although we added the test case in the 1.7 cycle, this wasn't a bug in 1.6 that I'm aware of. That's the version I'm using to try to reproduce what you're seeing.

I'm using the following layout to test:

foo/
  __init__.py
  bar.py
  car.py

The bar.py file is empty and car.py has:

import unittest

from django.test import TestCase


class SampleTest(unittest.TestCase):

    def test_one(self):
        assert True


class DjangoTest(TestCase):

    def test_one(self):
        assert True

Running python manage.py test foo.bar returns:

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK

Is this a correct interpretation of the scenario your describing?

comment:5 Changed 11 months ago by cjerdonek

Thanks for putting the test case together! I ran it and got 0 tests, too. But then I realized it's not discovering tests because the filenames don't match the default test pattern. When I renamed the files to test_bar.py and test_car.py, I was able to reproduce the problem. (Sorry, I probably should have said foo.test_bar in my original post above.)

Last edited 11 months ago by cjerdonek (previous) (diff)

comment:6 Changed 11 months ago by prestontimmons

  • Triage Stage changed from Unreviewed to Accepted

Yes, I can reproduce this now. That explains why the existing test reports back without error as well.

comment:7 Changed 11 months ago by prestontimmons

After some further testing I realized this is fixed in 1.7. That's because we now only run discovery if the test label is a module or directory.

comment:8 Changed 11 months ago by cjerdonek

So just to clarify, what changes need to be made in what versions? From the comments, it seems the test needs to be fixed in 1.7 so that it has a chance of failing, and the bug can be fixed in versions prior to 1.7.

(By the way, I don't quite understand the explanation for why it's working in 1.7. In the test case, the test label is a module, which you say will cause discovery to be run. But you also said that Python's unittest discovery is what has the bug?)

Last edited 11 months ago by cjerdonek (previous) (diff)

comment:9 Changed 11 months ago by carljm

I don't think the severity of this bug justifies backporting, so the relevant question is what still needs fixing in 1.7.

comment:10 Changed 11 months ago by prestontimmons

Sorry for not being clear. I meant package, not module.

No change is needed for 1.7. The behavior described was fixed at the same time as #21206.

When the test runner is given a label, two things happen.

1) Tests are first loaded using unittest loadTestsFromName
2) If none are found, and the path is a package or folder, unittest discovery is run.

Before #21206 we didn't check if a path was a package or folder. That's what exposed the buggy unittest discovery behavior.

comment:11 Changed 11 months ago by prestontimmons

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.
Back to Top