#13173 closed (fixed)
entire admin_scripts test suite fails on Windows if sys.executable path has a space in it.
Reported by: | Gabriel Hurley | Owned by: | Gabriel Hurley |
---|---|---|---|
Component: | Core (Management commands) | Version: | 1.1 |
Severity: | Keywords: | admin_scripts | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
This is an edge case to be sure, but it sure is annoying if you have Python installed somewhere funny...
If you have Python installed on Windows to a location like "C:\Program Files\python2.6\" then every test in regressiontests.admin_scripts will fail. This is due to the (known) poor escaping done by Popen
.
Simply quoting the sys.executable path isn't enough, because if Popen
finds a quote at the beginning of
args
, it strips that quote and automatically assumes it corresponds to a quote at the end of
args
and will fail with mismatched quotes without realizing what it's done.
The most common solution I could find is to double-quote the args
string if you need it to properly handle the extra spaces.
The attached patch currently passes the tests with Python 2.6.4 on Ubuntu and Python 2.6.5 on Windows (both with and without spaces in the sys.executable
path).
It's not an incredibly urgent bug, but I figured I'd add a fix here.
Attachments (1)
Change History (9)
comment:1 by , 15 years ago
milestone: | → 1.2 |
---|---|
Triage Stage: | Unreviewed → Ready for checkin |
comment:2 by , 15 years ago
Triage Stage: | Ready for checkin → Accepted |
---|
With the patch applied, I cannot get the admin_scripts tests to pass if sys.executable does NOT have a space in it. For example:
C:\u\kmt\django\trunk\tests>\bin\Python27\python.exe runtests.py --settings=testdb.sqlite -v1
produces failures like:
====================================================================== FAIL: test_builtin_command (regressiontests.admin_scripts.tests.DjangoAdminAlternateSettings) alternate: django-admin builtin commands fail with an import error when no settings provided ---------------------------------------------------------------------- Traceback (most recent call last): File "C:\u\kmt\django\trunk\tests\regressiontests\admin_scripts\tests.py", line 400, in test_builtin_command self.assertOutput(err, 'environment variable DJANGO_SETTINGS_MODULE is undefined') File "C:\u\kmt\django\trunk\tests\regressiontests\admin_scripts\tests.py", line 153, in assertOutput self.failUnless(msg in stream, "'%s' does not match actual output text '%s'" % (msg, stream)) AssertionError: 'environment variable DJANGO_SETTINGS_MODULE is undefined' does not match actual output text 'The filename, directory name, or volume label syntax is incorrect. '
comment:3 by , 15 years ago
Patch needs improvement: | set |
---|
comment:4 by , 15 years ago
I'm gonna have to take this one back to the drawing board as it seems to be *very* finicky. Quoting for the shell across systems isn't exactly a simple problem. I thought I'd found one that worked since it was happy on the systems I had access to, but apparently not. :-/
comment:5 by , 15 years ago
Forgot to mention the box with the failures up above is Windows XP. That's the only Windows version I have.
comment:6 by , 15 years ago
Patch needs improvement: | unset |
---|---|
Status: | new → assigned |
I was able to reproduce the failure noted by Karen above. I've got a new patch here which is a bit smarter. I've tried it on Windows XP, Ubuntu, and Mac OS X with success so far, so...
Give this one a go and see how it does. It's a massive pain testing all the different system configurations with this thing (since it involves moving Python installs on my machines). I think it's good, but getting a few more testers on it would be really good.
by , 15 years ago
Attachment: | admin_scripts_quoting.diff added |
---|
Patch includes smarter quoting to account for spaces in executable path and Popen irregularities.
comment:7 by , 15 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Bug that causes test suite failures is important; especially when there is a RFC patch.