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

fix: React inlinesSnapshots and corruption in jest --coverage #34

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

semoal
Copy link

@semoal semoal commented May 7, 2021

Resolves #33 and #32

Actually tried this changes in my codebase and I've got the expected results, also tried with your local tests and inlineSnapshoting was failing and nows works nice, also coverage was failing and now it's good :)

Found problems

1º Problem: inlineSnapshoting looks to need sourcemaps or some connection between the original file without the transform to know where he has to introduce the inlineSnapshot.
2º Problem: jest --coverage probably worked nice on projects without React, but the actual babel-jest has as dependency when using it with React, @babel/preset-react and @babel/preset-env, so every test with React acts strange..
3º Problem: also was using the babel-jest attribute practically always, because that indexOf("ock(") has no sense, ther's a lot of probability to contain that keywords in the source code without using jest.mock

Solutions

1º Solution to 1º problem: enable sourcemaps by default
2º Solution to 2º problem: remove the commonjs transformer and use the recommended settings by babel-jest.
3º Solution to 3º problem: indexOf to jest.mock

@aelbore
Copy link
Owner

aelbore commented May 8, 2021

@semoal thanks for your PR, it would be good if you can add tests and run the test if everything is working fine. thanks

@semoal
Copy link
Author

semoal commented May 8, 2021

Hello Jay, actually your tests are working fine and before they weren't. Also the coverage was failing and wasn't being tested on the CI, so i've added it to the CI and enabled to run on pullrequest.

 PASS  tests/index.spec.ts
 PASS  tests/react-js.spec.js
 PASS  tests/react-ts-mock.spec.tsx
 PASS  tests/react-ts.spec.tsx
 PASS  tests/react-jsx.spec.jsx
 PASS  examples/names-ts/index.spec.ts
 PASS  examples/ts-jsconfig-paths/app/index.spec.ts

Test Suites: 7 passed, 7 total
Tests:       10 passed, 10 total
Snapshots:   6 passed, 6 total
Time:        9.625 s
---------------------------------|---------|----------|---------|---------|-------------------
File                             | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
---------------------------------|---------|----------|---------|---------|-------------------
All files                        |     100 |       75 |     100 |     100 |
 examples/names-ts               |     100 |      100 |     100 |     100 |
  index.ts                       |     100 |      100 |     100 |     100 |
  names.ts                       |     100 |      100 |     100 |     100 |
 examples/react-js               |     100 |      100 |     100 |     100 |
  App.js                         |     100 |      100 |     100 |     100 |
 examples/react-jsx              |     100 |      100 |     100 |     100 |
  App.jsx                        |     100 |      100 |     100 |     100 |
 examples/react-ts               |     100 |      100 |     100 |     100 |
  App.tsx                        |     100 |      100 |     100 |     100 |
 examples/ts-jsconfig-paths/app  |     100 |      100 |     100 |     100 |
  index.ts                       |     100 |      100 |     100 |     100 |
 examples/ts-jsconfig-paths/libs |     100 |      100 |     100 |     100 |
  calculator.ts                  |     100 |      100 |     100 |     100 |
 src                             |     100 |       75 |     100 |     100 |
  index.ts                       |     100 |    73.08 |     100 |     100 | 20-23,46,48-49
  options.ts                     |       0 |        0 |       0 |       0 |
  transformer.ts                 |     100 |      100 |     100 |     100 |
  utils.ts                       |     100 |      100 |     100 |     100 |
---------------------------------|---------|----------|---------|---------|-------------------

Test Suites: 7 passed, 7 total
Tests:       10 passed, 10 total
Snapshots:   6 passed, 6 total
Time:        7.73 s

@aelbore
Copy link
Owner

aelbore commented May 10, 2021

@semoal Thanks for this

Resolves #33 and #32

Actually tried this changes in my codebase and I've got the expected results, also tried with your local tests and inlineSnapshoting was failing and nows works nice, also coverage was failing and now it's good :)

Found problems

1º Problem: inlineSnapshoting looks to need sourcemaps or some connection between the original file without the transform to know where he has to introduce the inlineSnapshot.

I think you can always put the sourcemap: true in the jest.config file

2º Problem: jest --coverage probably worked nice on projects without React, but the actual babel-jest has as dependency when using it with React, @babel/preset-react and @babel/preset-env, so every test with React acts strange..

i tried on my local the results are almost the same without the babel reactl presets
image

3º Problem: also was using the babel-jest attribute practically always, because that indexOf("ock(") has no sense, ther's a lot of probability to contain that keywords in the source code without using jest.mock

I think this is a good idea, thank you

Solutions

1º Solution to 1º problem: enable sourcemaps by default
2º Solution to 2º problem: remove the commonjs transformer and use the recommended settings by babel-jest.
3º Solution to 3º problem: indexOf to jest.mock

also thank you for have the pull_request in the ci

@semoal
Copy link
Author

semoal commented May 10, 2021

Which version of Node do you use (I use 12, just tried with 14 and also fails)?
I've run:

git checkout master
rm -rf node_modules/ yarn.lock
yarn install
# postinstalls runs yarn build
yarn test --coverage

And the suite fails hardly:

 FAIL  tests/react-js.spec.js
  ● Test suite failed to run

    SyntaxError: /Users/sergio.moreno/OSS/esbuild-jest/examples/react-js/App.js: Missing semicolon. (4:19)

      2 |
      3 | export default function App() {
    > 4 |   return <div>hello world!</div>
        |                    ^
      5 | }

and:

 FAIL  tests/index.spec.ts
  ● Test suite failed to run

    Jest: Couldn't locate all inline snapshots.

      at Object.parse (node_modules/jest-snapshot/build/InlineSnapshots.js:267:11)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Coverage differs a lot from the babel coverage and crashes on .jsx files
2 participants