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(server): serverMode 'ws' option #2082

Merged
merged 6 commits into from
Jul 5, 2019

Conversation

knagaitsev
Copy link
Collaborator

  • This is a bugfix
  • This is a feature
  • This is a code refactor
  • This is a test update
  • This is a docs update
  • This is a metadata update

For Bugs and Features; did you add new tests?

Yes

Motivation / Use-Case

Adds the option serverMode: 'ws'. Please review and merge #2077 before this.

I still want to work on serverMode-option.test.js further such that it tests to see that Server.js interacts with a socket server implementation correctly. Currently, it only checks that Server.js calls the constructor of the server implementation. I will work on this later.

Breaking Changes

None

Additional Info

@codecov
Copy link

codecov bot commented Jul 1, 2019

Codecov Report

Merging #2082 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2082      +/-   ##
==========================================
+ Coverage   94.46%   94.47%   +<.01%     
==========================================
  Files          32       32              
  Lines        1210     1212       +2     
  Branches      334      335       +1     
==========================================
+ Hits         1143     1145       +2     
  Misses         65       65              
  Partials        2        2
Impacted Files Coverage Δ
lib/utils/getSocketServerImplementation.js 90.47% <100%> (+1%) ⬆️

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 aa31d87...2b743a8. Read the comment docs.

alexander-akait
alexander-akait previously approved these changes Jul 1, 2019
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.

/cc @hiroppy

@hiroppy
Copy link
Member

hiroppy commented Jul 1, 2019

@Loonride plz rebase

@knagaitsev
Copy link
Collaborator Author

I just added some tests to confirm through mocks that the server interacts with the socket server implementation correctly. I'm not sure if serverMode-option.test.js is the best place for it though. Maybe it should be in Server.test.js, or something else?

hiroppy
hiroppy previously approved these changes Jul 3, 2019
@hiroppy
Copy link
Member

hiroppy commented Jul 3, 2019

I think it is ok.

@alexander-akait
Copy link
Member

Something wrong with prettier, need fix

@knagaitsev
Copy link
Collaborator Author

Not sure what the problem was exactly, but seems to be fixed

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.

/cc @hiroppy

@hiroppy hiroppy merged commit 04483f4 into webpack:master Jul 5, 2019
anikethsaha pushed a commit to anikethsaha/webpack-dev-server that referenced this pull request Jul 7, 2019
* feat(server): server mode ws string option

* test(server): rearrange bad host test

* test(server): added mock server implementation tests

* test(server): remove bad host test temporarily

* test(server): re added bad host test
knagaitsev added a commit to knagaitsev/webpack-dev-server that referenced this pull request Jul 31, 2019
* feat(server): server mode ws string option

* test(server): rearrange bad host test

* test(server): added mock server implementation tests

* test(server): remove bad host test temporarily

* test(server): re added bad host test
@knagaitsev knagaitsev added gsoc Google Summer of Code scope: ws(s) labels Aug 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gsoc Google Summer of Code scope: ws(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants