Opened 9 years ago

Closed 6 years ago

#25941 closed Cleanup/optimization (fixed)

Improve the error message for runtests.py when Django isn't on the path

Reported by: Keryn Knight Owned by: Thomas Allison
Component: Testing framework Version: dev
Severity: Normal Keywords:
Cc: django@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Anton Samarchyan)

Every time I think about contributing, literally the first thing I want to do is do a git clone and run the tests.
This is currently annoying, because it requires (according to the documentation), my either adding the newly cloned directory to my path permanently (and knowing how to do that. I do, but it raises the barrier to entry for others) or having cloned it via pip which intrinsically puts it on the path and smashes it into a src directory.

Instead, I have to go and look in the documentation (which is squirrelled away via Documentation -> How to get involved -> Write unit tests) and find PYTHONPATH=..:$PYTHONPATH ./runtests.py because there's literally no chance I'm going to remember the specifics of that every time I sporadically want to get involved (questions in could go through my mind: do I need to put a semi-colon in? Do I need to export it? do I need to provide a specific settings file?)

Executing ./runtests.py without the incantation helpfully just errors with:

Traceback (most recent call last):
  File "./runtests.py", line 13, in <module>
    import django
ImportError: No module named django

or if you're in a virtualenv with an older extant copy of Django (by way of example) ...

Traceback (most recent call last):
  File "./runtests.py", line 18, in <module>
    from django.test.runner import default_test_processes
ImportError: cannot import name 'default_test_processes'

rather than let me know how to get back on the straight and narrow.

The barrier to entry is too high. If runtests.py cannot execute, a better error message would be useful. Ostensibly something like the following would probably work, for the fresh clone scenario (but not the already-installed-on-path one):

try:
    import django
except ImportError:
    sys.stdout.write("Some error message explaining how to run the thing\n")
    sys.exit(1)

Really though, a single entry point ./runtests.sh which passes through arguments ($@ I think?) that might want to be changed would make life easier from both a user and documentation standpoint ("just run the shell script in the tests directory")

(Additional note: there is a README.rst file in the tests directory, but it a) says to cd to a directory which does not exist if you're already in the directory [which you probably are to run the tests the documented way] and b) requires installing requirements files that the quickstart doesn't)

Change History (11)

comment:1 by Keryn Knight, 9 years ago

A word of clarification, in the original ticket I said having cloned it via git which intrinsically puts it on the path and smashes it into a src directory where I actually meant pip, rather than git.

Version 0, edited 9 years ago by Keryn Knight (next)

comment:2 by Moritz Sichert, 9 years ago

Wouldn't the recommended workflow be more like the following?

. ./my/test/venv/bin/activate
git clone url/to/django.git
cd django
pip install -e .
cd tests
pip install -r requirments/py3.txt
./runtests.py

If you do it like that you won't have to fiddle with PYTHONPATH and you also won't run into trouble with having installed multiple possibly older versions of Django.

comment:3 by Tim Graham, 9 years ago

Summary: Supply a runtests.sh in the tests directory.Improve the error message for runtests.py when Django isn't on the path
Triage Stage: UnreviewedAccepted
Type: New featureCleanup/optimization

I'm not so enthusiastic about providing a shell script, however, I wouldn't object to a better error message for runtests.py for the "import django" ImportError. It could point at tests/README.rst and that file could be updated with the workflow suggested by Moritz.

comment:4 by Keryn Knight, 9 years ago

The suggested workflow is not the same though, is it? The PYTHONPATH twiddling, though a pain to remember, isn't installing django into your environment, it's just adjusting it temporarily (for the test run). This is a marked difference (because actually, the invocation can just be PYTHONPATH=.. ./runtests.py --parallel=N for the simple case within the tests dir, AFAIK). I'm also unconvinced why I need to install it at all - the available documentation (online and in the README.rst) already demonstrates it is unnecessary to do so. And I rather presume that the quickstart wasn't arrived at by choosing the least memorable way of executing the tests.

in reply to:  4 ; comment:5 by Moritz Sichert, 9 years ago

Replying to kezabelle:

because actually, the invocation can just be PYTHONPATH=.. ./runtests.py --parallel=N for the simple case within the tests dir, AFAIK

That's not exactly true. You can't run a large number of tests if you didn't pip install -r tests/requirements/py3.txt before. That means you always have to "initialize" your environment (beyond cloning the git repo) to be able to run most of the tests.

in reply to:  5 comment:6 by Keryn Knight, 9 years ago

Of that I'm aware; I was demonstrating that even referencing any prior $PYTHONPATH is not actually a requirement (again, AFAIK, and again, I'm sure there are reasons the quick-start includes it); and I think the runtests script actually prevents any tests from running without requirements having been installed (and tells you what to do!)

comment:7 by Anton Samarchyan, 8 years ago

Description: modified (diff)

comment:8 by Thomas Allison, 6 years ago

Owner: changed from nobody to Thomas Allison
Status: newassigned

comment:9 by Thomas Allison, 6 years ago

Has patch: set

comment:10 by Mariusz Felisiak, 6 years ago

Triage Stage: AcceptedReady for checkin

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 6 years ago

Resolution: fixed
Status: assignedclosed

In 3e8b7333:

Fixed #25941 -- Improved error message for runtests.py when django is not on path.

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