Opened 13 months ago

Closed 9 months ago

#22068 closed Bug (fixed)

Trailing slash after some test suites leads to test failure

Reported by: MattBlack Owned by: akshay1994.leo
Component: Testing framework Version: master
Severity: Normal Keywords:
Cc: akshay1994.leo Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This happend yesterday and today, running:

PYTHONPATH=..:$PYTHONPATH python2 ./runtests.py --settings=test_sqlite test_utils

Testing against Django installed in '/home/mattblack/10.12.216.104/django'
Creating test database for alias 'default'...
Creating test database for alias 'other'...
...................................s............
----------------------------------------------------------------------
Ran 48 tests in 0.160s

OK (skipped=1)

test is successful


PYTHONPATH=..:$PYTHONPATH python2 ./runtests.py --settings=test_sqlite test_utils/

https://dpaste.de/r5uu to see the traceback, it's just huge to be pasted here (and it's not the full one)

fails


same for util_test suite. The problem is reproducible in python 2 and 3.

Change History (13)

comment:1 Changed 13 months ago by MattBlack

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

I pasted the traceback in a gist too, here the link https://gist.github.com/MattBlack85/9038717

comment:2 Changed 13 months ago by anonymous

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 13 months ago by akshay1994.leo

  • Cc akshay1994.leo added
  • Owner set to akshay1994.leo
  • Status changed from new to assigned

comment:4 Changed 13 months ago by akshay1994.leo

  • Has patch set

I've written a patch for this, and created a pull request ( https://github.com/django/django/pull/2314 ).
Do I need to submit a test case for this?

comment:5 Changed 13 months ago by timo

  • Resolution set to wontfix
  • Status changed from assigned to closed

I'm not convinced this is needed, and in fact, it may cause confusion. The argument "test_utils" is an app_label not a file system path. If you want to run a subset of those tests, you would use test_utils.tests.AssertQuerysetEqualTests. Please reopen if I've missed something.

comment:6 Changed 12 months ago by akshay1994.leo

  • Resolution wontfix deleted
  • Status changed from closed to new

Correct me if I am wrong, but running tests from project's manage.py allows test_labels and also allows supplying paths to a directory to discover test-cases.
( https://docs.djangoproject.com/en/dev/topics/testing/overview/#running-tests ).
Shouldn't this also work with the same behaviour.

comment:7 Changed 9 months ago by aaugustin

Indeed, the docs even show an example with a trailing slash.

comment:8 Changed 9 months ago by timo

I don't know if runtests.py should have the same behavior. For example, ./runtests.py ../django/contrib/admin/ doesn't discover any tests. The slash method isn't documented in running some of Django's tests.

comment:9 Changed 9 months ago by prestontimmons

The problem here is that if a trailing slash is supplied, like with ./runtests.py test_utils/, the setup in runtests.py won't recognize the path as a module, hence it won't add the module to INSTALLED_APPS. The discover runner will still do discovery in the folder, though. This is what causes the test failures.

Tim, your example for ./runtests.py ../django/contrib/admin/ isn't quite correct since there aren't any tests in that directory. Try ./runtests.py ../django/contrib/sitemaps and you'll see the problem.

I think the easiest way to run into this problem is when using tab completion. Typing ./runtests.py test_u<tab> appends the trailing slash, which then has to be removed for the tests to run correctly.

The supplied patch looks like it simply ignores the trailing slash for the common case of it being added with tab completion. This could be helpful because the test failures don't tell you that the trailing slash caused INSTALLED_APPS to be set up incorrectly.

There would still be failures with a path to the contrib folder, but I don't think people run into that very often.

comment:10 Changed 9 months ago by timo

Oh hm, well django/contrib/admin does have a tests.py. I thought it should/would be discovered?

Preston, are you +1 on the patch then?

comment:11 Changed 9 months ago by prestontimmons

Oops. You're right. There is a tests.py in django/contrib/admin. The unittest discovery doesn't find tests in it, though.

This is because it only has one test case based on StaticLiveServerCase which doesn't specify any test_ methods.

comment:12 Changed 9 months ago by prestontimmons

The idea here looks fine to me. The patch no longer works since the update from optparse to argparse.

Here's an updated one that does the same thing:

https://github.com/django/django/pull/2795

comment:13 Changed 9 months ago by Tim Graham <timograham@…>

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

In 68efbfde5e35013c14c0c30cabb1c94b80a48807:

Fixed #22068 -- Made runtests.py remove trailing slashes from test labels.

When using tab-completion it's easy to accidentally run a test with
a trailing slash, which causes INSTALLED_APPS to be set incorrectly.
Normalize the test labels to avoid this common error.

Thanks MattBlack for the suggestion.

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