Opened 13 years ago
Closed 13 years ago
#16342 closed Bug (duplicate)
Django unittests run against installed django by default (rather than the checkout)
Reported by: | Soren Hansen | Owned by: | nobody |
---|---|---|---|
Component: | Testing framework | Version: | 1.3 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
By default, tests/runtests.py will import various things from "django". This means that if you're lucky, you don't have a Django install in your standard PYTHONPATH and you'll get an error, realise the error of your ways, and you can amend your PYTHONPATH. If you're less lucky, runtests.py will happily import the *installed* django, and you will get very, very confused why tests are passing even though you know you broke something.
Attachments (1)
Change History (7)
by , 13 years ago
Attachment: | set_the_pythonpath.patch added |
---|
comment:1 by , 13 years ago
Has patch: | set |
---|
Attached a patch to fix it. I've no clue how to test this (or whether to do so at all, really). Feedback happily accepted.
comment:2 by , 13 years ago
Summary: | Django unittests run against install django by default (rather than the checkout) → Django unittests run against installed django by default (rather than the checkout) |
---|
comment:3 by , 13 years ago
I was just told about ticket #9947, which refers to the same issue.
I'd like to attack this problem somehow. I'm not familiar with the bug management rules here. If it's more appropriate to reopen the old one and ditch this one, that's fine. Feel free to close this.
I can vaguely relate to the desire to run new tests against old code.
For those of you who actually make use of this, am I correct to assume that this is less common than running new tests against new code? I mean, it seems more natural to me that development of both "real" code and tests happens in the code checkout, but I've been surprised before :)
Would everyone's needs be met if the default was to prepend the toplevel dir to sys.path, but there was a simple way to disable this behaviour (like setting an environment variable or something)?
comment:4 by , 13 years ago
Patch needs improvement: | set |
---|
An alternative is to implement a warning when the tests are run against a version of Django that is not the local checkout. That might be acceptable by the core developers.
Technically, your ticket is a duplicate of another ticket that was closed by a core developer as wontfix. The standard procedure would be to close it and tell you to start a discussion on django-developers.
However, I think it's something that can easily turn off new contributors, so we should try to help them a little bit without hindering the workflow of the core developers. I'll try to get some feedback, and I don't close the ticket right now.
comment:5 by , 13 years ago
I agree that this turns off new developers, but it's an absolute non-issue for the more experienced devs. To the best of my knowledge, everyone who does much work on the codebase uses virtualenv or directly modifies their path variable to isolate their environment. This means that there isn't a system install of Django at all, and there's absolutely no chance that there is more than one valid "django" to import.
We should really be teaching new users to use virtualenv at the end of the tutorial. If we can't get it there, we should at least include it in the documentation on writing tests or working on Django.
Mucking around with the path in the test suite is likely to be a nonstarter. It's much better to let Python do the imports as expected, and require users to modify the environment themselves where necessary.
comment:6 by , 13 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
@Soren - the bug management rules here are that if you have found a duplicate, you close the new ticket as a duplicate. Jacob's comment on closing #9947 still apply, so I don't see any action required here.
Patch to always prepend the top level srcdir to sys.path.