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

test: deflake test-tls-passphrase #29134

Closed
wants to merge 1 commit into from

Conversation

@lpinca
Copy link
Member

commented Aug 15, 2019

Move socket.end() to client.

Fixes: #28111
Refs: #27569

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
@lpinca

This comment has been minimized.

Copy link
Member Author

commented Aug 15, 2019

$ ./tools/test.py -J --repeat=500 test/parallel/test-tls-passphrase.js 
=== release test-tls-passphrase ===                   
Path: parallel/test-tls-passphrase
events.js:186
      throw er; // Unhandled 'error' event
      ^

Error: read ECONNRESET
    at TLSWrap.onStreamRead (internal/stream_base_commons.js:183:27)
Emitted 'error' event on TLSSocket instance at:
    at emitErrorNT (internal/streams/destroy.js:91:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:59:3)
    at processTicksAndRejections (internal/process/task_queues.js:77:11) {
  errno: -54,
  code: 'ECONNRESET',
  syscall: 'read'
}
Command: out/Release/node /Users/luigi/code/node/test/parallel/test-tls-passphrase.js
=== release test-tls-passphrase ===                   
Path: parallel/test-tls-passphrase
events.js:186
      throw er; // Unhandled 'error' event
      ^

Error: read ECONNRESET
    at TLSWrap.onStreamRead (internal/stream_base_commons.js:183:27)
Emitted 'error' event on TLSSocket instance at:
    at emitErrorNT (internal/streams/destroy.js:91:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:59:3)
    at processTicksAndRejections (internal/process/task_queues.js:77:11) {
  errno: -54,
  code: 'ECONNRESET',
  syscall: 'read'
}
Command: out/Release/node /Users/luigi/code/node/test/parallel/test-tls-passphrase.js

...
@lpinca

This comment has been minimized.

Copy link
Member Author

commented Aug 15, 2019

@bnoordhuis
Copy link
Member

left a comment

LGTM with a suggestion.

test/parallel/test-tls-passphrase.js Outdated Show resolved Hide resolved

@lpinca lpinca force-pushed the lpinca:deflake/test-tls-passphrase branch from 20c24fd to 6db2364 Aug 15, 2019

@sam-github
Copy link
Member

left a comment

@lpinca It looks like you are changing it so client ends connection rather than server, to prevent races. Is it still flaky, or not flaky? I'm not sure if the errors you pinged me about were before or after your fix. The changes LGTM

@lpinca

This comment has been minimized.

Copy link
Member Author

commented Aug 15, 2019

The error is before fix, it is no longer flaky now unless I raise --repeat to 1000. After that a new errors occurs (EADDRNOTAVAIL) but that is a different issue. See #28337 (comment).

Also it looks this fixes #28111. Will add the "Fixes:" metadata.

test: deflake test-tls-passphrase
Move `socket.end()` to client.

Fixes: #28111
Refs: #27569

@lpinca lpinca force-pushed the lpinca:deflake/test-tls-passphrase branch from 6db2364 to 6d6de17 Aug 15, 2019

@Trott
Trott approved these changes Aug 16, 2019
@nodejs-github-bot

This comment has been minimized.

@Trott

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

Landed in a0cc62f

@Trott Trott closed this Aug 19, 2019

Trott added a commit that referenced this pull request Aug 19, 2019
test: deflake test-tls-passphrase
Move `socket.end()` to client.

Fixes: #28111
Refs: #27569

PR-URL: #29134
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

@lpinca lpinca deleted the lpinca:deflake/test-tls-passphrase branch Aug 19, 2019

targos added a commit that referenced this pull request Aug 20, 2019
test: deflake test-tls-passphrase
Move `socket.end()` to client.

Fixes: #28111
Refs: #27569

PR-URL: #29134
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos referenced this pull request Aug 20, 2019
JeniaBR added a commit to JeniaBR/node that referenced this pull request Sep 11, 2019
test: deflake test-tls-passphrase
Move `socket.end()` to client.

Fixes: nodejs#28111
Refs: nodejs#27569

PR-URL: nodejs#29134
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
JeniaBR added a commit to JeniaBR/node that referenced this pull request Sep 11, 2019
test: deflake test-tls-passphrase
Move `socket.end()` to client.

Fixes: nodejs#28111
Refs: nodejs#27569

PR-URL: nodejs#29134
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.