1278890 - Add mach test package command for running the various harnesses

Status: RESOLVED FIXED

(Testing :: General, defect, P1)

Description

[Andrew Halberstadt [:ahal]]

As part of an effort to make debugging tests remotely easy, we are providing an interface to run test harnesses via mach directly from a tests.zip. Running tests from the srcdir would still be preferable, but is likely a long way out. This isn't hard to do and will be useful in the meantime.

This will hook up xpcshell to the mach in the tests.zip.

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1278890 - Search for a firefox binary in the test package mach bootstrap,

Review commit: https://reviewboard.mozilla.org/r/58888/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/58888/

Comment 2

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1278890 - Add a method for normalizing test paths to the test package mach environment,

The test path structure is slightly different in the test package compared to a srcdir. So we may need
to normalize the specified paths such that they are relative to a srcdir. Because every test harness
needs to do this, this method is being added to the bootstrap for re-use.

Review commit: https://reviewboard.mozilla.org/r/64042/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/64042/

Comment 3

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1278890 - Add xpcshell support to test package mach environment,

This adds the 'xpcshell-test' command to the mach environment found in the test
package. This will allow developers to easily run xpcshell after checking out
and interactive worker.

Review commit: https://reviewboard.mozilla.org/r/64044/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/64044/

Comment 4

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1278890 - Add reftest support to test package mach environment,

This adds reftest support to the test package mach environment. It will allow
developers to easily run reftests after checking out an interactive worker.

Review commit: https://reviewboard.mozilla.org/r/64046/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/64046/

Comment 5

[Andrew Halberstadt [:ahal]]

Comment on attachment 8761822 [details]
Bug 1278890 - Search for a firefox binary in the test package mach bootstrap,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58888/diff/1-2/

Comment 6

[Armen [:armenzg]]

Comment on attachment 8761822 [details]
Bug 1278890 - Search for a firefox binary in the test package mach bootstrap,

https://reviewboard.mozilla.org/r/58888/#review61026

It looks functionally sound. I just want more comments to help out with giving context.

::: testing/tools/mach_test_package_bootstrap.py:98
(Diff revision 2)
> +        with open(context.mozharness_config, 'r') as f:
> +            config = json.load(f)
> +        workdir = os.path.join(config['base_work_dir'], config['work_dir'])
> +        search_paths.append(os.path.join(workdir, 'application'))
> +
> +    # Check for test-stage setup

What does test-stage mean in this context?

::: testing/tools/mach_test_package_bootstrap.py:99
(Diff revision 2)
> +            config = json.load(f)
> +        workdir = os.path.join(config['base_work_dir'], config['work_dir'])
> +        search_paths.append(os.path.join(workdir, 'application'))
> +
> +    # Check for test-stage setup
> +    dist_bin = os.path.join(os.path.dirname(context.package_root), 'bin')

Isn't this context.bin_dir?

::: testing/tools/mach_test_package_bootstrap.py:110
(Diff revision 2)
> +            return mozinstall.get_binary(path, 'firefox')
> +        except mozinstall.InvalidBinary:
> +            continue
> +
> +
>  def bootstrap(test_package_root):

Could you please add a docstring to this function?
It is becoming rather big.

::: testing/tools/mach_test_package_bootstrap.py:128
(Diff revision 2)
>  
>      def populate_context(context, key=None):
>          if key is not None:
>              return
>  
>          context.package_root = test_package_root

What is package_root?

Comment 7

[Armen [:armenzg]]

Comment on attachment 8770611 [details]
Bug 1278890 - Add a method for normalizing test paths to the test package mach environment,

https://reviewboard.mozilla.org/r/64042/#review61098

::: testing/tools/mach_test_package_bootstrap.py:112
(Diff revision 1)
>              continue
>  
>  
> +def normalize_test_path(test_root, path):
> +    if os.path.isabs(path) or os.path.exists(path):
> +        return os.path.normpath(os.path.abspath(path))

Do we need to check for isabs() if we're going to call abspath() in the return statement?

Is the path useful if it is absolute, however, it does not exist?

Comment 8

[Armen [:armenzg]]

Comment on attachment 8770612 [details]
Bug 1278890 - Add xpcshell support to test package mach environment,

https://reviewboard.mozilla.org/r/64044/#review61100

Comment 9

[Armen [:armenzg]]

Comment on attachment 8770613 [details]
Bug 1278890 - Add reftest support to test package mach environment,

https://reviewboard.mozilla.org/r/64046/#review61102

Comment 10

[Andrew Halberstadt [:ahal]]

https://reviewboard.mozilla.org/r/64042/#review61098

> Do we need to check for isabs() if we're going to call abspath() in the return statement?
> 
> Is the path useful if it is absolute, however, it does not exist?

Yeah, we need to check isabs to prevent absolute paths from reaching the code path below (where we try to os.path.join it). You're right to notice that the abspath() call is redundant in that case, but the code is a bit cleaner this way.

Also, absolute paths that don't exist aren't useful, but I return it anyway so the underlying harnesses can handle them however they're supposed to be handled (e.g log an error etc). I don't really want to log these errors at this point as that would make running tests on an interactive worker produce different output from the normal harness.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/06ca3f2b883b
Search for a firefox binary in the test package mach bootstrap, r=armenzg
https://hg.mozilla.org/integration/autoland/rev/45d6a990dd28
Add a method for normalizing test paths to the test package mach environment, r=armenzg
https://hg.mozilla.org/integration/autoland/rev/3e6f9e3a99a1
Add xpcshell support to test package mach environment, r=armenzg
https://hg.mozilla.org/integration/autoland/rev/683bf5d3c1cd
Add reftest support to test package mach environment, r=armenzg

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/06ca3f2b883b
https://hg.mozilla.org/mozilla-central/rev/45d6a990dd28
https://hg.mozilla.org/mozilla-central/rev/3e6f9e3a99a1
https://hg.mozilla.org/mozilla-central/rev/683bf5d3c1cd