-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
replace mocha with jest #1650
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
replace mocha with jest #1650
Conversation
@@ -31,7 +31,7 @@ function runWebackDevServer(testArgs, configPath) { | |||
const args = [webpackDevServerPath, '--config', configPath].concat(testArgs); | |||
|
|||
return new Promise((resolve, reject) => { | |||
const child = spawn('node', args, { cwd, env }); | |||
const child = execa('node', args, { cwd, env }); |
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.
It is strange to use spawn
, the following error occurs.
spawn node ENOENT
So We will use execa.
Good job, let's wait CI green 👍 |
Codecov Report
@@ Coverage Diff @@
## master #1650 +/- ##
==========================================
+ Coverage 75.14% 82.43% +7.29%
==========================================
Files 10 15 +5
Lines 688 484 -204
Branches 0 120 +120
==========================================
- Hits 517 399 -118
+ Misses 171 68 -103
- Partials 0 17 +17
Continue to review full report at Codecov.
|
@hiroppy hm, coverage is very decrease, something wrong? |
hmmm.... I don't know that. Current setting is |
Test failed on appveyor.(Environment: nodejs_version=8, webpack_version=latest)
|
The following error occurs only at Node 8 🤔(it also happened on my mac)
|
@evilebottnawi Since I do not know the cause, I temporarily verified it. |
test/Routes.test.js
Outdated
const expected = | ||
process.version.split('.')[0] === 'v8' | ||
? 'key1=value1,key2=value2' | ||
: 'key1=value1, key2=value2'; |
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.
Maybe just remove spaces from expected? I am afraid another version can also have same problem?
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.
version | output | spaces |
---|---|---|
6 | key1=value1, key2=value2 |
y |
7 | key1=value1,key2=value2 |
n |
8 | key1=value1,key2=value2 |
n |
9 | key1=value1,key2=value2 |
n |
10 | key1=value1, key2=value2 |
y |
11 | key1=value1, key2=value2 |
y |
6, 10, 11 have the spaces but 7, 8, 9 don't have.
So, I change the code to ['v7', 'v8', 'v9'].includes
.
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.
@hiroppy Can you send this change in commit?
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.
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.
Separated this commit(10e4bc2) /cc @evilebottnawi
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.
Great job! One note
@hiroppy Good job, let's wait CI green |
@evilebottnawi Thanks for reviewing! CI is green:) |
@hiroppy let's improve CI testing https://github.com/webpack/webpack-dev-server/blob/master/.travis.yml (need add 8 and 11) https://github.com/webpack/webpack-dev-server/blob/master/appveyor.yml (need add node 11) Maybe refactor and improve config (add cache) Can you take care? |
Yep, I'll work on that:) |
* it has been deleted when webpack#1650
* it has been deleted when #1650
For Bugs and Features; did you add new tests?
Motivation / Use-Case
#1649 (comment)
Breaking Changes
Additional Info
Currently, since duplication of ports occurred, the test is executed in series.
I would like to correspond with another PR.
If all CIs pass, remove wip.