Opened 3 years ago

Closed 23 months ago

#19941 closed Cleanup/optimization (fixed)

Make running django's test suite easier

Reported by: akaariai Owned by: nobody
Component: Testing framework Version: master
Severity: Normal Keywords:
Cc: timograham@…, apollo13 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 3 years ago by apollo13

  • Triage Stage changed from Unreviewed to Accepted

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

comment:2 Changed 3 years ago by aaugustin

  • Triage Stage changed from Accepted to Ready for checkin

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

comment:3 Changed 3 years ago by aaugustin

  • Needs documentation set
  • Triage Stage changed from Ready for checkin to Accepted

Not RFC yet in fact :)

comment:4 Changed 2 years ago by timo

  • Cc timograham@… added
  • Needs documentation unset

PR with update docs.

comment:5 Changed 2 years ago by loic84

  • Triage Stage changed from Accepted to Ready for checkin

Looks good to me.

comment:6 Changed 2 years ago by timo

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 2 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 2 years ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from new to closed

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 2 years ago by jezdez

  • Resolution fixed deleted
  • Status changed from closed to new

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 2 years ago by jezdez (previous) (diff)

comment:10 Changed 2 years ago by apollo13

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 2 years ago by apollo13

  • Cc apollo13 added

comment:12 Changed 2 years ago by akaariai

  • Has patch unset
  • Triage Stage changed from Ready for checkin to Accepted

comment:13 Changed 23 months ago by timo

  • Has patch set

Pull request with changes proposed above.

comment:14 Changed 23 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from new to closed

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