Add --dump-tests option in mochitest/xpcshell for all tests that will be run

RESOLVED FIXED in Firefox 43

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: vaibhav1994, Assigned: vaibhav1994)

Tracking

unspecified
mozilla43
Points:
---

Firefox Tracking Flags

(firefox43 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
From https://bugzilla.mozilla.org/show_bug.cgi?id=1197318#c4:

ahal:
I'd propose adding a --dump-tests option to mochitest. When this option is passed in, at the end of mochitest.active_tests, instead of returning, it would save the tests that would be run to a file and immediately call sys.exit(). This way, chunk-finder could invoke the harness with whatever command line is being used in automation, with the addition of --dump-tests. It could then scan the files for the missing test. This has the benefit of always being up to date with the right configuration.

You could similarly add a --dump-tests option to xpcshell as well. As another bonus, this approach could work with test harnesses that don't use manifestparser, like reftest.
(Assignee)

Updated

3 years ago
Summary: Add --dump-tests option in mochitest/xpcshell to dump the active_tests run → Add --dump-tests option in mochitest/xpcshell for all tests that will be run
(Assignee)

Comment 1

3 years ago
Created attachment 8651437 [details]
MozReview Request: Bug 1197541 -  Added --dump-tests option to mochitest and xpcshell for all tests that will be run. r=ahal

Bug 1197541 -  Added --dump-tests option to mochitest and xpcshell for all tests that will be run. r=ahal
Attachment #8651437 - Flags: review?(ahalberstadt)
(Assignee)

Comment 2

3 years ago
Andrew, in the review active_tests.json will be dumped to:
* /{obj_dir}/_tests/testing/mochitest/active_tests.json for mochitests
* /testing/xpcshell/active_tests.json for xpcshell (since there is no self.topobjdir present in it)

Let me know if the paths will be fine or if I need to change them.
(Assignee)

Updated

3 years ago
Blocks: 1197543
Attachment #8651437 - Flags: review?(ahalberstadt)
Comment on attachment 8651437 [details]
MozReview Request: Bug 1197541 -  Added --dump-tests option to mochitest and xpcshell for all tests that will be run. r=ahal

https://reviewboard.mozilla.org/r/16945/#review15159

Thanks, looks good. Just some minor comments to address.

::: testing/mochitest/mochitest_options.py:327
(Diff revision 1)
> +          "help": "Dump all the tests that will be run to active_tests.json",

I think we should add "suppress": True here. Alone it isn't that useful for finding chunks because users still need to dig through the file. So might as well hide it from the help menu.

::: testing/mochitest/runtests.py:1982
(Diff revision 1)
> +            active_tests_path = os.path.join(SCRIPT_DIR, 'active_tests.json')

Instead of hardcoding a path, let's just require the path on the command line. That way you can invoke the harness with --dump-tests multiple times without overwriting the file.
(Assignee)

Comment 4

3 years ago
Comment on attachment 8651437 [details]
MozReview Request: Bug 1197541 -  Added --dump-tests option to mochitest and xpcshell for all tests that will be run. r=ahal

Bug 1197541 -  Added --dump-tests option to mochitest and xpcshell for all tests that will be run. r=ahal
Attachment #8651437 - Flags: review?(ahalberstadt)
(Assignee)

Comment 5

3 years ago
Andrew, I have also added --dump-tests to mach for xpcshell-test since now it accepts filename, and it wouldn't make sense if we can't pass it through mach.
Comment on attachment 8651437 [details]
MozReview Request: Bug 1197541 -  Added --dump-tests option to mochitest and xpcshell for all tests that will be run. r=ahal

https://reviewboard.mozilla.org/r/16945/#review15271

A general nit throughout:
Please update from camelcase to underscores for new variable names. I know it's already a mix of both, but new variables should be underscored.

But with that fixed, looks good!

::: testing/mochitest/runtests.py:1982
(Diff revision 2)
> +            # Allow user to use shorthand '~' to home directory

nit: this comment isn't necessary

::: testing/xpcshell/runxpcshelltests.py:832
(Diff revision 2)
> +            # Allow user to use shorthand '~' to home directory

nit: comment not necessary
Attachment #8651437 - Flags: review?(ahalberstadt) → review+
(Assignee)

Updated

3 years ago
Attachment #8651437 - Flags: review+ → review?(ahalberstadt)
(Assignee)

Comment 7

3 years ago
Comment on attachment 8651437 [details]
MozReview Request: Bug 1197541 -  Added --dump-tests option to mochitest and xpcshell for all tests that will be run. r=ahal

Bug 1197541 -  Added --dump-tests option to mochitest and xpcshell for all tests that will be run. r=ahal
(Assignee)

Comment 8

3 years ago
Comment on attachment 8651437 [details]
MozReview Request: Bug 1197541 -  Added --dump-tests option to mochitest and xpcshell for all tests that will be run. r=ahal

Bug 1197541 -  Added --dump-tests option to mochitest and xpcshell for all tests that will be run. r=ahal
(Assignee)

Comment 10

3 years ago
Comment on attachment 8651437 [details]
MozReview Request: Bug 1197541 -  Added --dump-tests option to mochitest and xpcshell for all tests that will be run. r=ahal

Carrying forward the r+
Attachment #8651437 - Flags: review?(ahalberstadt) → review+
(Assignee)

Comment 11

3 years ago
Try run passes (even though this patch does not require a try run).
Assignee: nobody → vaibhavmagarwal
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/bb27412c9a79
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.