Closed Bug 799291 Opened 12 years ago Closed 12 years ago

Move mozbuild/mach commands alongside test runners

Categories

(Testing :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla19

People

(Reporter: gps, Assigned: gps)

References

Details

Attachments

(3 files, 1 obsolete file)

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)
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)
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)
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 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+
They should. Python's path handling transparently handles both slash flavors.
Attachment #669335 - Flags: review?(jhammel) → review+
Attachment #669339 - Flags: review?(jhammel) → review+
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
Closed: 12 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.