Closed Bug 920184 Opened 8 years ago Closed 8 years ago

mach xpcshell-test should automatically ensure test files are up to date


(Testing :: XPCShell Harness, defect)

Not set


(Not tracked)



(Reporter: gps, Assigned: gps)


(Blocks 1 open bug)


(Whiteboard: [mozbase])


(1 file, 1 obsolete file)

Now that test files are installed via install manifests (bug 901990), we should now be able to have the mach command(s) for running tests automatically ensure test files are up to date before running them.

Let's prototype this with the xpcshell command.

mach has a facility to invoke other mach commands. We could probably make the xpcshell-test command invoke |mach build install-tests| with NO_REMOVE=1 in the environment (to prevent removals).
Whiteboard: [mozbase]
Patch should be mostly self-explanatory.

The rules in are such that if an xpcshell.ini or
is updated, we'll perform a traversal. So, the developer
edit-test loop with this patch should "just work."
Attachment #809381 - Flags: review?(ted)
Assignee: nobody → gps
Duplicate of this bug: 920784
Now with mochitest command goodness.

I moved the logic for installing tests into a new class. I've been
warning to create a high-level "build API" class for a while. This patch
is my excuse.
Attachment #812061 - Flags: review?(ted)
Attachment #809381 - Attachment is obsolete: true
Attachment #809381 - Flags: review?(ted)
Comment on attachment 812061 [details] [diff] [review]
Have mach xpcshell-test automatically install test files

Review of attachment 812061 [details] [diff] [review]:

::: python/mozbuild/mozbuild/controller/
@@ +536,5 @@
> +
> +class BuildDriver(MozbuildObject):
> +    """Provides a high-level API for build actions."""
> +
> +    def install_tests(self, remove=True):

Seems a little weird that you have to override the default for remove every single place you invoke it.

::: testing/mochitest/
@@ +579,4 @@
>          self._ensure_state_subdir_exists('.')
> +        driver = self._spawn(BuildDriver)
> +        driver.install_tests(remove=False)

This repeated code feels like it wants to wind up in a decorator.
Attachment #812061 - Flags: review?(ted) → review+

I can see both comments. But, the default behavior of "install_tests" from the perspective of the build system is to delete untracked files. So, a default to remove from the Python API makes sense.

Once we have 100% coverage of _tests, we can remove the support for controlling removing from the API.

w.r.t. the repeated code, there is no common class for everything to inherit from. I'm not sure 2 lines is worth it.
Flags: in-testsuite-
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Blocks: clobber
You need to log in before you can comment on or make changes to this bug.