Trailing slash after some test suites leads to test failure

[MattBlack]

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.

[MattBlack]

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

[akshay1994.leo]

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?

[Tim Graham]

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.

[akshay1994.leo]

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.

[Aymeric Augustin]

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

[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.

[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.

[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?

[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.

[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

[Tim Graham <timograham@…>]

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.