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

Produce source maps when instrumenting code #9460

Merged
merged 3 commits into from Jan 25, 2020

Conversation

mbpreble
Copy link
Contributor

@mbpreble mbpreble commented Jan 24, 2020

Summary

This change generates inline and separate file source maps for the instrumented code produced when Jest runs with --coverage in the case that the code has been transformed.

While coverage reporting appears to function correctly in this case prior to this PR, debugging code under test did not work as expected, nor did stack traces emitted by errors in code under test.

Fixes #5739

Test plan

I primarily tested this by running Jest against a small test project (https://github.com/mbpreble/jest-test)

Running my local Jest branch against the project produces the expected coverage output and stack trace for the thrown error in the test code. I am also able to debug Jest using the Node debugger and hit break points in my test code:

 FAIL  ../test-project/src/app.test.ts
  hello world
    ✕ should say hello world (3ms)

  ● hello world › should say hello world

    hello world

       6 | 
       7 | export const helloWorld =  () => {
    >  8 |     throw new Error('hello world')
         |           ^
       9 | };
      10 | 
      11 | export const uncoveredCode = () => {

      at Object.<anonymous>.exports.helloWorld (app.ts:8:11)
      at Object.<anonymous> (app.test.ts:5:16)

----------|---------|----------|---------|---------|-------------------
File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
----------|---------|----------|---------|---------|-------------------
All files |      75 |      100 |      50 |      75 |                   
 app.ts   |      75 |      100 |      50 |      75 | 12                
----------|---------|----------|---------|---------|-------------------
Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        2.358s
Ran all test suites.

image

Unit tests have been added/modified to reflect the new source map behavior.

@codecov-io
Copy link

codecov-io commented Jan 24, 2020

Codecov Report

Merging #9460 into master will increase coverage by 0.03%.
The diff coverage is 84.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9460      +/-   ##
==========================================
+ Coverage   64.96%   64.99%   +0.03%     
==========================================
  Files         283      283              
  Lines       12104    12108       +4     
  Branches     2990     2992       +2     
==========================================
+ Hits         7863     7870       +7     
+ Misses       3604     3603       -1     
+ Partials      637      635       -2
Impacted Files Coverage Δ
packages/jest-transform/src/ScriptTransformer.ts 69.64% <84.61%> (+1%) ⬆️
packages/expect/src/utils.ts 96.22% <0%> (+1.25%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8607684...7c01589. Read the comment docs.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thank you! I left a couple of nits but overall this LGTM 👍

@mbpreble
Copy link
Contributor Author

Thanks for the approval, expect to be able to work through the feedback over the next few days.

@mbpreble
Copy link
Contributor Author

Pushed a cleanup commit addressing review feedback.

@SimenB SimenB changed the title Produce source maps when instrumenting code (#5739) Produce source maps when instrumenting code Jan 25, 2020
@SimenB SimenB merged commit 03a7877 into jestjs:master Jan 25, 2020
@SimenB
Copy link
Member

SimenB commented Jan 25, 2020

Thank you! A long-standing issue, great that it's finally fixed 👍

return code;
}
if (result && result.code) {
return result as TransformResult;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could create a type guard function and avoid the cast, but it's not any more type safe. so meh

SimenB added a commit to SimenB/jest that referenced this pull request Mar 29, 2020
SimenB added a commit to SimenB/jest that referenced this pull request Mar 29, 2020
SimenB added a commit to SimenB/jest that referenced this pull request Mar 29, 2020
@SimenB
Copy link
Member

SimenB commented Mar 29, 2020

I'm going to revert this in #9724. @mbpreble happy to re-land this if you're able to fix it! 🙂

@github-actions
Copy link

This pull request 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 May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

collectCoverage breaks TypeScript source stepping
4 participants