★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/25222Nouvelle recherche | Portail wanayoo
Skip to content

/node

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: fix test-tls-securepair-client #25222

Closed
wants to merge 2 commits into from

Conversation

@Trott
Copy link
Member

commented Dec 26, 2018

test-tls-securepair-client has been failing for over a year but no one
noticed because pummel tests are almost never run.

In preparation for running pummel tests once a day in CI, fix
test-tls-securepair-client.

test-tls-securepair-client does not seem to need to be in the pummel
directory. Move it to sequential. (It can't go into parallel because it
uses common.PORT and therefore might conflict with another test that is
using system-assigned available ports.)

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

This comment has been minimized.

Sign in to view
Copy link

commented Dec 26, 2018

@Trott build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2065/pipeline

@nodejs-github-bot nodejs-github-bot added the test label Dec 26, 2018

Trott referenced this pull request Dec 26, 2018
@addaleax
tls: make deprecated tls.createSecurePair() use public API 
Make the deprecated `tls.createSecurePair()` method use other public
APIs only (`TLSSocket` in particular).

Since `tls.createSecurePair()` has been runtime-deprecated only
since Node 8, it probably isn’t quite time to remove it yet,
but this patch removes almost all of the code complexity that
is retained by it.

The API, as it is documented, is retained. However, it is very likely
that some users have come to rely on parts of undocumented API
of the `SecurePair` class, especially since some of the existing
tests checked for those. Therefore, this should definitely be
considered a breaking change.

PR-URL: #17882
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Verified
This commit was signed with a verified signature.
addaleax Anna Henningsen
GPG key ID: 9C63F3A6CD2AD8F9 Learn about signing commits
9301b8a
@addaleax
addaleax approved these changes Dec 26, 2018
View changes

@Trott Trott force-pushed the Trott:fix-securepair branch from 9b4a93e to 407521d Dec 26, 2018

@cjihrig
cjihrig approved these changes Dec 26, 2018
View changes

@Trott Trott added the work in progress (WIP) label Dec 26, 2018

@Trott

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Dec 26, 2018

Works everywhere now but Windows...
@lundibundi
lundibundi approved these changes Dec 26, 2018
View changes
test/sequential/test-tls-securepair-client.js Outdated Show resolved Hide resolved
@lundibundi lundibundi referenced this pull request Dec 27, 2018
Open
4 of 4 tasks complete
@jasnell
jasnell approved these changes Dec 27, 2018
View changes

@Trott Trott force-pushed the Trott:fix-securepair branch from 407521d to a994d5d Jan 4, 2019

@Trott

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Jan 5, 2019

Here's what the Windows failure looks like:

16:02:14 not ok 619 sequential/test-tls-securepair-client
16:02:14   ---
16:02:14   duration_ms: 5.680
16:02:14   severity: fail
16:02:14   exitcode: 1
16:02:14   stack: |-
16:02:14     Using default temp DH parameters
16:02:14     WAIT-ACCEPT
16:02:14     ACCEPT
16:02:14     WAIT-ACCEPT
16:02:14     client connected
16:02:14     (node:5684) [DEP0064] DeprecationWarning: tls.createSecurePair() is deprecated. Please use tls.TLSSocket instead.
16:02:14     assert.js:86
16:02:14       throw new AssertionError(obj);
16:02:14       ^
16:02:14     
16:02:14     AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
16:02:14     
16:02:14     -1 !== 0
16:02:14     
16:02:14         at process.<anonymous> (c:\workspace\node-test-binary-windows\test\sequential\test-tls-securepair-client.js:174:12)
16:02:14         at process.emit (events.js:193:15)
16:02:14         at process.exit (internal/process/per_thread.js:150:15)
16:02:14         at Timeout._onTimeout (c:\workspace\node-test-binary-windows\test\sequential\test-tls-securepair-client.js:104:13)
16:02:14         at listOnTimeout (timers.js:324:15)
16:02:14         at processTimers (timers.js:268:5)
16:02:14   ...

@Trott Trott force-pushed the Trott:fix-securepair branch from a994d5d to a193928 Jan 7, 2019

Trott added 2 commits Dec 26, 2018
@Trott
test: fix test-tls-securepair-client 
test-tls-securepair-client has been failing for over a year but no one
noticed because pummel tests are almost never run.

In preparation for running pummel tests once a day in CI, fix
test-tls-securepair-client.
1e57cd6
@Trott
test: move test-tls-securepair-client out of pummel 
test-tls-securepair-client does not seem to need to be in the pummel
directory. Move it to sequential. (It can't go into parallel because it
uses common.PORT and therefore might conflict with another test that is
using system-assigned available ports.)
Loading status checks…
2a879a3

@Trott Trott force-pushed the Trott:fix-securepair branch from a193928 to 2a879a3 Jan 7, 2019

@Trott

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Jan 7, 2019

Added a common.skip() for Windows. My reasoning is that this test is currently broken and not run in CI at all. This PR fixes it everywhere but Windows and moves it somewhere that it will run in CI. So even if it is skipped on Windows, that's still a significant improvement over the current state. When this lands, I'll open an issue for the test to be fixed on Windows and add a help wanted label to it.

CI: https://ci.nodejs.org/job/node-test-pull-request/19959/

@Trott Trott added author ready and removed work in progress (WIP) labels Jan 7, 2019

@Trott

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Jan 7, 2019

Landed in ec5884a...0066fdb

@Trott Trott closed this Jan 7, 2019

Trott added a commit to Trott/io.js that referenced this pull request Jan 7, 2019
@Trott
test: fix test-tls-securepair-client 
test-tls-securepair-client has been failing for over a year but no one
noticed because pummel tests are almost never run.

In preparation for running pummel tests once a day in CI, fix
test-tls-securepair-client.

PR-URL: nodejs#25222
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
a5c9491
Trott added a commit to Trott/io.js that referenced this pull request Jan 7, 2019
@Trott
test: move test-tls-securepair-client out of pummel 
test-tls-securepair-client does not seem to need to be in the pummel
directory. Move it to sequential. (It can't go into parallel because it
uses common.PORT and therefore might conflict with another test that is
using system-assigned available ports.)

PR-URL: nodejs#25222
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
0066fdb
addaleax added a commit that referenced this pull request Jan 9, 2019
@Trott @addaleax
test: fix test-tls-securepair-client 
test-tls-securepair-client has been failing for over a year but no one
noticed because pummel tests are almost never run.

In preparation for running pummel tests once a day in CI, fix
test-tls-securepair-client.

PR-URL: #25222
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Verified
This commit was signed with a verified signature.
addaleax Anna Henningsen
GPG key ID: 9C63F3A6CD2AD8F9 Learn about signing commits
7ac1db2
addaleax added a commit that referenced this pull request Jan 9, 2019
@Trott @addaleax
test: move test-tls-securepair-client out of pummel 
test-tls-securepair-client does not seem to need to be in the pummel
directory. Move it to sequential. (It can't go into parallel because it
uses common.PORT and therefore might conflict with another test that is
using system-assigned available ports.)

PR-URL: #25222
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Verified
This commit was signed with a verified signature.
addaleax Anna Henningsen
GPG key ID: 9C63F3A6CD2AD8F9 Learn about signing commits
ce7bbd2
refack added a commit to refack/node that referenced this pull request Jan 14, 2019
@Trott @refack
test: fix test-tls-securepair-client 
test-tls-securepair-client has been failing for over a year but no one
noticed because pummel tests are almost never run.

In preparation for running pummel tests once a day in CI, fix
test-tls-securepair-client.

PR-URL: nodejs#25222
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
6938fd3
refack added a commit to refack/node that referenced this pull request Jan 14, 2019
@Trott @refack
test: move test-tls-securepair-client out of pummel 
test-tls-securepair-client does not seem to need to be in the pummel
directory. Move it to sequential. (It can't go into parallel because it
uses common.PORT and therefore might conflict with another test that is
using system-assigned available ports.)

PR-URL: nodejs#25222
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
ce5c2d0
@BridgeAR BridgeAR referenced this pull request Jan 16, 2019
Merged
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
@Trott @BridgeAR
test: fix test-tls-securepair-client 
test-tls-securepair-client has been failing for over a year but no one
noticed because pummel tests are almost never run.

In preparation for running pummel tests once a day in CI, fix
test-tls-securepair-client.

PR-URL: nodejs#25222
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Verified
This commit was signed with a verified signature.
BridgeAR Ruben Bridgewater
GPG key ID: F07496B3EB3C1762 Learn about signing commits
27b23c4
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
@Trott @BridgeAR
test: move test-tls-securepair-client out of pummel 
test-tls-securepair-client does not seem to need to be in the pummel
directory. Move it to sequential. (It can't go into parallel because it
uses common.PORT and therefore might conflict with another test that is
using system-assigned available ports.)

PR-URL: nodejs#25222
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Verified
This commit was signed with a verified signature.
BridgeAR Ruben Bridgewater
GPG key ID: F07496B3EB3C1762 Learn about signing commits
f8f7524
@MylesBorins MylesBorins referenced this pull request Jan 24, 2019
Merged
BethGriggs added a commit that referenced this pull request Apr 28, 2019
@Trott @BethGriggs
test: fix test-tls-securepair-client 
test-tls-securepair-client has been failing for over a year but no one
noticed because pummel tests are almost never run.

In preparation for running pummel tests once a day in CI, fix
test-tls-securepair-client.

PR-URL: #25222
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Verified
This commit was signed with a verified signature.
BethGriggs Bethany Nicolle Griggs
GPG key ID: D7062848A1AB005C Learn about signing commits
3ce744a
BethGriggs added a commit that referenced this pull request Apr 28, 2019
@Trott @BethGriggs
test: move test-tls-securepair-client out of pummel 
test-tls-securepair-client does not seem to need to be in the pummel
directory. Move it to sequential. (It can't go into parallel because it
uses common.PORT and therefore might conflict with another test that is
using system-assigned available ports.)

PR-URL: #25222
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Verified
This commit was signed with a verified signature.
BethGriggs Bethany Nicolle Griggs
GPG key ID: D7062848A1AB005C Learn about signing commits
0d5e54d
@BethGriggs BethGriggs referenced this pull request May 1, 2019
Merged
BethGriggs added a commit that referenced this pull request May 10, 2019
@Trott @BethGriggs
test: fix test-tls-securepair-client 
test-tls-securepair-client has been failing for over a year but no one
noticed because pummel tests are almost never run.

In preparation for running pummel tests once a day in CI, fix
test-tls-securepair-client.

PR-URL: #25222
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Verified
This commit was signed with a verified signature.
BethGriggs Bethany Nicolle Griggs
GPG key ID: D7062848A1AB005C Learn about signing commits
4301e5e
BethGriggs added a commit that referenced this pull request May 10, 2019
@Trott @BethGriggs
test: move test-tls-securepair-client out of pummel 
test-tls-securepair-client does not seem to need to be in the pummel
directory. Move it to sequential. (It can't go into parallel because it
uses common.PORT and therefore might conflict with another test that is
using system-assigned available ports.)

PR-URL: #25222
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Verified
This commit was signed with a verified signature.
BethGriggs Bethany Nicolle Griggs
GPG key ID: D7062848A1AB005C Learn about signing commits
166a61a
MylesBorins added a commit that referenced this pull request May 16, 2019
@Trott @MylesBorins
test: fix test-tls-securepair-client 
test-tls-securepair-client has been failing for over a year but no one
noticed because pummel tests are almost never run.

In preparation for running pummel tests once a day in CI, fix
test-tls-securepair-client.

PR-URL: #25222
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Verified
This commit was signed with a verified signature.
MylesBorins Myles Borins
GPG key ID: 933B01F40B5CA946 Learn about signing commits
26b69fd
MylesBorins added a commit that referenced this pull request May 16, 2019
@Trott @MylesBorins
test: move test-tls-securepair-client out of pummel 
test-tls-securepair-client does not seem to need to be in the pummel
directory. Move it to sequential. (It can't go into parallel because it
uses common.PORT and therefore might conflict with another test that is
using system-assigned available ports.)

PR-URL: #25222
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Verified
This commit was signed with a verified signature.
MylesBorins Myles Borins
GPG key ID: 933B01F40B5CA946 Learn about signing commits
f58dbb3
This was referenced May 29, 2019
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell

@addaleax addaleax

@cjihrig cjihrig

@lundibundi lundibundi

Assignees
No one assigned
Labels
author ready test
Projects
None yet
Milestone
No milestone
6 participants
@Trott @nodejs-github-bot @jasnell @addaleax @cjihrig @lundibundi
You can’t perform that action at this time.