Closed
Bug 962919
Opened 11 years ago
Closed 11 years ago
Invoking mochitest-remote with a directory omits B2G excludes from manifest
Categories
(Testing :: Mochitest, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla30
People
(Reporter: drno, Assigned: drno)
Details
Attachments
(1 file, 3 obsolete files)
1.75 KB,
patch
|
drno
:
review+
|
Details | Diff | Splinter Review |
When "mach mochitest-remote" is invoked for B2G and executes all tests the list of tests are filtered according to the excludes list in the b2g.json manifest.
But if a directory is added as a parameter the manifest with its excludes gets ignored. If that directory contains tests which fail on B2G, you have to remove them manually from the mochitest.ini file to not have them executed.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → drno
Assignee | ||
Comment 1•11 years ago
|
||
If the a single is provided the manifest with its excludes is still ignored, but if a directory is provided the content of the directory is filtered by the excludes from the B2G manifest.
Attachment #8364117 -
Flags: review?(ted)
Attachment #8364117 -
Flags: review?(hskupin)
Comment 2•11 years ago
|
||
Comment on attachment 8364117 [details] [diff] [review]
use_manifest_for_mochitest_with_directory.patch
Review of attachment 8364117 [details] [diff] [review]:
-----------------------------------------------------------------
I think Gregory should be a better reviewer for mach related code changes as I am.
Attachment #8364117 -
Flags: review?(hskupin) → review?(gps)
Comment 3•11 years ago
|
||
Comment on attachment 8364117 [details] [diff] [review]
use_manifest_for_mochitest_with_directory.patch
Review of attachment 8364117 [details] [diff] [review]:
-----------------------------------------------------------------
I think this is okay, but I'm not really familiar with B2G testrunners, so punting to ahal just to be sure.
Attachment #8364117 -
Flags: review?(ted)
Attachment #8364117 -
Flags: review?(gps)
Attachment #8364117 -
Flags: review?(ahalberstadt)
Comment 4•11 years ago
|
||
Comment on attachment 8364117 [details] [diff] [review]
use_manifest_for_mochitest_with_directory.patch
Review of attachment 8364117 [details] [diff] [review]:
-----------------------------------------------------------------
This makes sense. I'd prefer if you could change the logic around so we only set options.testManifest in one place, but it's not a big deal.
Attachment #8364117 -
Flags: review?(ahalberstadt) → review+
Assignee | ||
Updated•11 years ago
|
Attachment #8364117 -
Attachment is obsolete: true
Assignee | ||
Comment 5•11 years ago
|
||
New patch with the same functionality, but without code duplication.
Attachment #8369754 -
Flags: review?(ahalberstadt)
Assignee | ||
Updated•11 years ago
|
Status: NEW → ASSIGNED
Comment 6•11 years ago
|
||
(In reply to Nils Ohlmeier [:drno] from comment #5)
> Created attachment 8369754 [details] [diff] [review]
> use_manifest_for_mochitest_with_directory.patch
>
> New patch with the same functionality, but without code duplication.
Looks like you accidentally submitted an empty patch?
Flags: needinfo?(drno)
Assignee | ||
Updated•11 years ago
|
Attachment #8369754 -
Attachment is obsolete: true
Attachment #8369754 -
Flags: review?(ahalberstadt)
Flags: needinfo?(drno)
Assignee | ||
Comment 7•11 years ago
|
||
Sorry about that. I should have either properly used 'hg qrefresh' before attaching the patch, or at least looked at the patch before attaching :-)
Attachment #8371842 -
Flags: review?(ahalberstadt)
Comment 8•11 years ago
|
||
Comment on attachment 8371842 [details] [diff] [review]
use_manifest_for_mochitest_with_directory.patch
Review of attachment 8371842 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks, looks good with the nit addressed!
::: testing/mochitest/mach_commands.py
@@ +144,4 @@
> options.testPath = test_path
> +
> + # filter test directiories or all tests according to the manifest
> + if ((not test_path) or (test_path_dir)):
nit: generally brackets are avoided in python where not necessary, just do:
if not test_path or test_path_dir:
Attachment #8371842 -
Flags: review?(ahalberstadt) → review+
Assignee | ||
Comment 9•11 years ago
|
||
v3 (carrying forward r+=ahal)
Addressed the nits from ahal's review.
Attachment #8373474 -
Flags: review+
Assignee | ||
Updated•11 years ago
|
Attachment #8371842 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 10•11 years ago
|
||
Keywords: checkin-needed
Comment 11•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in
before you can comment on or make changes to this bug.
Description
•