Closed Bug 1697066 Opened 2 years ago Closed 1 year ago

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

Categories

(Testing :: Code Coverage, task, P2)

Unspecified
Linux
task

Tracking

(Fission Milestone:MVP, firefox92 wontfix, firefox93 wontfix, firefox94 fixed)

RESOLVED FIXED
94 Branch
Fission Milestone MVP
Tracking Status
firefox92 --- wontfix
firefox93 --- wontfix
firefox94 --- fixed

People

(Reporter: cpeterson, Assigned: ahal)

References

(Blocks 1 open bug)

Details

(Whiteboard: fission-soft-blocker)

Attachments

(2 files)

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.

Severity: N/A → --
Component: DOM: Content Processes → Code Coverage
Priority: P3 → --
Product: Core → Testing

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

Assignee: nobody → ahal
Severity: -- → N/A
Priority: -- → P2

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

Fission Milestone: M7a → M8

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.

Flags: needinfo?(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.

Flags: needinfo?(cpeterson) → needinfo?(mcastelluccio)

(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.

Flags: needinfo?(mcastelluccio)

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.

Whiteboard: fission-soft-blocker

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

Fission Milestone: M8 → MVP

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

Flags: needinfo?(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.

Flags: needinfo?(ahal)
Status: NEW → ASSIGNED
  • 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?

Flags: needinfo?(cpeterson)

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.

(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.

Flags: needinfo?(cpeterson) → needinfo?(ahal)

Makes sense!

Flags: needinfo?(ahal)

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.

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.

Flags: needinfo?(mcastelluccio)

(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.

Flags: needinfo?(mcastelluccio)

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)

Flags: needinfo?(mcastelluccio)

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.

Flags: needinfo?(mcastelluccio)
Attachment #9239268 - Attachment description: WIP: Bug 1697066 - [ci] Run ccov tasks with fission enabled → Bug 1697066 - [ci] Run ccov tasks with fission enabled, r?#releng-reviewers!
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1118f2c0bd9d
[ci] Run ccov tasks with fission enabled, r=releng-reviewers,jmaher
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 94 Branch
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

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

Pushed by ahalberstadt@mozilla.com:
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

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

Status: REOPENED → RESOLVED
Closed: 1 year ago1 year ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.