An attempt to fix coverage after updating Jest to 25.2.0

[thorn0]

Seems like the coverage wasn't collected because of transform:{} in jest.config.js.
Any idea why we needed this line there? Apparently, it disables the Babel transform, so we probably should disable it conditionally or use coverageProvider: "v8": #7897

closes #7893

[SimenB]

Hey! This wasn't supposed to change with 25.2. We need Babel (or v8) to instrument code, but that should happen regardless of a transformer being defined or not. It's late here now, but I can investigate tomorrow

[SimenB]

The issue is this line: https://github.com/facebook/jest/blob/2c666c21c6efc1fcadaf820cc9b47bf0ac10505a/packages/jest-transform/src/ScriptTransformer.ts#L230

Removing it, or forcing it to false, fixes the issue. It was added in jestjs/jest#9460

[thorn0]

@SimenB It was late too, I couldn't make sure it's indeed a bug to report. Would you recommend switching to coverageProvider: "v8" on Node 13 or should we stick with Babel for now?

[SimenB]

The V8 report is faster, while the Babel one is more accurate. I'd take a look at the html report locally and compare them, and decide based on that 🙂

[thorn0]

E.g. V8 reports that the line 212 is covered now whereas according to Babel it's not. Whom should I trust? :)

[SimenB]

It's half red, not sure what that means?

In general I'd be more trustful of the Babel report as it has existed for longer and is more well-tested. If you're able to find places where the V8 report is wrong though, an issue would be awesome so we could fix it 👍

[thorn0]

These pages compare coverage for two given commits. Half-red means the line was not covered in the older commit but is covered now. The older report in both cases is the same and was generated by Babel.

[SimenB]

Aha. Probably wrong then 😀 If you could reduce it down and open up a bug report that'd be awesome. Probably something for v8-to-istanbul, but might be the implementation in Jest as well, so let's start there

[thorn0]

So, sticking with Babel then.

If you could reduce it down and open up a bug report that'd be awesome.

Added this to my OSS to-do list.

BTW, what should actually mean "covered" in this specific case?

• "() => false was the result of the conditional operator",
• "() => false was called",
• either of the above happened,
• both?

[SimenB]

I think ideally covered should mean "both" - coverage should be able to differentiate between "else case in ternary hit" and "function defined was called".

[thorn0]

superseded by #7901

Thanks @SimenB !