Skip to content

test: enable tests for webpack-dev-server@beta-1 #2573

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

Merged
merged 26 commits into from
Apr 23, 2021

Conversation

snitin315
Copy link
Member

What kind of change does this PR introduce?
tests

Did you add tests for your changes?
yes
If relevant, did you update the documentation?
No
Summary

Does this PR introduce a breaking change?
No

Other information
No

@snitin315 snitin315 requested a review from a team as a code owner March 28, 2021 07:59
@snitin315 snitin315 marked this pull request as draft March 28, 2021 08:27
@alexander-akait
Copy link
Member

We need update snapshot resolver to include webpack-dev-server version too (otherwise snapshots will be broken), also some tests should be rewritten like isDevServer3 and isDevServer4

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need snapshot resolver here, for webpack-dev-server v3 and webpack-dev-server v4, and for webpack v4 and v5

@snitin315
Copy link
Member Author

Yep WIP.

@snitin315 snitin315 force-pushed the test/dev-server branch 2 times, most recently from 28e9d62 to cb804c1 Compare April 5, 2021 08:49
@codecov
Copy link

codecov bot commented Apr 5, 2021

Codecov Report

Merging #2573 (7d42899) into master (40b2c59) will increase coverage by 0.50%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2573      +/-   ##
==========================================
+ Coverage   95.20%   95.70%   +0.50%     
==========================================
  Files          30       30              
  Lines        1501     1491      -10     
  Branches      429      429              
==========================================
- Hits         1429     1427       -2     
+ Misses         72       64       -8     
Impacted Files Coverage Δ
packages/serve/src/startDevServer.ts 91.22% <0.00%> (-1.76%) ⬇️
packages/webpack-cli/lib/webpack-cli.js 96.81% <0.00%> (+0.75%) ⬆️
packages/serve/src/index.ts 94.73% <0.00%> (+3.50%) ⬆️

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 40b2c59...7d42899. Read the comment docs.

@alexander-akait
Copy link
Member

alexander-akait commented Apr 5, 2021

Ideally we should run it on the next beta release (i.e. beta-2), I will do release and update snapshots, and all should work fine, thanks for helping

@alexander-akait
Copy link
Member

I will finish it, anyway good job, big thanks

@alexander-akait alexander-akait marked this pull request as ready for review April 7, 2021 14:22
@alexander-akait
Copy link
Member

Again, tests with changing directory and do not return to default state...

@alexander-akait
Copy link
Member

@snitin315 Do you have idea where we have tests which change process.cwd()?

@snitin315
Copy link
Member Author

No idea, from the logs it seems loader and plugin tests might be the culprits.

Screenshot 2021-04-08 at 3 23 12 PM

Screenshot 2021-04-08 at 3 23 28 PM

@anshumanv
Copy link
Member

lets try to skip them and see if CI passes

@alexander-akait
Copy link
Member

I can't understand which part of code change our cwd 😞

@alexander-akait
Copy link
Member

alexander-akait commented Apr 9, 2021

Found it is in packages/generators/__tests__

@webpack/cli-team We need rewrite these test on standard tests, somebody want to help? We need do it in other PR

@alexander-akait
Copy link
Member

due yeoman-test

@alexander-akait
Copy link
Member

I just drop them to test it here

package.json Outdated
@@ -82,9 +82,9 @@
"ts-jest": "^26.4.3",
"ts-node": "^9.1.1",
"typescript": "^4.1.3",
"webpack": "^5.25.0",
"webpack": "^4.46.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this shouldn't change

@anshumanv
Copy link
Member

need to update some snapshots, I think it looks good now

@alexander-akait
Copy link
Member

Merging is hell

@alexander-akait
Copy link
Member

Hope all will be fine now and we can finish it, found some bug on webpack-dev-server side

@alexander-akait alexander-akait merged commit a56761e into master Apr 23, 2021
@alexander-akait alexander-akait deleted the test/dev-server branch April 23, 2021 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants