bcoe / c8

output coverage reports using Node.js' built in coverage
ISC License
1.98k stars 89 forks source link

Accept coverage info, collected from the browser #339

Open canonic-epicure opened 2 years ago

canonic-epicure commented 2 years ago

I'm developing a cross-platform testing tool, which can run tests in browsers, Node and Deno: https://siesta.works

I'm trying to promote c8 as a recommended code coverage tool for it. It seems to work well for the Node.js tests.

However, if I collect coverage objects from the Chromium web pages, c8 does not recognize them as such. This is because Chromium's coverage object seems to have different format from Node's.

Node's format:

{
    "result": [{
        "scriptId": "7",
        "url": "file:///home/nickolay/workspace/siesta-workspace/siesta-monorepo/packages/siesta/bin/siesta.js",
        ...
    }],
    "source-map-cache": {
        "file:///home/nickolay/workspace/siesta-workspace/siesta-monorepo/node_modules/.pnpm/@web+dev-server@0.1.24_rollup@2.58.0/node_modules/@web/dev-server/dist/index.js": {
            "lineLengths": [13, 62, 137, 51, 133, 54, 127, 61, 148, 57, 130, 65, 142, 33],
        },

Chromium's format:

[
    {
        "scriptId": "32",
        "url": "http://localhost:8000/browser.js",
        ...
        "source": "..."
    }
]

So browser's format includes "raw" sources and does not contain the results property. Data from browser is collected with Playwright: https://playwright.dev/docs/api/class-coverage

I'm willing to contribute a PR normalizing this format difference when generating a report, would you accept it and may be provide some guidance?

canonic-epicure commented 2 years ago

Hm.. It seems there's a fundamental assumption, that coverage info comes from the terminal process (http://domain/path urls are not supported at all, the node's path module is used to handle all urls). In browser case all urls are 'http'. I guess I'll have to convert the http urls to file://.

canonic-epicure commented 2 years ago

So, by changing all urls to file:// with el.url = el.url.replace(/^https?:\/\//, 'file:///'), wrapping the results from browser with { result : ... }, using --allowExternal and applying this small patch to lib/report.js: https://github.com/canonic-epicure/c8/commit/0ed7994cec03ab10052f9c974971a789a256e064

I was able to receive decently looking report: image

Sources are not shown of course: image

This makes me positive its overall doable.

bcoe commented 2 years ago

@canonic-epicure this seems like a great feature request.

I think the logic might end up complicated enough for resolving URLs, and making it fit into the assumptions made by c8, that we should probably add a new helper, something like ./lib/util/chrome-to-v8.js, which would spit out a file that then works with v8-to-istanbul?

canonic-epicure commented 2 years ago

@bcoe

I think the logic might end up complicated enough for resolving URLs

Yes, would need to generalize the path module so that it could work with http urls.

we should probably add a new helper, something like ./lib/util/chrome-to-v8.js, which would spit out a file that then works with v8-to-istanbul

Probably. I'm open for suggestions and ideas. The thing is, it seems istanbul reports also assumes that all urls are file-system. So one would need to tweak them as well.

In theory, if the "generalized path" library would be created, it would be a matter of replacing the path symbol (plus may be inserting an async point for sources fetching, right now the sourceFinder in context is a synchronous method).

Right now I was able to workaround w/o patching c8, by changing all urls to file://. So I'm replacing http://localhost:8000/path/file.js to file://localhost/8000/path/file.js. Then I'm overriding the _getSourceMap method:

                    c8Report._getSourceMap = function (v8ScriptCov) {
                        return { source : v8ScriptCov.source }
                    }

and also run method (to add custom sourceFinder):

                    c8Report.run = async function () {
                        const context = istanbulLibReport.createContext({
                            // custom source finder
                            sourceFinder    : (url : string) => launcher.getSourcesOfCoverageFile(url),
                            dir             : this.reportsDirectory,
                            ...
                    }

Also need to enable allowExternal config, otherwise TestExclude tries to perform file-system resolution. And it all seems to work, just feels a bit hackish (and can show strange urls if port is included).

If you think its a sane feature request, I'm ready to contribute all the necessary code (after we come up with a plan).

bcoe commented 2 years ago

The thing is, it seems istanbul reports also assumes that all urls are file-system. So one would need to tweak them as well.

Updating Istanbul reports to not assume file system paths might be a good starting point. I think the reports most likely to make this assumption would be the HTML reports, which actually load files off disk I believe πŸ€”

If you think its a sane feature request, I'm ready to contribute all the necessary code (after we come up with a plan).

I think this sounds like a reasonable requests, users often ask how to handle browser coverage with c8, and there hasn't ever been a good answer.

With regards to coming up with a plan, perhaps a good starting point would be to draft a markdown RFC with a rough plan.

canonic-epicure commented 2 years ago

With regards to coming up with a plan, perhaps a good starting point would be to draft a marddown RFC with a rough plan.

Ok, I'll be preparing one gradually. Will let you know.

prantlf commented 1 year ago

I was able to use the raw script coverage generated by puppeteer together with c8 report. My tests run in multiple pages served at http://localhost:5000:

// enable collecting JavaScript code coverage with raw coverage
await page.coverage.startJSCoverage({
  resetOnNavigation: false,
  includeRawScriptCoverage: true
})

// load one or more pages and run tests in them
...

// collect JavaScript code coverage
const coverage = await page.coverage.stopJSCoverage()

// extract raw script coverage with URLs replaced with absolute paths
const urlBase = 'http://localhost:5000/'
const cwd = process.cwd()
const result = coverage.map(({ rawScriptCoverage }) => {
  rawScriptCoverage.url = rawScriptCoverage.url.replace(urlBase, cwd)
  return rawScriptCoverage
})

// write the coverage like c8 expects it
await mkdir('coverage/tmp', { recursive: true })
await writeFile(`coverage/tmp/out.json`, JSON.stringify({ result }))

If you want to evaluate the coverage with c8 and not with nyc, there's no need to convert it with puppeteer-to-istanbul or v8-to-istanbul.

Cactusbone commented 1 year ago

Hello ! I'm trying to get c8 to report coverage from Nightwatch.js

From what I understand in this issue, what's missing is the sourcemap from url to local files. From the first post, I believe the source-map-cache may be missing, however I do have a result property is the data from Chrome browser (using devtools-protocol).

First of all, should I create a new ticket for that ?

What would be the best way to get that sourcemap ?

I'm thinking of two ways, either I listen to Debugger.scriptParsed events from devtools-protocol, and use the sourceMapURL to match the scriptId (looks like I'm out of luck https://github.com/SeleniumHQ/selenium/issues/10804). Or I can download the bundles from url in coverage output and then source maps from comments.

Does this looks like a good way to get usage coverage ? Am I missing something ? any tips ?

Thanks!

prantlf commented 1 year ago

@Cactusbone, aren't the source maps in the file system together with the bundled assets? You shouldn't need to fetch them using the browser devtools protocol. I use the code above to catch code coverage in the source files, with the help of the source maps:

Sources:

src
β”œβ”€β”€ bundled-engine.js
β”œβ”€β”€ graph.js
β”œβ”€β”€ index.js
β”œβ”€β”€ renderer.js
β”œβ”€β”€ script-editor.js
└── separate-engine.js

Build output:

dist
β”œβ”€β”€ index.min.js      (ends with "//# sourceMappingURL=index.min.js.map")
└── index.min.js.map  (contains source paths like ../src/graph.js)

Test run:

❯ node test  (writes coverage/tmp/out.json)

Configuration in package.json:

"c8": {
  "excludeAfterRemap": true,
  "exclude": [
    "src/codejar",
    "src/prism"
  ],
  "reporter": [
    "lcov",
    "text"
  ],
  "checkCoverage": true,
  "branches": 100,
  "functions": 100,
  "lines": 100,
  "statements": 100
}))

Coverage check:

❯ npx c8 report
--------------------|---------|----------|---------|---------|-------------------
File                | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
--------------------|---------|----------|---------|---------|-------------------
All files           |    98.4 |      100 |     100 |    98.4 |
 bundled-engine.js  |     100 |      100 |     100 |     100 |
 graph.js           |     100 |      100 |     100 |     100 |
 renderer.js        |     100 |      100 |     100 |     100 |
 script-editor.js   |   96.84 |      100 |     100 |   96.84 | 71-76
 separate-engine.js |     100 |      100 |     100 |     100 |
--------------------|---------|----------|---------|---------|-------------------
ERROR: Coverage for lines (98.4%) does not meet global threshold (100%)
ERROR: Coverage for statements (98.4%) does not meet global threshold (100%)

(excludeAfterRemap doesn't work well, but it's another story...)

Cactusbone commented 1 year ago

@prantlf Thanks a lot ! This is what I was missing :)

Since we're not storing either the bundles nor the sources on the filesystem, It wasn't working for me (and the raw script coverage mentions in this ticket misled me). I'll see what I can do (either store them on the filesystem, or inline them using "source-map-cache").

Cactusbone commented 1 year ago

Yes, it works !

I had to save both the bundles and its map at the root of the projet, and add the bundles to my include section too.

However, with or without excludeAfterRemap, I get 100% coverage of way too many files (just like the issue you opened about excludeAfterRemap). I'll look into it :)

and from the code, it's not possible, for example with a single passage in a function containing a if/else statement, I get both the if and the else paths covered

image

Aghassi commented 2 months ago

I just wanted to chime in here that I had to drop C8 in favor of monocart-coverage-report to get v8 + lcov generation working. c8 seems to be adopting this now which is good. For my use case, I needed more leverage over how and when reports were generated, hence I opted to use monocart directly. I also had to sanitize the lcov to remove files that were generated and didn't map back to real files.