Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate --coverage support #6

Open
nicolo-ribaudo opened this issue Apr 3, 2022 · 11 comments · May be fixed by #13
Open

Investigate --coverage support #6

nicolo-ribaudo opened this issue Apr 3, 2022 · 11 comments · May be fixed by #13

Comments

@nicolo-ribaudo
Copy link
Owner

It's possible that this is already working, because Babel uses this runner and collects a coverage when running it's tests: https://github.com/babel/babel/runs/5789483528. If it already works great, I just need to document it.

It's also possible that it's only working partially, or that it's not working at all and that Babel's coverage reporting is based on something else.

(cc @psychobolt - jestjs/jest#9430 (comment))

@SimenB
Copy link
Contributor

SimenB commented Apr 4, 2022

I would assume coverage doesn't work, but I've been wrong before! 😀 However, adding support for v8 based coverage should be straight forward. E.g. use https://github.com/SimenB/collect-v8-coverage (what Jest uses) and store it as v8Coverage on the test result: https://github.com/facebook/jest/blob/80cf80055682919251b9567ef482755d5f060d47/packages/jest-runner/src/runTest.ts#L338.

@nicolo-ribaudo
Copy link
Owner Author

Thank you!

@fisker
Copy link
Contributor

fisker commented Apr 4, 2022

I don't think the coverage works, I added c8 when using in Prettier, https://github.com/prettier/prettier/blob/3cbee742a792413211eea1e4fe2d0dad26ff37ea/.github/workflows/dev-test.yml#L79

It will be great if we can make it work.

@psychobolt
Copy link

psychobolt commented Apr 4, 2022

Yes I assumed --coverageProvider=v8 would work. However, that seems to not work as well.

@SimenB
Copy link
Contributor

SimenB commented Apr 4, 2022

Yeah, runner needs to hook it up. But should as mentioned be very little code 🤞

@fisker
Copy link
Contributor

fisker commented Apr 6, 2022

This CI log maybe useful, I got an error message here

Details
ERROR: Cannot assign to read only property 'message' of object 'SyntaxError: import.meta may appear only with 'sourceType: "module"' (293:39)
Consider renaming the file to '.mjs', or setting sourceType:module or sourceType:unambiguous in your Babel config for this file.'
STACK: TypeError: Cannot assign to read only property 'message' of object 'SyntaxError: import.meta may appear only with 'sourceType: "module"' (293:39)
Consider renaming the file to '.mjs', or setting sourceType:module or sourceType:unambiguous in your Babel config for this file.'
    at parser (/home/runner/work/prettier/prettier/node_modules/@babel/core/lib/parser/index.js:87:21)
    at parser.next (<anonymous>)
    at parse (/home/runner/work/prettier/prettier/node_modules/@babel/core/lib/parse.js:31:37)
    at parse.next (<anonymous>)
    at evaluateSync (/home/runner/work/prettier/prettier/node_modules/gensync/index.js:251:28)
    at sync (/home/runner/work/prettier/prettier/node_modules/gensync/index.js:89:14)
    at getAst (/home/runner/work/prettier/prettier/node_modules/istanbul-lib-instrument/src/read-coverage.js:16:12)
    at readInitialCoverage (/home/runner/work/prettier/prettier/node_modules/istanbul-lib-instrument/src/read-coverage.js:31:17)
    at generateEmptyCoverage (/home/runner/work/prettier/prettier/node_modules/@jest/reporters/build/generateEmptyCoverage.js:155:72)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

@fisker
Copy link
Contributor

fisker commented Apr 6, 2022

And the error disappear when add coverageProvider: 'v8', log

@nicolo-ribaudo
Copy link
Owner Author

nicolo-ribaudo commented Apr 6, 2022

Yeah, the error is probably caused by istanbul/nyc reading coverage data that does not exist (I'm still wondering how Babel's coverage currently works 🤷)

EDIT I didn't see you were using c8.

@fisker
Copy link
Contributor

fisker commented Apr 6, 2022

I wounder why this error been swallowed? #6 (comment)

@SimenB SimenB linked a pull request Apr 6, 2022 that will close this issue
@SimenB
Copy link
Contributor

SimenB commented Apr 6, 2022

See #13, I think that's working 🙂

@ticup
Copy link

ticup commented Nov 24, 2022

Hi all, this might be obvious for some, but maybe not for others, so I'm leaving it here.

The jest-light-runner decreased our running time by almost a 5-fold, which is a tremendous performance increase, thanks! 🎉

We have a big typescript repo that's compiled to js using tsc and we couldn't get the coverage to work trying the proposed on-the-fly babel-register-esm loader option.
Having installed the required packages, created the babel config and running:

NODE_OPTIONS='--loader=babel-register-esm' jest --collectCoverage

We still get no coverage, even though the loader is clearly running:
Screenshot 2022-11-24 at 17 36 51
Screenshot 2022-11-24 at 17 36 33

As an alternative, we found we can use the v8 instrumentation and c8 outside of jest as follows:

npx c8 npx jest

Our jest config looks something like this (in its essence):

{
  runner: "jest-light-runner",
  testEnvironment: 'node',
  testMatch: ["**/?(*.)+(unit.spec).js"],
  roots: ["<rootDir>/dist"]
}

And a .c8rc.json:

{
  "src": "/src",
  "exclude": [
    "**/node_modules/**"
  ],
  "all": true,
  "extension": ".js"
}

Using v8 coverage does come with it's downsides though: jestjs/jest#11188
but this gives us an option for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants