Opened 11 years ago
Closed 10 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: | dev |
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 by , 11 years ago
comment:2 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:3 by , 11 years ago
Cc: | added |
---|---|
Owner: | set to |
Status: | new → assigned |
comment:4 by , 11 years ago
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 by , 11 years ago
Resolution: | → wontfix |
---|---|
Status: | assigned → 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 by , 11 years ago
Resolution: | wontfix |
---|---|
Status: | closed → 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:8 by , 10 years ago
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 by , 10 years ago
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 by , 10 years ago
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 by , 10 years ago
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 by , 10 years ago
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:
comment:13 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
I pasted the traceback in a gist too, here the link https://gist.github.com/MattBlack85/9038717