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.

Sign up for GitHub

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

Jump to bottom

fix and improve flaky test-https-connect-localport #26881

Closed
wants to merge 4 commits into from
Closed

fix and improve flaky test-https-connect-localport #26881

wants to merge 4 commits into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Mar 23, 2019

First commit:

test: move test-https-connect-localport to sequential

test-https-connect-localport uses a hard-coded port number. Therefore
the test cannot be in the parallel directory because it will sometimes
conflict with other tests that run at the same time and request that the
OS provide an available port.

Fixes: https://github.com/nodejs/node/issues/26862

Second commit:

test: use common.PORT instead of hardcoded number

In sequential/test-https-connect-localport, replace 34567 with
common.PORT

Third commit:

test: replace localhost IP with 'localhost' for TLS conformity

test-https-connect-localport currently causes a runtime deprecation
warning: "[DEP0123] DeprecationWarning: Setting the TLS ServerName
to an IP address is not permitted by RFC 6066. This will be ignored
in a future version."

Change IP usage to the string 'localhost' instead.

Fourth commit:

test: refactor test-https-connect-localport

Use arrow functions for callbacks. Replace uses of `this` with explicit
variables. Add a trailing comma in a multiline object literal
declaration.
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Trott added 4 commits March 23, 2019 09:34
@Trott
test: move test-https-connect-localport to sequential
dad3deb 
test-https-connect-localport uses a hard-coded port number. Therefore
the test cannot be in the parallel directory because it will sometimes
conflict with other tests that run at the same time and request that the
OS provide an available port.

Fixes: nodejs#26862
@Trott
test: use common.PORT instead of hardcoded number
0afa99b 
In sequential/test-https-connect-localport, replace 34567 with
common.PORT.
@Trott
test: replace localhost IP with 'localhost' for TLS conformity
5348b10 
test-https-connect-localport currently causes a runtime deprecation
warning: "[DEP0123] DeprecationWarning: Setting the TLS ServerName
to an IP address is not permitted by RFC 6066. This will be ignored
in a future version."

Change IP usage to the string 'localhost' instead.
@Trott
test: refactor test-https-connect-localport
277ede3 
Use arrow functions for callbacks. Replace uses of `this` with explicit
variables. Add a trailing comma in a multiline object literal
declaration.
@Trott Trott added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Mar 23, 2019
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Mar 23, 2019
@Trott Trott mentioned this pull request Mar 23, 2019
Closed
@Trott
Copy link
Member Author

Trott commented Mar 23, 2019

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

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 24, 2019
@gireeshpunathil
Copy link
Member

fast track? 👍

@Trott Trott added the fast-track PRs that do not need to wait for 48 hours to land. label Mar 24, 2019
@Trott
Copy link
Member Author

Trott commented Mar 24, 2019

Landed in 5b59b5f...4a40ea6

@Trott Trott closed this Mar 24, 2019
Trott added a commit to Trott/io.js that referenced this pull request Mar 24, 2019
@Trott
test: move test-https-connect-localport to sequential
0f7e24a 
test-https-connect-localport uses a hard-coded port number. Therefore
the test cannot be in the parallel directory because it will sometimes
conflict with other tests that run at the same time and request that the
OS provide an available port.

Fixes: nodejs#26862

PR-URL: nodejs#26881
Fixes: nodejs#26862
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Trott added a commit to Trott/io.js that referenced this pull request Mar 24, 2019
@Trott
test: use common.PORT instead of hardcoded number
229759f 
In sequential/test-https-connect-localport, replace 34567 with
common.PORT.

PR-URL: nodejs#26881
Fixes: nodejs#26862
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Trott added a commit to Trott/io.js that referenced this pull request Mar 24, 2019
@Trott
test: replace localhost IP with 'localhost' for TLS conformity
f5969f0 
test-https-connect-localport currently causes a runtime deprecation
warning: "[DEP0123] DeprecationWarning: Setting the TLS ServerName
to an IP address is not permitted by RFC 6066. This will be ignored
in a future version."

Change IP usage to the string 'localhost' instead.

PR-URL: nodejs#26881
Fixes: nodejs#26862
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Trott added a commit to Trott/io.js that referenced this pull request Mar 24, 2019
@Trott
test: refactor test-https-connect-localport
4a40ea6 
Use arrow functions for callbacks. Replace uses of `this` with explicit
variables. Add a trailing comma in a multiline object literal
declaration.

PR-URL: nodejs#26881
Fixes: nodejs#26862
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Mar 27, 2019
@Trott @targos
test: move test-https-connect-localport to sequential
0470dc6 
test-https-connect-localport uses a hard-coded port number. Therefore
the test cannot be in the parallel directory because it will sometimes
conflict with other tests that run at the same time and request that the
OS provide an available port.

Fixes: nodejs#26862

PR-URL: nodejs#26881
Fixes: nodejs#26862
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Mar 27, 2019
@Trott @targos
test: use common.PORT instead of hardcoded number
4fbec0c 
In sequential/test-https-connect-localport, replace 34567 with
common.PORT.

PR-URL: nodejs#26881
Fixes: nodejs#26862
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Mar 27, 2019
@Trott @targos
test: replace localhost IP with 'localhost' for TLS conformity
547d01d 
test-https-connect-localport currently causes a runtime deprecation
warning: "[DEP0123] DeprecationWarning: Setting the TLS ServerName
to an IP address is not permitted by RFC 6066. This will be ignored
in a future version."

Change IP usage to the string 'localhost' instead.

PR-URL: nodejs#26881
Fixes: nodejs#26862
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Mar 27, 2019
@Trott @targos
test: refactor test-https-connect-localport
544253d 
Use arrow functions for callbacks. Replace uses of `this` with explicit
variables. Add a trailing comma in a multiline object literal
declaration.

PR-URL: nodejs#26881
Fixes: nodejs#26862
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
targos pushed a commit that referenced this pull request Mar 27, 2019
@Trott @targos
test: move test-https-connect-localport to sequential
4919e4b 
test-https-connect-localport uses a hard-coded port number. Therefore
the test cannot be in the parallel directory because it will sometimes
conflict with other tests that run at the same time and request that the
OS provide an available port.

Fixes: #26862

PR-URL: #26881
Fixes: #26862
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
targos pushed a commit that referenced this pull request Mar 27, 2019
@Trott @targos
test: use common.PORT instead of hardcoded number
011c205 
In sequential/test-https-connect-localport, replace 34567 with
common.PORT.

PR-URL: #26881
Fixes: #26862
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
targos pushed a commit that referenced this pull request Mar 27, 2019
@Trott @targos
test: replace localhost IP with 'localhost' for TLS conformity
838fb95 
test-https-connect-localport currently causes a runtime deprecation
warning: "[DEP0123] DeprecationWarning: Setting the TLS ServerName
to an IP address is not permitted by RFC 6066. This will be ignored
in a future version."

Change IP usage to the string 'localhost' instead.

PR-URL: #26881
Fixes: #26862
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
targos pushed a commit that referenced this pull request Mar 27, 2019
@Trott @targos
test: refactor test-https-connect-localport
3cae010 
Use arrow functions for callbacks. Replace uses of `this` with explicit
variables. Add a trailing comma in a multiline object literal
declaration.

PR-URL: #26881
Fixes: #26862
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
@targos targos mentioned this pull request Mar 27, 2019
Merged
@Trott Trott deleted the localport-test-fix branch January 13, 2022 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ZYSzys ZYSzys ZYSzys approved these changes

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

@watilde watilde watilde approved these changes

@lpinca lpinca lpinca approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. flaky-test Issues and PRs related to the tests with unstable failures on the CI. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Trott @nodejs-github-bot @gireeshpunathil @lpinca @watilde @ZYSzys