Skip to content

test: fix nits in test/fixtures/tls-connect.js - #28880

Closed
lpinca wants to merge 1 commit into
nodejs:masterfrom
lpinca:fix/nits
Closed

test: fix nits in test/fixtures/tls-connect.js#28880
lpinca wants to merge 1 commit into
nodejs:masterfrom
lpinca:fix/nits

Conversation

@lpinca

Copy link
Copy Markdown
Member
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Jul 28, 2019
function maybeCallback() {
if (!callback)
return;
if (server.conn && (client.conn || client.err)) {

@lpincalpincaJul 28, 2019

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

client.err is set by the 'error' listener of the client socket or in the catch branch of the try...catch around tls.connect(). In the latter case maybeCallback() is not used as the callback is called directly. In the former client.conn is also set.

@TrottTrott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 30, 2019
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@Trott

Copy link
Copy Markdown
Member

Landed in 834a413

@TrottTrott closed this Jul 30, 2019
Trott pushed a commit to Trott/io.js that referenced this pull request Jul 30, 2019
PR-URL: nodejs#28880
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@lpinca
lpinca deleted the fix/nits branch July 31, 2019 18:54
targos pushed a commit that referenced this pull request Aug 2, 2019
PR-URL: #28880
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@BridgeARBridgeAR mentioned this pull request Aug 6, 2019
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@lpinca@nodejs-github-bot@Trott@jasnell@cjihrig