Move mozbuild/mach commands alongside test runners

RESOLVED FIXED in mozilla19

Status

Testing
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gps, Assigned: gps)

Tracking

unspecified
mozilla19
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 669334 [details] [diff] [review]
Part 1: Move and consolidate code alongside test runners, v1

Currently the mozbuild Python package contains code for invoking test runners. It also provides the mach commands which interface into this proxy code.

In the series of patches about to be posted, the testing code from python/mozbuild is moved to be alongside the test runners themselves. This should increase the visibility of the code to the maintainers of the respective test runners. This should hopefully lead to better integration between test runners and mach.

A nice side effect of the series of patches is that mozbuild no longer contains code for running tests. Hooray!

In the new world, mach integration is contained within "mach_commands.py" files in the same directory as the test runners. The filename is irrelevant. But, we do hold a reference to it in the main $srcdir/mach script. We choose to have a standalone "proxy" module between mach and the test runners rather than integrating mach directly into the test runners because it is more loosely coupled. It also still keeps mach NPOTB, as the test runners avoid having to import its modules.

In the long run, MozbuildObject will likely be refactored into some kind of mix-in class provided by mach. We'll cross that bridge when we come to it.
Attachment #669334 - Flags: review?(jhammel)
Attachment #669334 - Flags: feedback?(ted.mielczarek)
Attachment #669334 - Flags: feedback?(Ms2ger)
(Assignee)

Comment 1

5 years ago
Created attachment 669335 [details] [diff] [review]
Part 2: Remove "test" command, v1

I think |mach test| is next to worthless until bug 765688 lands. At that point we can reintroduce it as something that is useful.
Assignee: nobody → gps
Status: NEW → ASSIGNED
Attachment #669335 - Flags: review?(jhammel)
(Assignee)

Comment 2

5 years ago
Created attachment 669337 [details] [diff] [review]
Part 3: Move mozbuild.testing into testing/

This was the only thing remaining in mozbuild.testing. I decided to create a new home for it in testing/. The most contentious part of this patch should be the naming and directory location. I have no clue what goes on in testing/.
Attachment #669337 - Flags: review?(jhammel)
(Assignee)

Comment 3

5 years ago
Created attachment 669339 [details] [diff] [review]
Part 3: Move mozbuild.testing into testing/, v2

I should learn to qref my patches. Initial version was horribly busted.
Attachment #669337 - Attachment is obsolete: true
Attachment #669337 - Flags: review?(jhammel)
Attachment #669339 - Flags: review?(jhammel)
Comment on attachment 669334 [details] [diff] [review]
Part 1: Move and consolidate code alongside test runners, v1

Review of attachment 669334 [details] [diff] [review]:
-----------------------------------------------------------------

wfm

::: python/mozbuild/mozbuild/testing/reftest.py
@@ +15,5 @@
> +    Command,
> +)
> +
> +
> +generic_help = 'Test to run. Can be specified as a single JS file, a ' +\

"JS file" doesn't make sense, especially since we don't support that.

::: python/mozbuild/mozbuild/testing/mochitest.py
@@ +15,5 @@
> +    Command,
> +)
> +
> +
> +generic_help = 'Test to run. Can be specified as a single JS file, a ' +\

At least in my part of m-c, mochitests are usually HTML files

::: python/mozbuild/mozbuild/testing/xpcshell.py
@@ +105,5 @@
> +             'or omitted. If omitted, the entire test suite is executed.')
> +    @CommandArgument('--debug', '-d', action='store_true',
> +        help='Run test in a debugger.')
> +    def run_xpcshell_test(self, **params):
> +

Drop the empty line
Attachment #669334 - Flags: feedback?(Ms2ger) → feedback+
Comment on attachment 669335 [details] [diff] [review]
Part 2: Remove "test" command, v1

Review of attachment 669335 [details] [diff] [review]:
-----------------------------------------------------------------

I had this on my todo list, so, go for it! ;)
Attachment #669335 - Flags: feedback+

Comment 6

5 years ago
Comment on attachment 669334 [details] [diff] [review]
Part 1: Move and consolidate code alongside test runners, v1

+# Individual files providing mach commands.
+MACH_MODULES = [
+    'layout/tools/reftest/mach_commands.py',
+    'testing/mochitest/mach_commands.py',
+    'testing/xpcshell/mach_commands.py',
+]
+

Will this work on, ahem, non-POSIX operating systems?

If so, then lgtm
Attachment #669334 - Flags: review?(jhammel) → review+
(Assignee)

Comment 7

5 years ago
They should. Python's path handling transparently handles both slash flavors.

Updated

5 years ago
Attachment #669335 - Flags: review?(jhammel) → review+

Updated

5 years ago
Attachment #669339 - Flags: review?(jhammel) → review+
(Assignee)

Comment 8

5 years ago
https://hg.mozilla.org/mozilla-central/rev/428846e73299
https://hg.mozilla.org/mozilla-central/rev/ce9274758544
https://hg.mozilla.org/mozilla-central/rev/cff221895afe
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Comment on attachment 669334 [details] [diff] [review]
Part 1: Move and consolidate code alongside test runners, v1

Looks like I was too late to the party on this one.
Attachment #669334 - Flags: feedback?(ted)
You need to log in before you can comment on or make changes to this bug.