Opened 5 years ago

Closed 5 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


By default, tests/ 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, 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)

set_the_pythonpath.patch (484 bytes) - added by Soren Hansen 5 years ago.
Patch to always prepend the top level srcdir to sys.path.

Download all attachments as: .zip

Change History (7)

Changed 5 years ago by Soren Hansen

Attachment: set_the_pythonpath.patch added

Patch to always prepend the top level srcdir to sys.path.

comment:1 Changed 5 years ago by Soren Hansen

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 Changed 5 years ago by Soren Hansen

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 Changed 5 years ago by Soren Hansen

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

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 Changed 5 years ago by Paul McMillan

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.

Last edited 5 years ago by Paul McMillan (previous) (diff)

comment:6 Changed 5 years ago by Russell Keith-Magee

Resolution: duplicate
Status: newclosed

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

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