1421633 - lcov_rewriter.py is missing out on some file:/// paths

Status: NEW

(Testing :: Code Coverage, enhancement)

Description

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

we have 20K+ references to SF: in the jsvm.zip file from test runs, here is an example of one that we get coverage (27 covered, 3 not covered), but no reference in codecov.io:
file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/invalidation/filter-userspace-offset.svg?offsetContainer
=foreignObject&mask=boundingBox

Comment 1

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

another one:
"file:///Z:/task_1511823081/build/tests/jsreftest/tests/test262/language/statements/const/cptn-value.js line 25 > eval"

in this case we might have to ignore this as it is an eval statement.

Comment 2

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

if svg files are ignored, then here is an example of a .html file:
file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/scrolling/fractional-scroll-area.html?top=0.4&outerBottom=100.4&innerBottom=199.6

Comment 3

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

another case we are missing is obj-firefox/ files, such as:
obj-firefox/dist/include/nsIDOMWindowCollection.h

this appears to come from:
https://searchfox.org/mozilla-central/source/dom/interfaces/base/nsIDOMWindowCollection.idl

I am not sure what to do here, we post lines covered from the obj-firefox file though.

Comment 4

[Marco Castelluccio [:marco]]

(In reply to Joel Maher ( :jmaher) (UTC-5) from comment #3)
> another case we are missing is obj-firefox/ files, such as:
> obj-firefox/dist/include/nsIDOMWindowCollection.h
> 
> this appears to come from:
> https://searchfox.org/mozilla-central/source/dom/interfaces/base/
> nsIDOMWindowCollection.idl
> 
> I am not sure what to do here, we post lines covered from the obj-firefox
> file though.

The lcov rewriter only deals with files generated by the JS engine, while this is a C++ header. Anyway, this is expected to be missing as we don't have a way to map C++ generated code to the IDL it was generated from.

Comment 5

[Marco Castelluccio [:marco]]

All the paths seem related to tests:
file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/invalidation/filter-userspace-offset.svg?offsetContainer
=foreignObject&mask=boundingBox
file:///Z:/task_1511823081/build/tests/jsreftest/tests/test262/language/statements/const/cptn-value.js
file:///builds/worker/workspace/build/tests/reftest/tests/layout/reftests/scrolling/fractional-scroll-area.html?top=0.4&outerBottom=100.4&innerBottom=199.6

Chris, are test files handled in a different way? Do you know why they are missing from chrome-map.json?

Comment 6

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

we are also missing testing/marionette/*.js from codecov.io, but I see it in activedata- these are not mapped files as far as I can tell.  There are a lot of other testing/* files.

Comment 7

[Kyle Lahnakoski [:ekyle]]

The ActiveData-ETL FileMapper can solve most of these problems, but there are still ambiguities in a number of files. I will be tracking the ambiguities, and solutions, in the ETL code:

https://github.com/klahnakoski/ActiveData-ETL/blob/etl/activedata_etl/imports/file_mapper.py#L24

Once these exceptions have grown, they may reveal a pattern we can make a rule out of.

Comment 8

[Nicolas B. Pierron [:nbp]]

Maybe it would be possible to make 2 lists, one which contains the list of files which have no coverage, and one which contains the list of SF: fields which are not being reported.

(In reply to Joel Maher ( :jmaher) (UTC-5) from comment #1)
> another one:
> "file:///Z:/task_1511823081/build/tests/jsreftest/tests/test262/language/
> statements/const/cptn-value.js line 25 > eval"
> 
> in this case we might have to ignore this as it is an eval statement.

I will also add that testing the code coverage of the test cases might not be that interesting either.
 file:///…/build/tests/jsreftest/tests/*.js  are part of the test suite, and not the tested code.

Comment 9

[Chris Manchester (limited bugmail, email directly)]

Most of these examples have query params tacked on to the file:// url, this might just be a matter of modifying the lcov rewriter to strip these off.

The only special handling for tests I'm aware of is a separate issue for certain kinds of harness files, where the mochitest harness writes out a chrome manifest during its run, and we need to save this manifest for consumption by the lcov rewriter. I had this wired up but I don't know if that's made it to the current set up: The option to pass to the mochitest runner is "--store-chrome-manifest", which we can then pass to the lcov rewriter in "--extra-chrome-manifests". This was only for certain harness files, I don't think it covers any of the things we've mentioned so far here. I'm not sure if we could generate these manifests during the build, that would also fix this.