Skip to content

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

Added tests to confirm that the client socket helper is using client implementations correctly.

Breaking Changes

None

Additional Info

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.

Looks good, maybe better name for test file?

@knagaitsev
Copy link
Collaborator Author

@evilebottnawi What do you think I should name it? It follows conventions of the client test directory, but you are right that maybe it is too generic.

@alexander-akait
Copy link
Member

Yes, just other name, maybe more complex, because name is misleading, any ideas?

@knagaitsev
Copy link
Collaborator Author

Yes, just other name, maybe more complex, because name is misleading, any ideas?

How about socket-util.test.js?

@hiroppy
Copy link
Member

hiroppy commented Jul 3, 2019

hm, this file has already included in utils/, so I think util is unnecessary. How about socket-helper.test.js? Anyway, I want to change the file name more specific but it is difficult.

@codecov
Copy link

codecov bot commented Jul 4, 2019

Codecov Report

Merging #2095 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2095   +/-   ##
=======================================
  Coverage   94.46%   94.46%           
=======================================
  Files          32       32           
  Lines        1210     1210           
  Branches      334      334           
=======================================
  Hits         1143     1143           
  Misses         65       65           
  Partials        2        2

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 9a09420...3dc2397. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Jul 4, 2019

Codecov Report

Merging #2095 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2095   +/-   ##
=======================================
  Coverage   94.46%   94.46%           
=======================================
  Files          32       32           
  Lines        1210     1210           
  Branches      334      334           
=======================================
  Hits         1143     1143           
  Misses         65       65           
  Partials        2        2

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 9a09420...3dc2397. Read the comment docs.

@hiroppy hiroppy merged commit aa31d87 into webpack:master Jul 4, 2019
knagaitsev added a commit to knagaitsev/webpack-dev-server that referenced this pull request Jul 31, 2019
* test(client): socket helper tests

* test(client): rename socket test, rename mock variable

* test(client): renamed socket helper test filename
@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