FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: test various calling patterns of net.connect() related functions by joyeecheung · Pull Request #11847 · nodejs/node · GitHub

/ node Public

test: test various calling patterns of net.connect() related functions - #11847

Closed
joyeecheung wants to merge 5 commits into
nodejs:masterfrom
joyeecheung:net-connect-test
ConversationCommits (5)ChecksFiles changed
Closed

test: test various calling patterns of net.connect() related functions#11847
joyeecheung wants to merge 5 commits into
nodejs:masterfrom
joyeecheung:net-connect-test

Conversation

joyeecheung commented Mar 14, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

Related: #11761
This PR adds coverage to more calling patterns of net.connect:

  • Cover the calling pattern net.connect(), net.createConnection(), new Socket().connect() with/without callback, with port passed in plain argument/in option
  • Refactor test-net-create-connection.js and rename it to test-net-connect-options-port.js
  • Add tests for connect(path)
  • Refactor test-net-connect-options.js, add tests for more new Socket() options.
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test, net

joyeecheung commented Mar 14, 2017 •
edited
Loading

Copy link
Copy Markdown
Member Author

CI: https://ci.nodejs.org/job/node-test-pull-request/6839/
New CI: https://ci.nodejs.org/job/node-test-pull-request/6841/

joyeecheung added net Issues and PRs related to the net subsystem. test Issues and PRs related to the tests. labels Mar 14, 2017

joyeecheung commented Mar 15, 2017

Copy link
Copy Markdown
Member Author

Error messages vary on different machines..loosen up the message validation and try again. CI: https://ci.nodejs.org/job/node-test-pull-request/6850/

joyeecheung changed the title test: test various calling patterns of net.connect() related functions [WIP]test: test various calling patterns of net.connect() related functions Mar 15, 2017
joyeecheung force-pushed the net-connect-test branch 2 times, most recently from 046cc82 to 969e285 Compare March 15, 2017 07:34
joyeecheung force-pushed the net-connect-test branch 10 times, most recently from d2d8b48 to 40e984d Compare March 15, 2017 11:35
joyeecheung changed the title [WIP]test: test various calling patterns of net.connect() related functions test: test various calling patterns of net.connect() related functions Mar 16, 2017

joyeecheung commented Mar 16, 2017

Copy link
Copy Markdown
Member Author

Something is wrong with net.connect({fd: TCPfd})(keep getting ENOTCONN or flaky libuv assertion failures)..probably has something to do with the state management. I'll remove it from this PR for now and investigate later. Pipe fd works fine though.

@jasnell PTAL, thanks.

CI is green now: https://ci.nodejs.org/job/node-test-pull-request/6867/

jasnell commented Mar 16, 2017

Copy link
Copy Markdown
Member

Still LGTM!

jasnell pushed a commit that referenced this pull request Mar 16, 2017
test: add more and refactor test cases to net.connect
7b830f4 
PR-URL: #11847
Reviewed-By: James M Snell <jasnell@gmail.com>

jasnell commented Mar 16, 2017

Copy link
Copy Markdown
Member

Landed in 7b830f4

jasnell closed this Mar 16, 2017
jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
test: add more and refactor test cases to net.connect
59d1520 
PR-URL: nodejs#11847
Reviewed-By: James M Snell <jasnell@gmail.com>

italoacasas commented Mar 21, 2017

Copy link
Copy Markdown

This PR need backport to v7

jasnell mentioned this pull request Apr 4, 2017
Closed
joyeecheung added a commit to joyeecheung/node that referenced this pull request Apr 11, 2017
test: add more and refactor test cases to net.connect
8f1277c 
PR-URL: nodejs#11847
joyeecheung mentioned this pull request Apr 11, 2017
Closed
3 tasks
joyeecheung mentioned this pull request May 9, 2017
Closed
3 tasks

gibfahn commented Jun 18, 2017

Copy link
Copy Markdown
Member

@joyeecheung Should this be backported to v6.x-staging? If yes please follow the guide and raise a backport PR, if no let me know or add the dont-land-on label.

If it is backported, the PR should include #12601.

gibfahn mentioned this pull request Jun 18, 2017
Closed
3 tasks
joyeecheung self-assigned this Oct 17, 2017
joyeecheung removed their assignment Nov 1, 2017
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes

Assignees

No one assigned

Labels

net Issues and PRs related to the net subsystem. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL