Enable collection of reftest code coverage with GCOV.

RESOLVED FIXED in Firefox 54

Status

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: sparky, Unassigned)

Tracking

(Blocks: 1 bug)

unspecified
mozilla54
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
We'd like to be able to collect code coverage from reftest with GCOV. This should be similar to mochitest-browser-chrome for linux64-ccov. The "--code-coverage" flag may work already and if not, it'll have to be implemented.
(Reporter)

Updated

2 years ago
Blocks: 1301170
(Reporter)

Updated

2 years ago
Blocks: 1301979
(Reporter)

Updated

2 years ago
Blocks: 1301980
a working patch is here:
https://hg.mozilla.org/try/rev/59f825e1d7d78e14ca8e086fab2f2cb47f59bc03

a lot has changed in task configuration since then, but this is a good starting place.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 2

2 years ago
Wow has that taskcluster folder ever changed in the last bit! I've found a bug in the tests.yml file also that will be fixed with this patch.

So this patch will add a transform for code coverage jobs and also implement code coverage for reftest, crashtest, and jsreftest all at once.

Here's a link to a test run using the new patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=de0f9113824af9334dbf47019d4e3bc221394b3d .

Reftest definitely works based on a previous job so this one is testing all the tests and making sure we haven't lost or broken anything in the process. It also tests crashtest and jsreftest to see if those work.
I like the transform- the simpler we can get our tests.yml file, I think the better off we will be- this patch simplifies it a lot :)
(Reporter)

Comment 4

2 years ago
Can you review this patch or should I have someone else review it? It's all set, and I've also disabled e10s for ref/jsref/crash-test because I'm not sure if the ETL can handle those differences yet.

I'm going to create another bug to add jsdcov into the transform and make the appropriate changes to tests.yml for the run-on-projects flag.
Flags: needinfo?(jmaher)
I will review this!
Flags: needinfo?(jmaher)
the patch on the try push is looking good and the try results are great, if you want to r=me on it and we can get it landed that would be ideal!
and a note, there are some flake8 errors with this patch
(Reporter)

Comment 8

2 years ago
I'm not sure why we're getting those flake8 errors because my changes didn't affect those parts. Also, it doesn't make sense to put an extra line where it says it needs one. What do you think?
for flake8, it could have been code from another revision that landed and then got fixed up- maybe refreshing your base revision?

for flake8 and other linters, they like to ensure there are 2 blank lines between methods/functions.  This offers a clear break.
(Reporter)

Comment 10

2 years ago
Ok, I've fixed the flaxe8 error. I was looking in the wrong 'tests' file. Here's a test run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bf323b25dd5e75ef87c9d60fbb9c07c3f1e0ffa9 .

I'll have a patch up for review later today.
Comment hidden (mozreview-request)

Comment 12

2 years ago
mozreview-review
Comment on attachment 8830126 [details]
Bug 1301978 - Enable reftest, crashtest, & jsreftest and use a transform on linux64-ccov tasks.

https://reviewboard.mozilla.org/r/107028/#review108222

thank you for putting this together!
Attachment #8830126 - Flags: review?(jmaher) → review+
(Reporter)

Comment 13

2 years ago
Thanks for the quick review! Would it be possible to have these changes landed soon?

Comment 14

2 years ago
Pushed by jmaher@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d0b05a7251a7
Enable reftest, crashtest, & jsreftest and use a transform on linux64-ccov tasks. r=jmaher

Comment 15

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d0b05a7251a7
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.