Closed
Bug 839340
Opened 11 years ago
Closed 11 years ago
"mach mochitest-* $DIR" doesn't work on Windows (os.path.sep problem)
Categories
(Firefox Build System :: Mach Core, enhancement)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla21
People
(Reporter: mbrubeck, Assigned: mbrubeck)
Details
Attachments
(2 files, 2 obsolete files)
3.34 KB,
patch
|
gps
:
review+
|
Details | Diff | Splinter Review |
1.60 KB,
patch
|
gps
:
review+
|
Details | Diff | Splinter Review |
This works: pymake -C $OBJDIR mochitest-plain toolkit/components/passwordmgr/test This fails: ./mach mochitest-plain toolkit/components/passwordmgr/test Part of the problem is this line, which adds a backslash (\) to the end of the path on Windows: http://hg.mozilla.org/mozilla-central/file/a46bc920998d/testing/moztesting/util.py#l17 ...so the test runner ends up with this URL (note the %20): http://mochi.test:8888/tests/toolkit/components/passwordmgr/test%20 If I pass backslashes (with two layers of escaping): ./mach mochitest-plain toolkit\\\\\\\\components\\\\\\\\passwordmgr\\\\\\\\test\\\\\\\\ ...then the correct path makes it through but the backslashes end up in the URL, making it invalid: http://mochi.test:8888/tests/toolkit%5Ccomponents%5Cpasswordmgr%5Ctest%5C
Assignee | ||
Comment 1•11 years ago
|
||
I think it's better not to use parse_test_path here at all. Unlike the mach reftest command, the mochitest commands don't actually need or use this parsing.
Assignee | ||
Comment 2•11 years ago
|
||
"mach reftest $DIR" is broken in a similar way but will require a different fix. I'm working on that now.
Assignee | ||
Comment 3•11 years ago
|
||
Uploaded the wrong patch by accident.
Attachment #711635 -
Attachment is obsolete: true
Attachment #711635 -
Flags: review?(gps)
Attachment #711676 -
Flags: review?(gps)
Assignee | ||
Comment 4•11 years ago
|
||
mach on Windows sets a TEST_PATH with backslashes, so TEST_PATH needs to be quoted in command lines. Also fixed some bugs in parse_test_path: * Call normpath on both test_path and topsrcdir before comparing them, so the comparison works whether you use forward- or backslashes on Windows. * Don't append a path separator; this is unnecessary since we use os.path.join to concatenate paths.
Attachment #711677 -
Flags: review?(gps)
Assignee | ||
Updated•11 years ago
|
Whiteboard: [has patch]
Comment 5•11 years ago
|
||
(In reply to Matt Brubeck (:mbrubeck) from comment #1) > Created attachment 711635 [details] [diff] [review] > patch > > I think it's better not to use parse_test_path here at all. Unlike the mach > reftest command, the mochitest commands don't actually need or use this > parsing. Remove the from moztesting.util import parse_test_path line, then, please.
Assignee | ||
Comment 6•11 years ago
|
||
(In reply to :Ms2ger from comment #5) > Remove the > > from moztesting.util import parse_test_path Done, thanks.
Attachment #711676 -
Attachment is obsolete: true
Attachment #711676 -
Flags: review?(gps)
Attachment #711767 -
Flags: review?(gps)
Comment 7•11 years ago
|
||
Comment on attachment 711767 [details] [diff] [review] fix mochitests (v2) Review of attachment 711767 [details] [diff] [review]: ----------------------------------------------------------------- LGTM.
Attachment #711767 -
Flags: review?(gps) → review+
Comment 8•11 years ago
|
||
Comment on attachment 711677 [details] [diff] [review] fix reftests Review of attachment 711677 [details] [diff] [review]: ----------------------------------------------------------------- Why do we need to add quotes to the paths in testsuite-targets.mk? Is mach passing a different path than "native" invocation? I think adding quotes is OK. I'm just curious. Will r+ once I have answer.
Assignee | ||
Comment 9•11 years ago
|
||
(In reply to Gregory Szorc [:gps] from comment #8) > Why do we need to add quotes to the paths in testsuite-targets.mk? Is mach > passing a different path than "native" invocation? I think adding quotes is > OK. I'm just curious. Manually invoking "make" is also broken if you use backslashes in TEST_PATH: make TEST_PATH=layout/reftests/scrolling reftest # this works make TEST_PATH=layout\\reftests\\scrolling reftest # broken mach always passes backslashes, so it is always broken. This patch fixes both "mach" and "make" with backslashes.
Comment 10•11 years ago
|
||
Comment on attachment 711677 [details] [diff] [review] fix reftests Review of attachment 711677 [details] [diff] [review]: ----------------------------------------------------------------- LGTM.
Attachment #711677 -
Flags: review?(gps) → review+
Assignee | ||
Comment 11•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f052742266d https://hg.mozilla.org/integration/mozilla-inbound/rev/d01bf199fcba
Whiteboard: [has patch]
Comment 12•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1f052742266d https://hg.mozilla.org/mozilla-central/rev/d01bf199fcba
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Updated•5 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•