-
Notifications
You must be signed in to change notification settings - Fork 51
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
base: master
Are you sure you want to change the base?
Conversation
@semoal thanks for your PR, it would be good if you can add tests and run the test if everything is working fine. thanks |
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 |
@semoal Thanks for this
also thank you for have the |
Which version of Node do you use (I use 12, just tried with 14 and also fails)? git checkout master
rm -rf node_modules/ yarn.lock
yarn install
# postinstalls runs yarn build
yarn test --coverage And the suite fails hardly:
and:
|
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