collectCoverage breaks TypeScript source stepping

[mikebm]

Do you want to request a feature or report a bug?
BUG

What is the current behavior?
When collectCoverage is set to true, then source stepping no longer functions correctly.

Steps:
git clone https://github.com/mikebm/jestbug.git
cd jestbug
npm install
Debug the ./tests/component.test.ts file. Attempt to step through the executeSomeLogic call on test 'should return 5'.

What is the expected behavior?
When stepping through the executeSomeLogic function, your debugger should show the correct line that you are on.

Please provide your exact Jest configuration and mention your Jest, node,
yarn/npm version and operating system.
Node v8.9.0
Jest 22.4.2
npm 5.6.0

[mikebm]

Note: The repo has been updated with decorators to prove a separate ts-jest line reporting issue. This issue is still reproducible with and without the decorators.

[SimenB]

Is this still an issue with the latest Jest and ts-jest?

[mikebm]

Yes, it is still an issue. I just tested with jest 23.6.0. I upgraded the test project as well.

[SimenB]

@huafu ideas?

[huafu]

I first thought it was related to: babel/babel#6577 but after some tests doesn't seem to be (@SimenB you can confirm that bare js instrumented thru jest do can be debugged step by step when coverage is on right?). In ts-jest we do patch it, but only if babel is required. I tried to patch it beforehand without success.

We do have e2e tests running over multiple versions of jest, babel and typescript for source mapping and then coverage, but I believe we do not have for both together, will drop an eye tommo.

@mikebm can you open an issue on ts-jest with the link to your repo, I'll try to look at it asap. We're about to publish a new release but it'll wait if this is indeed ts-jest related and if it can get fixed quickly.

BTW, your jest config can be simplified a lot @mikebm:

{
  "preset": "ts-jest",
  "automock": true,
  "collectCoverage": true,
  "coverageDirectory": "<rootDir>/coverage/jest",
  "coverageReporters": ["lcov", "json"],
  "resetMocks": true,
  "resetModules": true,
  "testResultsProcessor": "jest-junit-reporter"
}

[huafu]

Ok so this is a jest bug and not ts-jest bug.

First here you're losing the input source maps in case it's not inlined but in the map property:

https://github.com/facebook/jest/blob/0cd757b71cef7aec07c9532f7b64739cc8baeb1f/packages/jest-runtime/src/script_transformer.js#L246

Then even if you had the source maps inlined, it'd be lost anyway there:

https://github.com/facebook/jest/blob/0cd757b71cef7aec07c9532f7b64739cc8baeb1f/packages/jest-runtime/src/script_transformer.js#L155-L171

...where useInlineSourceMaps is false, and even tho, no possibly generated source maps is returned.

I'll try to provide a PR here soon.

Update: the title of this issue should be: enabling coverages breaks source maps – it's not only for typescript.

[SimenB]

There's probably a reason why we don't factor in the source map in that case. @jwbay @aaronabramov do you know if there are?

[huafu]

@SimenB any news on this? I meant, I'd be happy to provide a PR but I'm not gonna try if there is a good reason for it to be like that ;-)

[SimenB]

Feel free to send a PR! I'm not sure how it works, but if we're able to show it's more correct, no reason not to merge it 🙂

[ruiaraujo]

@huafu any chance of doing the PR?

[huafu]

@ruiaraujo I actually had to run on another thing, and my project to hurry to finish. I have started a PR, AFAIR it was not as hard as I thought, ...was harder.

So no, not finished, and now can't even remember what was the issue I was stuck with.

FYI this is my branch, but I think I started again from sratch locally. Anyway, could give you the idea of what should be done.

I'm sorry it's not in my priority because you can always run without coverage on your local while debugging and add --coverage for the final run (or the unique on CI) to have coverage.

[atdiff]

Just curious if this is still being worked on. Looks like the code was refactored some over the past few months, so that the ScriptTransformer.ts is in the jest-transform package. For now will go with the workaround but would like to see an overall fix.

[mbpreble]

Looking at this a bit since I tend to run into it.
@huafu's PR is a really useful start, even if the code has moved around since.

If you wire it up to push the instrumented sourceMap into cache, you get correct stack traces for errors in the code under test.

Debugging in code under test works if you additionally request an inline sourceMap in the instrumented code (sourceMaps: 'both' instead of true).

Currently just have a mess of a local POC branch but this is working how I expect it to. Hope to get something pushed this week.

[mbpreble]

Created PR #9460 to address this - I can debug transformed typescript code under test and stack traces emitted from code under test correctly correspond to source lines. There isn't any obvious harm to existing functionality.

[SimenB]

Had to revert the fix due to a regression, so reopening this one

[mbpreble]

Pushing the source maps (between source and instrumented code with coverage enabled) to Jest's cache for non-transformed code seems to break coverage reporting.

It's certainly possible to go back to only pushing out source maps for transformed file, this line in the reverted PR could be this instead:
const shouldEmitSourceMaps = (!!transform && !!map)

This puts back coverage for untransformed files, although stack traces of errors from the files can pop out with the line numbers from the instrumented code. This seems to be on par with master, so it might be a path forward, if not ideal.

One thing I'm seeing here is that there's a mapCoverage bit being set when instrumenting non-transformed code: https://github.com/facebook/jest/blob/master/packages/jest-transform/src/ScriptTransformer.ts#L265

I can disable this here and restore coverage reporting although I don't know if this is safe to do.

[SimenB]

One thing I'm seeing here is that there's a mapCoverage bit being set when instrumenting non-transformed code

If you can remove it and coverage is still correct that sounds a good path forward. It exists exactly to map coverage correctly when source maps are in play. It used to be a top level option to jest, but was moved to be inferred in #5177. which contains a bit of useful discussion.

[mbpreble]

Opened up a new PR here, the main difference is that mapCoverage is being set to false throughout ScriptTransformer. I erred on the side of staying away from doing a deep clean or redefining types that cross module boundaries.

We seem to get all of the improvements for debugging and stack traces of the original PR, without doing harm to non-transformed coverage.

[github-actions]

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.