Opened 4 years ago

Closed 3 years ago

#19941 closed Cleanup/optimization (fixed)

Make running django's test suite easier

Reported by: Anssi Kääriäinen Owned by: nobody
Component: Testing framework Version: master
Severity: Normal Keywords:
Cc: timograham@…, Florian Apolloner Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Two suggested changes to tests/runtests.py:

  1. Automatically use tests/../django as the Django version.
  2. If no other settings given (either through --settings or DJANGO_SETTINGS_MODULE) then use test_sqlite as settings

Both of the above changes are available from: https://github.com/akaariai/django/compare/test_auto_settings

The first item raises a question: is there some use case where one wants to run tests against *different* django version? To me it seems one wouldn't want to do this, the tests are tied to specific django version.

I can't see any drawback by doing the second item.

Change History (14)

comment:1 Changed 4 years ago by Florian Apolloner

Triage Stage: UnreviewedAccepted

I fully agree with 1. and 2. -- I am against supporting other Django versions.

comment:2 Changed 4 years ago by Aymeric Augustin

Triage Stage: AcceptedReady for checkin

LGTM, with the caveat that the long term goal is to get rid of runtests.py, not extend it...

comment:3 Changed 4 years ago by Aymeric Augustin

Needs documentation: set
Triage Stage: Ready for checkinAccepted

Not RFC yet in fact :)

comment:4 Changed 3 years ago by Tim Graham

Cc: timograham@… added
Needs documentation: unset

PR with update docs.

comment:5 Changed 3 years ago by loic84

Triage Stage: AcceptedReady for checkin

Looks good to me.

comment:6 Changed 3 years ago by Tim Graham

I'm unsure why import django is needed? Also the comment in the docstring of upath references a "try-except of import django" which isn't present.

comment:7 Changed 3 years ago by loic84

I spotted the import django and I assumed it was meant as an assert.

In light of the docstring, maybe the idea was to provide a helpful error message?

comment:8 Changed 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In b2314d9e1e08749f2c05151f9cd44520d2b3a03a:

Fixed #19941 -- Modified runtests.py to make running the tests easier.

  1. Automatically use tests/../django as the Django version.
  2. If settings aren't provided through --settings or DJANGO_SETTINGS_MODULE) then use test_sqlite.

comment:9 Changed 3 years ago by Jannis Leidel

Resolution: fixed
Status: closednew

Reopening the sys.path.insert call makes the test setup differ from standard installation procedures. We've for a long time tried hard to move away from modifying the sys.path.

As a background, by mangling the sys.path we risk missing import errors that happen when Django is installed with tools like pip. Instead we should promote the use of pip install -e path/to/django when developing Django.

Last edited 3 years ago by Jannis Leidel (previous) (diff)

comment:10 Changed 3 years ago by Florian Apolloner

After discussing this a bit more with jezdez in IRC we came to the following conclusion:

  • pip install -e . isn't perfect (in the sense that it doesn't mimic a sdist install by 100%) but still better than sys.path hacks.
  • drop sys.path hacks again.
  • Update the documentation to note pip install -e and PYTHONPATH=.. alternatives.
  • Output something like print("Testing against Django installed in '%s'" % django.__file__) to the console, to allow easier debugging.

comment:11 Changed 3 years ago by Florian Apolloner

Cc: Florian Apolloner added

comment:12 Changed 3 years ago by Anssi Kääriäinen

Has patch: unset
Triage Stage: Ready for checkinAccepted

comment:13 Changed 3 years ago by Tim Graham

Has patch: set

Pull request with changes proposed above.

comment:14 Changed 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In c573d6de173c0e6743473081cde786e749641a48:

Fixed #19941 -- Removed sys.path hack when running the test suite.

Thanks jezdez for the suggestion.

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