1301214 - Enable collection of devtools c++ code coverage with GCOV and create a devtools task for linux64-ccov.

Status: RESOLVED FIXED

(Testing :: Code Coverage, defect)

Description

[Greg Mierzwinski [:sparky]]

We'd like to be able to collect c++ code coverage for devtools. To get this working I believe we'd have to enable it to use the "--code-coverage" flag just like it is done for browser-chrome. It may in fact work already as devtools and browser-chrome are both in mochitest.

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

I think here we just need to define the test to run against a linux64-ccov build (in test.yml) and pass the proper flag to mochitest to enable and manage settings for coverage (this is only needed for JSVM which we should do).

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

once we get the ccov builds properly supported in the tree and on try only, we just need to verify on try and land any changes to test.yml configuration.

Comment 3

[cody_tran95]

Attachment: Bug 1301214 - Changed action of code coverage options to store_true and enabled code coverage for mochitest-devtools-chrome.

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

Comment 4

[Greg Mierzwinski [:sparky]]

Comment on attachment 8803110 [details]
Bug 1301214 - Changed action of code coverage options to store_true and enabled code coverage for mochitest-devtools-chrome.

https://reviewboard.mozilla.org/r/87328/#review86380

I think this is great, nice work! There are only a few small things to take care of before it's ready.

::: taskcluster/ci/desktop-test/tests.yml:285
(Diff revision 1)
>      max-run-time: 5400
>      chunks: 10
> +    run-on-projects:
> +        by-test-platform:
> +            linux64-ccov/opt: []
> +            default: ['all']
>      e10s:
>          by-test-platform:
>              # Bug 1242986: linux64/debug mochitest-devtools-chrome e10s is not greened up yet
>              linux64/debug: false
> +            linux64-ccov/opt: false
>              default: both
> +    max-run-time:

Duplicate declaration of the max-run-time attribute here.

::: taskcluster/ci/desktop-test/tests.yml:299
(Diff revision 1)
>              linux64/debug: false
> +            linux64-ccov/opt: false
>              default: both
> +    max-run-time:
> +        by-test-platform:
> +            linux64-ccov/opt: 7200

Is 2 hours necessary or can we reduce the max-run-time?

::: taskcluster/ci/desktop-test/tests.yml:321
(Diff revision 1)
> -            - --mochitest-suite=mochitest-devtools-chrome-chunked
> +                    - --mochitest-suite=mochitest-devtools-chrome-chunked
>      instance-size:
>          by-test-platform:
>              # Bug 1281241: migrating to m3.large instances
>              linux64-asan/opt: legacy
> +            linux64-ccov/opt: xlarge

I believe mochitest-bc needed this extra large instance because it had bugs otherwise. Do we also get bugs in devtools if we use a smaller instance?

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8803110 [details]
Bug 1301214 - Changed action of code coverage options to store_true and enabled code coverage for mochitest-devtools-chrome.

https://reviewboard.mozilla.org/r/87328/#review86530

assuming you validate the instance type we need (as per Greg's review comment), the rest looks good.

Comment 7

[cody_tran95]

Comment on attachment 8803110 [details]
Bug 1301214 - Changed action of code coverage options to store_true and enabled code coverage for mochitest-devtools-chrome.

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

Comment 8

[cody_tran95]

Comment on attachment 8803110 [details]
Bug 1301214 - Changed action of code coverage options to store_true and enabled code coverage for mochitest-devtools-chrome.

https://reviewboard.mozilla.org/r/87328/#review86380

> Is 2 hours necessary or can we reduce the max-run-time?

Pushed to try with default time of 5400 and that seemed to work

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d65a467e35a2

> I believe mochitest-bc needed this extra large instance because it had bugs otherwise. Do we also get bugs in devtools if we use a smaller instance?

Changed to default and it worked.

Comment 9

[Greg Mierzwinski [:sparky]]

Comment on attachment 8803110 [details]
Bug 1301214 - Changed action of code coverage options to store_true and enabled code coverage for mochitest-devtools-chrome.

https://reviewboard.mozilla.org/r/87328/#review87468

Good work Cody! It's good to go now in my opinion.

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

can you rebase this commit?  autoland failed to rebase, can you do it manually and then repush to mozreview?

here are some general guidelines and you can find the rebase stuff on there:
http://mozilla-version-control-tools.readthedocs.io/en/latest/hgmozilla/firefoxworkflow.html

Comment 11

[cody_tran95]

Comment on attachment 8803110 [details]
Bug 1301214 - Changed action of code coverage options to store_true and enabled code coverage for mochitest-devtools-chrome.

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

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e36c5db0191e
Changed action of code coverage options to store_true and enabled code coverage for mochitest-devtools-chrome. r=gmierz2+560562,jmaher

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e36c5db0191e