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

feat: check for common errors when using the wrong test environment #8245

Merged
merged 9 commits into from
Mar 28, 2020

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Mar 30, 2019

Summary

For #5815.

This probably should not live in jest-message-util, but that was the easiest to cover exec errors (from jest-runtime) and test errors (from jest-jasmine and jest-circus). Thoughts on how to better organise it?

Also, sucky message 😅 Ideas?

Test plan

e2e tests added

@jeysal
Copy link
Contributor

jeysal commented Mar 30, 2019

Thoughts on how to better organise it?

Not sure there's a better way, if we make this a separate util and use formatExecError(checkForCommonBla(err)) in places where the error might actually be caused by forgetting to configure the correct environment, over time you'd just forget to use checkForCommonBla in places where it should be.
Could make this check optional though so places like https://github.com/facebook/jest/blob/04e6a66d2ba8b18bee080bb28547db74a255d2c7/packages/jest-runner/src/runTest.ts#L57 can opt out?

@SimenB SimenB force-pushed the check-for-common-env-errors branch from 9537dd9 to 4144568 Compare April 4, 2019 06:53
@SimenB
Copy link
Member Author

SimenB commented Apr 10, 2019

@scotthovestadt thoughts on this (beyond failing CI)?

@nickserv
Copy link
Contributor

Any help needed with this (or something else related to #5815)?

@SimenB
Copy link
Member Author

SimenB commented Mar 28, 2020

I had completely forgotten about this one, thanks for the bump! Lemme try to rebase this and move the logic to jest-runtime instead, which is where we check for syntax errors and tell the user to use transforms.

@SimenB SimenB force-pushed the check-for-common-env-errors branch from 330c27d to b68f138 Compare March 28, 2020 08:50
@codecov-io
Copy link

codecov-io commented Mar 28, 2020

Codecov Report

Merging #8245 into master will increase coverage by 0.00%.
The diff coverage is 50.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #8245   +/-   ##
=======================================
  Coverage   64.93%   64.93%           
=======================================
  Files         288      288           
  Lines       12190    12199    +9     
  Branches     3023     3024    +1     
=======================================
+ Hits         7916     7922    +6     
- Misses       3636     3638    +2     
- Partials      638      639    +1     
Impacted Files Coverage Δ
packages/jest-message-util/src/index.ts 66.89% <50.00%> (-1.46%) ⬇️
packages/expect/src/utils.ts 96.22% <0.00%> (+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 db055c2...d214b34. Read the comment docs.

@SimenB SimenB force-pushed the check-for-common-env-errors branch from 23182a9 to d214b34 Compare March 28, 2020 09:30
@SimenB
Copy link
Member Author

SimenB commented Mar 28, 2020

Thinking about it, I'll just leave it where it is, as the source of the error might be in many places it's nice to not have the logic spread around. And since the function is not exported, if we later want to move/refactor it it's not a breaking change

@SimenB SimenB merged commit 2a4b073 into jestjs:master Mar 28, 2020
@SimenB SimenB deleted the check-for-common-env-errors branch March 28, 2020 15:20
@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.

6 participants