fix issue jest 14766

[cenfun]

fix issue jestjs/jest#14766

[SimenB]

that should be disabled on exit then I assume?

It seems odd to me we need to enable the debugger, tho. any immediate ideas on why?

[cenfun]

@SimenB Thanks for the reminder, it has been updated.
The idea originates from Playwright and Puppeteer

• https://github.com/microsoft/playwright/blob/main/packages/playwright-core/src/server/chromium/crCoverage.ts#L88
• https://github.com/puppeteer/puppeteer/blob/main/packages/puppeteer-core/src/cdp/Coverage.ts#L252

But I don't know why, hope someone can explain.

[kristof-mattei]

I tried to do some research as well as to why this is important. Searching GitHub reveals a whole bunch of places where Debugger.enable is combined with Profiler.enable but a reason in a commit message or so is nowhere to be found.

But to repeat @cenfun's link: https://github.com/puppeteer/puppeteer/blob/88df4075171cdd87ad60241dd77028c96b1a7d9b/packages/puppeteer-core/src/cdp/Coverage.ts#L240-L248

This is code written by the Puppeteer authors themselves (i.e. Google) and the do the same.

I also have a repro if you'd like:

https://github.com/actions-rs-plus/core.

If you clone, npm i etc and then do JEST_JUNIT_OUTPUT_DIR=reports/ JEST_JUNIT_OUTPUT_NAME=unit-tests-report.xml npm run test:ci -- --runInBand --coverage you'll see missed lines.

Apply the patch above in node_modules/collect-v8-coverage and all jumps back to 100%.

[HarelM]

@SimenB is there anything preventing this from being merged? Can I help in any way to push this forward?

[D4N14L]

@HarelM I can't speak for Simen, but in our own internal repositories we tried to implement this patch ourselves, and while it worked and fixed coverage reporting for us, it caused steep performance regressions in Jest.

[mudcovered]

@HarelM I can't speak for Simen, but in our own internal repositories we tried to implement this patch ourselves, and while it worked and fixed coverage reporting for us, it caused steep performance regressions in Jest.

What good is performance if the data produced is garbage. I'd rather have my tests run slower and have accurate results than faster tests with coverage that is meaningless.

[D4N14L]

@mudcovered sure, but, respectfully, why not expect both? Performance and correctness were once possible. Why not now? Also FWIW, you can take a less impactful hit to performance if you use the Babel coverage provider, which absolutely shouldn't be necessary if you already have transpiled JavaScript.

It's completely reasonable to expect better from an industry standard testing framework, especially since Node 18 is currently in maintenance mode and will be out of support come April.

[raphaelboukara]

Hi @SimenB
Is there a decision to merge this one actually? Or we have to find for another solution?