-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
test: make use of async/await #1996
Conversation
Codecov Report
@@ Coverage Diff @@
## next #1996 +/- ##
=====================================
Coverage 92.4% 92.4%
=====================================
Files 28 28
Lines 1067 1067
Branches 324 324
=====================================
Hits 986 986
Misses 77 77
Partials 4 4 Continue to review full report at Codecov.
|
This comment has been minimized.
This comment has been minimized.
"presets": ["@babel/preset-env"], | ||
"env": { | ||
"test": { | ||
"plugins": ["@babel/plugin-transform-runtime"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jest uses babel as the default, so we need regeneratorruntime
to use async/await. If we want to use the native async/await, we have to set current
using @babel/env
or reset transform
.
Note: babel-jest is automatically installed when installing Jest and will automatically transform files if a babel configuration exists in your project. To avoid this behavior, you can explicitly reset the transform configuration option:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need this plugin for async/await:
- It is inside
@babel/preset-env
- We use next branch, in node@8 it is built-in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jest doesn't use babel as the default, we need babel-jest
package for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, if we have .babelrc
, jest uses babel-jest
. See the documentation.
The section is Making your Babel config jest-aware
.
In fact, we can use for-await-of
with our current configuration. This is the feature that is only originally supported for v10 and higher.
We can set transform: {} for stopping to use babel, but we've already used ESM in client-src, so we should use babel in jest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
client-src/clients/BaseClient.js
Outdated
@@ -1,5 +1,3 @@ | |||
'use strict'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Affected by "sourceType": "module"
in ./client-src/.eslintrc
32e0047
to
51e4caf
Compare
This comment has been minimized.
This comment has been minimized.
51e4caf
to
91eec73
Compare
91eec73
to
0e0c340
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good, one note
I want to wait for #2005 merged |
For Bugs and Features; did you add new tests?
modify only
Motivation / Use-Case
We don't support Node6, so we can use async/await.
Breaking Changes
no
Additional Info
Next step, I'll refactor test/run-server and delete TODO.