Opened 3 years ago

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

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

comment:2 Changed 3 years ago by anonymous

Triage Stage: UnreviewedAccepted

comment:3 Changed 3 years ago by akshay1994.leo

Cc: akshay1994.leo added
Owner: set to akshay1994.leo
Status: newassigned

comment:4 Changed 3 years 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 3 years ago by Tim Graham

Resolution: wontfix
Status: assignedclosed

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 3 years ago by akshay1994.leo

Resolution: wontfix
Status: closednew

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

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

comment:8 Changed 3 years ago by Tim Graham

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

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

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

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

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 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

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