1378239 - Change moz-info 'coverage' flag name to avoid ambiguity between ccov and jsdcov skipped tests.

Status: RESOLVED FIXED

(Testing :: Code Coverage, enhancement)

Description

[Greg Mierzwinski [:sparky]]

The flag currently in use to disable tests on linux64-ccov has the same name as the flag that is used by tests that need to be skipped on linux64-jsdcov. We should have two names so that the tests are not mistakenly disabled in both. 

For clarity, suppose that two tests within xpcshell are disabled. There is no way to distinguish which platform they should be disabled on and they will be disabled on both by default. Adding a second flag will help us make this distinction and remove the ambiguity.

A simple name to change it to would be 'ccoverage'. Then, the tests running on jsdcov could have the name 'jscoverage'.

Comment 1

[Greg Mierzwinski [:sparky]]

Attachment: Bug 1378239 - Change mozinfo 'coverage' flag to 'ccov' to avoid ambiguity.

This patch renames the mozinfo flag 'coverage' to 'ccoverage' to avoid ambiguity in whether a test is being skipped for linux64-ccov or for linux64-jsdcov. It also removes the 'runtests.py' mozinfo hack and renames all occurrences of 'coverage' that are used for skipping tests in linux64-ccov.

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

Comment 2

[Greg Mierzwinski [:sparky]]

:ahal, I wasn't sure who else could review this so if you think someone else should, feel free to change the reviewer. It's a small mozinfo change and here's a test run with these changes: https://treeherder.mozilla.org/#/jobs?repo=try&revision=625f57873175618a67262edbba1c281da5ce3c39

Comment 3

[Andrew Halberstadt [:ahal]]

Comment on attachment 8883569 [details]
Bug 1378239 - Change mozinfo 'coverage' flag to 'ccov' to avoid ambiguity.

https://reviewboard.mozilla.org/r/154506/#review159622

Thanks, looks good!

::: python/mozbuild/mozbuild/mozinfo.py:98
(Diff revision 1)
>      d['require_signing'] = substs.get('MOZ_REQUIRE_SIGNING') == '1'
>      d['no_legacy_extensions'] = substs.get('MOZ_ALLOW_LEGACY_EXTENSIONS') == '0'
>      d['official'] = bool(substs.get('MOZILLA_OFFICIAL'))
>      d['updater'] = substs.get('MOZ_UPDATER') == '1'
>      d['artifact'] = substs.get('MOZ_ARTIFACT_BUILDS') == '1'
> -    d['coverage'] = substs.get('MOZ_CODE_COVERAGE') == '1'
> +    d['ccoverage'] = substs.get('MOZ_CODE_COVERAGE') == '1'

This might just be better as 'ccov'. I'll leave it up to you.

Comment 4

[Greg Mierzwinski [:sparky]]

Thanks for the review!

I thought about this at the beginning and my reasoning for using this name is that it would be easier for us to find all skipped tests with a simple query on dxr like this: https://dxr.mozilla.org/mozilla-central/search?q=skip-if+coverage&redirect=false

But now that I think about it more, linux64-ccov runs js code coverage as well and 'ccoverage' doesn't really represent that. I'll change it to 'ccov' so that it's easier to see that these tests are skipped on linux64-ccov and not linux64-jsdcov.

Comment 5

[Greg Mierzwinski [:sparky]]

Comment on attachment 8883569 [details]
Bug 1378239 - Change mozinfo 'coverage' flag to 'ccov' to avoid ambiguity.

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

Comment 6

[Greg Mierzwinski [:sparky]]

Comment on attachment 8883569 [details]
Bug 1378239 - Change mozinfo 'coverage' flag to 'ccov' to avoid ambiguity.

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

Comment 7

[Greg Mierzwinski [:sparky]]

Fixed the commit message in the latest revision.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/789ccac5e77d
Change mozinfo 'coverage' flag to 'ccov' to avoid ambiguity. r=ahal

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/789ccac5e77d