1697066 - Enable "ccov" code coverage tests for Fission+WebRender on Linux

Status: RESOLVED FIXED

(Testing :: Code Coverage, task, P2)

Description

[Chris Peterson [:cpeterson]]

We want to make sure our tests aren't missing any critical Fission code paths. We would like to run ccov for the following basic test suites with Fission (plus WebRender for tests that we're already running on Linux with WebRender):

• mochitest-browser-chrome (Fission without WebRender)
• mochitest-devtools-chrome (Fission without WebRender)
• mochitest-plain with xorigin mode (Fission with WebRender)
• mochitest-plain without xorigin mode (Fission with WebRender)
• web-platform-tests (Fission with WebRender)

I'm waiting to hear from the DevTools team if they would like us to run ccov for mochitest-devtools-chrome or any other DevTools tests that might have different code paths with Fission. Edit: The DevTools team confirmed that they would like us to measure the code coverage of mochitest-devtools-chrome.

We only need to test on Linux.

Comment 1

[Chris Peterson [:cpeterson]]

Assigning remaining "Enable Fission tests on more platforms" bugs to ahal.

Comment 2

[Chris Peterson [:cpeterson]]

Deferring to Fission M8 milestone because ccov tests don't need to block our Linux Beta experiment (but should still block shipping Fission).

Comment 3

[Andrew Halberstadt [:ahal]]

Hey Chris, ccov is very costly and we're worried that duplicating all these tasks won't be worth while. Do you know if anyone will be committing to use the data generated by these tasks? Maybe a better conversation would be to transition the old ccov tasks over to fission instead, seeing as it will become the default? I don't have specific details on the cost here atm.

Comment 4

[Chris Peterson [:cpeterson]]

(In reply to Andrew Halberstadt [:ahal] from comment #3)

Hey Chris, ccov is very costly and we're worried that duplicating all these tasks won't be worth while. Do you know if anyone will be committing to use the data generated by these tasks? Maybe a better conversation would be to transition the old ccov tasks over to fission instead, seeing as it will become the default? I don't have specific details on the cost here atm.

@ Marco, how do you feel about converting the current ccov tasks to Fission before Fission is enabled by default?

As the EPM for the Fission project, this sounds fine to me :) but you might have different priorities for ccov. I expect Fission to be enabled by default near the end of 2021.

Comment 5

[Marco Castelluccio [:marco]]

(In reply to Chris Peterson [:cpeterson] from comment #4)

(In reply to Andrew Halberstadt [:ahal] from comment #3)

Hey Chris, ccov is very costly and we're worried that duplicating all these tasks won't be worth while. Do you know if anyone will be committing to use the data generated by these tasks? Maybe a better conversation would be to transition the old ccov tasks over to fission instead, seeing as it will become the default? I don't have specific details on the cost here atm.

@ Marco, how do you feel about converting the current ccov tasks to Fission before Fission is enabled by default?

As the EPM for the Fission project, this sounds fine to me :) but you might have different priorities for ccov. I expect Fission to be enabled by default near the end of 2021.

I agree with moving the ccov build to Fission. It doesn't reflect yet what we ship to users, but it will soon.

I'm just a bit scared the coverage data could be somehow broken by Fission (I don't expect it will be, but you never know!), so I'd like to see a try push and compare the coverage data before/after.

Comment 6

[Chris Peterson [:cpeterson]]

This bug is a soft blocker for Fission M8. We'd like to fix it before our M8 Release experiment, but we won't delay the experiment waiting for it.

Comment 7

[Chris Peterson [:cpeterson]]

Moving from Fission M8 to MVP because ahal is out on PTO until August 11.

Comment 8

[Chris Peterson [:cpeterson]]

Andrew, do you have cycles to switch the ccov tests from e10s to Fission before the end of August?

Comment 9

[Andrew Halberstadt [:ahal]]

Probably not, mostly because I'll be on PTO for half of next week. I'll add this to the Next column of our board though.

Comment 10

[Andrew Halberstadt [:ahal]]

• mochitest-browser-chrome (Fission without WebRender)
• mochitest-devtools-chrome (Fission without WebRender)

Chris, since disabling webrender is no longer possible shall I assume you want these enabled with webrender now instead?

Comment 11

[Andrew Halberstadt [:ahal]]

Oh wait, you probably mean swr which is the default variant enabled there for fission.. I'll leave the needinfo so you can clarify regardless.

Comment 12

[Chris Peterson [:cpeterson]]

(In reply to Andrew Halberstadt [:ahal] from comment #10)

• mochitest-browser-chrome (Fission without WebRender)
• mochitest-devtools-chrome (Fission without WebRender)

Chris, since disabling webrender is no longer possible shall I assume you want these enabled with webrender now instead?

I think the only reason I requested "Fission without WebRender" for those particular tests was that they were previously running without WebRender. I didn't want to add two new variables (enabling Fission and enabling WebRender) at the same time.

If swr is the default for mochitest-browser-chrome and mochitest-devtools-chrome on Fission, then we can use swr for Fission ccov, too.

Comment 13

[Andrew Halberstadt [:ahal]]

Makes sense!

Comment 14

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1697066 - [ci] Run ccov tasks with fission enabled, r?#releng-reviewers!

Here's the task diff for mozilla-central:
+test-linux1804-64-ccov-qr/opt-mochitest-browser-chrome-swr-fis-e10s
-test-linux1804-64-ccov-qr/opt-mochitest-devtools-chrome-e10s
+test-linux1804-64-ccov-qr/opt-mochitest-devtools-chrome-fis-e10s
-test-linux1804-64-ccov-qr/opt-mochitest-plain-e10s
+test-linux1804-64-ccov-qr/opt-mochitest-plain-fis-e10s
+test-linux1804-64-ccov-qr/opt-mochitest-plain-fis-xorig-e10s
-test-linux1804-64-ccov-qr/opt-web-platform-tests-e10s
+test-linux1804-64-ccov-qr/opt-web-platform-tests-fis-e10s

Note we weren't previously running any ccov browser-chrome tasks on Linux so
that's why no task was removed on that configuration.

Comment 15

[Andrew Halberstadt [:ahal]]

Marco, here's a try push with the added tasks:
https://treeherder.mozilla.org/jobs?repo=try&revision=5f9deea0fdbe0d545947290c007b11a3688b0fd5

Is that enough to compare coverage results against? Or do you need a full run with all ccov tasks for a fair comparison?

I will note that we previously weren't running any browser-chrome ccov tasks on Linux and now we are, so the odds of this patch regressing coverage accuracy are low.

Comment 16

[Marco Castelluccio [:marco]]

(In reply to Andrew Halberstadt [:ahal] from comment #15)

Marco, here's a try push with the added tasks:
https://treeherder.mozilla.org/jobs?repo=try&revision=5f9deea0fdbe0d545947290c007b11a3688b0fd5

Is that enough to compare coverage results against? Or do you need a full run with all ccov tasks for a fair comparison?

It'd be better to add all ccov tasks, so we can then compare the overall coverage percentage to make sure the results are not wildly different (overall coverage is currently around 65%). We could make a deeper comparison, but I think it isn't worth it, we just want to make sure the results are sane.
Here's how to generate a local report: https://firefox-source-docs.mozilla.org/tools/code-coverage/index.html#generate-report-locally.

I will note that we previously weren't running any browser-chrome ccov tasks on Linux and now we are, so the odds of this patch regressing coverage accuracy are low.

We are running test-linux1804-64-ccov-qr/opt-mochitest-browser-chrome-swr-e10s-* tasks, see for example https://treeherder.mozilla.org/jobs?repo=mozilla-central&searchStr=linux%2Cbrowser-chrome%2Cccov&revision=3634785345fe6391e073ebf19e3f7ce031a43511.

Comment 17

[Andrew Halberstadt [:ahal]]

Here's an updated try run:
https://treeherder.mozilla.org/jobs?repo=try&revision=1dc3410a5a6a731ec632006b5ae3f9fbc6ed64f8

I tried to follow the directions and generate a report, but the resulting index.html just shows 0% across the board. Not sure what I'm doing wrong? Any ideas Marco? (Or would you be able to verify yourself? :p)

Comment 18

[Marco Castelluccio [:marco]]

There was a bug in the report script that I fixed, you should be able to run it now.
I've also run it myself. The overall coverage percentage looks in line with the non-Fission one, so I think we can go ahead.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1118f2c0bd9d
[ci] Run ccov tasks with fission enabled, r=releng-reviewers,jmaher

Comment 20

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/1118f2c0bd9d

Comment 21

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1697066 - [ci] Stop running ccov M-swr(bc) tasks as we already run them with fission, r?marco!

Comment 22

[Andrew Halberstadt [:ahal]]

I accidentally enabled M-fis-swr(bc) in addition to M-swr(bc) rather than replacing it.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ede86e9e0fdb
[ci] Stop running ccov M-swr(bc) tasks as we already run them with fission, r=marco

Comment 24

[Chris Peterson [:cpeterson]]

Setting status-firefox93=wontfix because we don't need to uplift this test change to Beta 93.

Comment 25

[Iulian Moraru]

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