#20449 closed Bug (fixed)

New test runner test fails if run from a different directory

Reported by: gcc Owned by: tadeck
Component: Testing framework Version: master
Severity: Normal Keywords:
Cc: marc.tamlyn@…, tadeck Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

======================================================================
ERROR: test_file_path (test_runner.test_discover_runner.DiscoverRunnerTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/travis/build/aptivate/django/tests/test_runner/test_discover_runner.py", line 65, in test_file_path
  ["test_discovery_sample/"],
File "/home/travis/build/aptivate/django/django/test/runner.py", line 63, in build_suite
  tests = self.test_loader.loadTestsFromName(label)
File "/usr/lib/python2.7/unittest/loader.py", line 91, in loadTestsFromName
  module = __import__('.'.join(parts_copy))
ImportError: Import by filename is not supported.

<https://next.travis-ci.org/aptivate/django/jobs/7275360>

It seems to be checking if a file exists, using a relative path, in django/test/runner.py:65:

    if not os.path.exists(label_as_path):
        tests = self.test_loader.loadTestsFromName(label)

And that will behave differently depending if you run your tests from inside the tests directory like this:

./runtests.py -v2 --settings=test_postgres_nogis test_runner

Or from the parent directory, as Travis currently does, like this:

    python -Wall tests/runtests.py --selenium --verbosity 2 \
        --settings=django_settings

I can work around it in Travis by changing working directory before running the tests, but is it worth fixing this in the test runner, perhaps using an absolute path based on __file__?

Carl Meyer wrote:

I don't think this should be fixed in the test runner itself; in general, file-path test labels _should_ be interpreted as relative to wherever you are running the tests from.


But it should be fixed in the test_runner.test_discover_runner.DiscoverRunnerTest.test_file_path test - that test apparently needs to isolate itself better by setting the CWD for the duration of the test, or something similar. Mind filing a bug? I should be able to take a look soon.

So here it is :)

Attachments (2)

1153.diff (1.2 KB) - added by tadeck 23 months ago.
DIFF for ticket #20449, from pull request no. 1153 (https://github.com/django/django/pull/1153)
1153.patch (1.5 KB) - added by tadeck 23 months ago.
PATCH for ticket #20449, from pull request no. 1153 (https://github.com/django/django/pull/1153)

Download all attachments as: .zip

Change History (11)

comment:1 Changed 23 months ago by gcc

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Version changed from 1.4 to master

comment:2 Changed 23 months ago by mjtamlyn

  • Cc marc.tamlyn@… added
  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 23 months ago by tadeck

  • Owner changed from nobody to tadeck
  • Status changed from new to assigned
  • Type changed from Uncategorized to Bug

Changed 23 months ago by tadeck

DIFF for ticket #20449, from pull request no. 1153 (https://github.com/django/django/pull/1153)

Changed 23 months ago by tadeck

PATCH for ticket #20449, from pull request no. 1153 (https://github.com/django/django/pull/1153)

comment:4 Changed 23 months ago by tadeck

  • Cc tadeck added
  • Has patch set
  • Resolution set to fixed
  • Status changed from assigned to closed

Pull request waiting for review: https://github.com/django/django/pull/1153

It uses context manager to temporarily change current working directory to one level up, then switches it back to what it was.

comment:5 follow-up: Changed 23 months ago by mjtamlyn

  • Resolution fixed deleted
  • Status changed from closed to new

Tickets are closed when the code is merged, not when the pull request is created.

comment:6 in reply to: ↑ 5 Changed 23 months ago by tadeck

Replying to mjtamlyn:

Tickets are closed when the code is merged, not when the pull request is created.

I am sorry, that was not intentional. Missed note on https://docs.djangoproject.com/en/dev/internals/contributing/triaging-tickets/#closing-tickets (and marking ticket as fixed resulted in closing it).

comment:7 Changed 23 months ago by Carl Meyer <carl@…>

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

In 022de7e1393541a3693919e1427d272df89f7f46:

Fixed #20449 - Corrected test sensitivity to current working dir.

comment:8 Changed 11 months ago by nikl@…

  • Resolution fixed deleted
  • Status changed from closed to new

Fixed in: https://github.com/django/django/pull/2657

This also happens to the (newer) test_testcase_ordering() with the current master:

tests/runtests.py test_runner
Testing against Django installed in '/home/dev/django/django/django'
Creating test database for alias 'default'...
Creating test database for alias 'other'...
.............E.ss.................
======================================================================
ERROR: test_testcase_ordering (test_runner.test_discover_runner.DiscoverRunnerTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/dev/django/django/tests/test_runner/test_discover_runner.py", line 107, in test_testcase_ordering
    suite = DiscoverRunner().build_suite(["test_discovery_sample/"])
  File "/home/dev/django/django/django/test/runner.py", line 66, in build_suite
    tests = self.test_loader.loadTestsFromName(label)
  File "/usr/lib/python2.7/unittest/loader.py", line 91, in loadTestsFromName
    module = __import__('.'.join(parts_copy))
ImportError: Import by filename is not supported.

----------------------------------------------------------------------
Ran 34 tests in 1.811s

FAILED (errors=1, skipped=2)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...

which can be fixed by applying the already existing with change_cwd(".."): pattern - maybe this might be moved to setUp()?

comment:9 Changed 11 months ago by mjtamlyn

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