We need to adjust based on differences in line info from #if and #include. The preprocessor leaves "//@line <number> "<file>"" comments in both cases we can use to correct line numbers. Dealing with #included files will be a more complicated, because we need to "split" lcov records, which could be tricky for summary info. jcranmer has a working prototype for rewriting line numbers within a file in https://bugzilla.mozilla.org/show_bug.cgi?id=1214885#c3

I'm going to upload what I have so far for this (used on top of bug 1214885 to produce http://people.mozilla.org/~cmanchester/jscov/ -- coverage on a linux debug build for the tests in browser/base/content/test/newtab). Not entirely sure who to ask for review on this -- I'll find someone once bug 1214885 gets closer to landing.

Mozreview associated it with the other bug because it's on top of those patches, but https://bugzilla.mozilla.org/show_bug.cgi?id=1214885#c14 is the relevant commit.

I was looking at some of the reports, and I am quite surprised by some values, such as: http://people.mozilla.org/~cmanchester/jscov/browser/base/content/test/newtab/browser_newtab_bug752841.js.gcov.html#615 The "functions" view reports functions which should apparently be part of another file. Could that be an issue with the aggregator, or is this an issue in the JS LCov implementation? Unless this is an artifact of the test suite?

(In reply to Nicolas B. Pierron [:nbp] from comment #3) > I was looking at some of the reports, and I am quite surprised by some > values, such as: > http://people.mozilla.org/~cmanchester/jscov/browser/base/content/test/ > newtab/browser_newtab_bug752841.js.gcov.html#615 > > The "functions" view reports functions which should apparently be part of > another file. Could that be an issue with the aggregator, or is this an > issue in the JS LCov implementation? Unless this is an artifact of the test > suite? Those results look merged with those for browser/base/content/test/newtab/head.js. Looking at the original .info file, the record in question starts with: SF:chrome://mochitests/content/browser/browser/base/content/test/newtab/head.js SF:chrome://mochitests/content/browser/browser/base/content/test/newtab/browser_newtab_bug752841.js FN:128,this.test FN:131,this.test/< FN:133,this.test/</< FN:141,setup FN:142,setup/< FN:143,setup/</< FN:144,cleanupAndFinish FN:145,setup/</</cleanupAndFinish/< FN:161,setup/promiseReady< FN:163,setup/promiseReady</< FN:1,top-level FN:45,top-level FN:237,getGrid FN:246,getCell The parser assumes one SF per record and attributes everything to the second file. When I run it again the results look better for the test file record, but the next record starts with: SF:chrome://mochitests/content/browser/browser/base/content/test/newtab/head.js SF:chrome://mochitests/content/browser/browser/base/content/test/newtab/head.js FN:1,top-level FN:45,top-level FN:78,top-level FN:82,top-level FN:141,setup FN:142,setup/< (two SF: entries again).

Ok, I will look into this double SF issue (Bug 1227735) with no end_of_record in between.

The "remoteenabled", and "remoterequired" flags are used by some chrome registered by test harnesses. In order to parse those manifests with mozpack, the two flags are added as permitted. Review commit: https://reviewboard.mozilla.org/r/32683/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/32683/

This script uses the info generated by the UrlMap backend to replace chrome urls found in lcov files with source-paths. There are several other cases a script might not correspond to a source url, such as "data:" uris, which are handled by omitting those sections in the produced lcov files. The set of files included by a JS source (if it is preprocessed) is also calculated, but not consumed at this time. A more extensive lcov rewriting mechanism will be added in the future to re-construct source information in produced coverage reports. Review commit: https://reviewboard.mozilla.org/r/32685/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/32685/

Review commit: https://reviewboard.mozilla.org/r/32687/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/32687/

Sam, the commit posted in comment 7 should be useful for finding source files from resource/chrome urls.

:chmanchester Are you still working on this? If so, take it back. Otherwise Greg will look at it. Thank you!

Chris said he could rebase the patches, if we were OK with the approach he took. I think the approach is fine. Another possibility (only for the rewriting of paths, not for the preprocessor-related stuff) would be to do it at runtime in the JSVM code that handles code coverage, by using nsIResProtocolHandler.

Right, I'll get these rebased if necessary and posted for review soon.

(In reply to Marco Castelluccio [:marco] from comment #11) > Chris said he could rebase the patches, if we were OK with the approach he > took. I think the approach is fine. > Another possibility (only for the rewriting of paths, not for the > preprocessor-related stuff) would be to do it at runtime in the JSVM code > that handles code coverage, by using nsIResProtocolHandler. Handling this in nsIResProtocolHandler may be worth pursuing. As I'm thinking through how this might work, the patches I have here pretty much assume the build and test are happening on the same machine, I don't know how this is going to work in automation exactly.

Comment on attachment 8712864 [details] Bug 1224691 - Add a script to re-write lcov files, replacing chrome urls with sourcefile locations. https://reviewboard.mozilla.org/r/32683/#review150404

I'm going to review the other patch tomorrow. I'm doing a high level review, but someone more comfortable with mozbuild should review too.

Comment on attachment 8712865 [details] Bug 1224691 - Parse lcov files and rewrite them based on preprocessor info. https://reviewboard.mozilla.org/r/32685/#review150804

Mike, per comment 17 can you give these a once over when you are able? Thanks

Comment on attachment 8712864 [details] Bug 1224691 - Add a script to re-write lcov files, replacing chrome urls with sourcefile locations. https://reviewboard.mozilla.org/r/32683/#review153498 ::: python/mozbuild/mozbuild/codecoverage/lcov_rewriter.py:1 (Diff revision 2) > +# This Source Code Form is subject to the terms of the Mozilla Public Where is lcov_rewriter.py actually used? The only thing I see so far is the test for it. Is it something a dev will run manually on lcov files?

Comment on attachment 8712865 [details] Bug 1224691 - Parse lcov files and rewrite them based on preprocessor info. https://reviewboard.mozilla.org/r/32685/#review153482 ::: python/mozbuild/mozbuild/codecoverage/lcov_rewriter.py:43 (Diff revision 2) > + self.branches = {} > + self.lines = {} > + > + def __iadd__(self, other): > + > + # These shouldn't differ. Is this an assumption that you need to verify? ::: python/mozbuild/mozbuild/codecoverage/lcov_rewriter.py:52 (Diff revision 2) > + self.functions.update(other.functions) > + > + for name, count in other.function_exec_counts.iteritems(): > + new_count = count > + if name in self.function_exec_counts: > + new_count = int(count) + int(self.function_exec_counts[name]) Do you expect the counts to ever not be integers? It seems this block could just be self.function_exec_counts[name] = count + self.function_exec_counts.get(name, 0)

Comment on attachment 8712864 [details] Bug 1224691 - Add a script to re-write lcov files, replacing chrome urls with sourcefile locations. https://reviewboard.mozilla.org/r/32683/#review153498 > Where is lcov_rewriter.py actually used? The only thing I see so far is the test for it. Is it something a dev will run manually on lcov files? Right, this can be used locally, or more likely in an automation job yet to be determined.

Pushed by cmanchester@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/aa2287646d68 Add a script to re-write lcov files, replacing chrome urls with sourcefile locations. r=marco,mshal https://hg.mozilla.org/integration/autoland/rev/5882727a7d16 Parse lcov files and rewrite them based on preprocessor info. r=marco,mshal

https://hg.mozilla.org/mozilla-central/rev/aa2287646d68 https://hg.mozilla.org/mozilla-central/rev/5882727a7d16