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

No way to add custom instrumentation or pre-instrument code for coverage reporting #9233

Closed
agilgur5 opened this issue Nov 27, 2019 · 7 comments

Comments

@agilgur5
Copy link

agilgur5 commented Nov 27, 2019

This is somewhere in between a bug and a feature (and maybe a question if I just haven't been able to find docs for doing custom instrumentation). It seems somewhat related to #8938 , #6522 , and #6242 which seem to all have issues with the fact that the transforms used for coverage can't be overridden.

🐛 Bug Report

When running jest --coverage, jest will instrument code with istanbul-lib-instrument on its own and does not seem to allow for pre-instrumented code or custom instrumentation.

For my specific case, I need the instrumenter to exclude a gigantic vendored file (ammo.js) that often dramatically slows things down or causes gc errors and later crashes for parsers. jest --coverage seems to error out on it when parsing a browserified version of it with "Infinite cycle detected". But the test passes fine with plain jest.
(The webpack test also passes fine for both jest and jest --coverage; I'm not totally sure why, but in the webpack case, the worker isn't in-lined, it's in a separate file. But I would think the same error would happen when instrumenting the worker file??)

When I manually instrument it myself with babel-plugin-istanbul, I make sure to ignore / exclude ammo.js from the instrumentation (if it's included it usually either runs very slowly or gets a bunch of gc errors and then crashes).

To Reproduce

Steps to reproduce the behavior:

  1. Clone agilgur5/physijs-webpack#jest-electron
  2. npm install
  3. npm test to see that tests pass normally with plain jest
  4. npx jest --coverage to see the webpack test pass and the browserify test take 15+ seconds to return an "Infinite cycle detected" error coming from istanbul-lib-instrument / @babel/traverse

Expected behavior

Jest allows you to add custom instrumentation to the transformer it runs for coverage. Currently there doesn't seem to be any way to override the transforms it makes during coverage (at least as far as I can tell).

Alternatively, Jest should use any existing pre-instrumentation it finds in files. i.e. if my file was pre-instrumented with babel-plugin-istanbul, jest --coverage should use that information and not create its own (it should just get usage info and report it against the existing pre-instrumentation). Jest's instrumentation could be disabled with a flag / config option.

Link to repl or repo (highly encouraged)

Per above, agilgur5/physijs-webpack#jest-electron is a fairly minimal reproduction. The source is basically just webpack.js, browserify.js, and browserify-worker-stub.js, which are each ~5-10 LoC, and their respective tests, which are also like ~5 LoC (everything in physijs/ is mostly vendored code, with physijs/vendor/ammo.js being the aforementioned gigantic file).

Oddly enough, agilgur5/physijs-webpack#coverage-jest exhibits the same error, despite the browserify test erroring out with plain jest (hence electron runner branch above). While jest (npm test) actually throws an error, jest --coverage (npm run coverage) similarly shows the "Infinite cycle detected" error.

You can see an example of code being pre-instrumented in the agilgur5/physijs-webpack#cypress-coverage branch in cypress/plugins/index.js (~40 LoC of config). I couldn't do the same thing in Jest, but was able to do it with Cypress.

envinfo

  System:
    OS: macOS High Sierra 10.13.6
    CPU: (4) x64 Intel(R) Core(TM) i5-4278U CPU @ 2.60GHz
  Binaries:
    Node: 10.16.0 - ~/.nvm/versions/node/v10.16.0/bin/node
    Yarn: 1.19.1 - ~/.nvm/versions/node/v10.16.0/bin/yarn
    npm: 6.13.1 - ~/.nvm/versions/node/v10.16.0/bin/npm
  npmPackages:
    jest: ^24.9.0 => 24.9.0 
@codeclown
Copy link

Even on a plain installation of jest without any transforms, importing a huge module like ammo.js causes the tests run very slowly. Reproducible cases in Details:

Prerequisites

In an empty folder, install Jest and nothing else:

$ npm install jest

Create file without-ammo.spec.js:

describe('without-ammo', () => {
  it('works', () => {
    expect(1).toEqual(1);
  });
});

Run it with Jest:

$ time ./node_modules/.bin/jest without-ammo.spec.js
 PASS  ./without-ammo.spec.js
  without-ammo
    ✓ works (4 ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        1.197 s
Ran all test suites matching /without-ammo.spec.js/i.

real	0m1.897s
user	0m1.746s
sys	0m0.381s

Download ammo.js to the folder.

$ curl https://raw.githubusercontent.com/kripken/ammo.js/4aec847448ae302f7e1899bfefdfc018eacc20d4/builds/ammo.js > ammo.js

Create file with-ammo.spec.js:

describe('with-ammo', () => {
  beforeAll(() => require('./ammo.js'));
  it('works', () => {
    expect(1).toEqual(1);
  });
});

Run it with Jest:

$ time ./node_modules/.bin/jest with-ammo.spec.js
 PASS  ./with-ammo.spec.js (22.375 s)
  with-ammo
    ✓ works (6 ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        23.121 s
Ran all test suites matching /with-ammo.spec.js/i.

real	0m24.571s
user	0m34.298s
sys	0m2.263s

@github-actions
Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Feb 17, 2023
@agilgur5
Copy link
Author

This has never gotten a response....

@github-actions github-actions bot removed the Stale label Feb 17, 2023
@mrazauskas
Copy link
Contributor

mrazauskas commented Feb 18, 2023

Is this a problem with v8 coverage provider as well?

There is a plan to remove babel-plugin-istanbul from Jest at some point (see #13461).

Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Feb 18, 2024
Copy link

This issue was closed because it has been stalled for 30 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 19, 2024
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants