Open Bug 1421633 Opened 3 years ago Updated 2 years ago
_rewriter .py is missing out on some file:/// paths
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
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.
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
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.
(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.
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?
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.
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.
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.
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.
You need to log in before you can comment on or make changes to this bug.