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

test: harden test-gc-http-client by BridgeAR · Pull Request #22373 · nodejs/node · GitHub

/ node Public

test: harden test-gc-http-client - #22373

Closed
BridgeAR wants to merge 1 commit into
nodejs:masterfrom
BridgeAR:harden-gc-test
ConversationCommits (1)ChecksFiles changed
Closed

test: harden test-gc-http-client#22373
BridgeAR wants to merge 1 commit into
nodejs:masterfrom
BridgeAR:harden-gc-test

Conversation

BridgeAR commented Aug 17, 2018

Copy link
Copy Markdown
Member

This reduces the total number of requests from 500 to 300 and triggers
more requests in parallel. It also moves some function creation out
and waits with the first request until the server is listening.

Fixes: #22336

@nodejs/testing PTAL

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

nodejs-github-bot commented Aug 17, 2018

Copy link
Copy Markdown
Collaborator

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

nodejs-github-bot added the test Issues and PRs related to the tests. label Aug 17, 2018

BridgeAR commented Aug 17, 2018

Copy link
Copy Markdown
Member Author

My local test:

> python tools/test.py -j 64 --repeat 64 parallel/test-gc-http-client.js
// Before:
[02:02|% 100|+   2|-  62]: Done
// After:                       
[01:24|% 100|+  64|-   0]: Done

BridgeAR commented Aug 17, 2018

Copy link
Copy Markdown
Member Author

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

joyeecheung commented Aug 17, 2018 •
edited by BridgeAR
Loading

Copy link
Copy Markdown
Member

stress test: https://ci.nodejs.org/job/node-stress-single-test/1993/ ✔️

joyeecheung mentioned this pull request Aug 17, 2018
Closed

BridgeAR commented Aug 17, 2018

Copy link
Copy Markdown
Member Author

The stress test came out green.

This was referenced Aug 17, 2018
Closed
Closed
joyeecheung approved these changes Aug 17, 2018
View reviewed changes
Comment thread test/parallel/test-gc-http-client.js Outdated

joyeecheung Aug 17, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can you wrap this in common.mustCall()?

BridgeAR Aug 18, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We have lots of tests that make sure that server.listen triggers the callback. If that would not hold, hundreds of tests would fail but if you think strongly about it, I'm going to add it.

joyeecheung Aug 18, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@BridgeAR I don't feel strongly about it :)

BridgeAR added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Aug 19, 2018

BridgeAR commented Aug 19, 2018

Copy link
Copy Markdown
Member Author

I wrapped the listen call in a mustCall now.

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

BridgeAR commented Aug 19, 2018

Copy link
Copy Markdown
Member Author

Resumed build https://ci.nodejs.org/job/node-test-pull-request/16572/

Trott commented Aug 19, 2018

Copy link
Copy Markdown
Member

@BridgeAR This needs to be rebased against master to bypass an unfortunate quirk of CI that's affecting some PRs.

test: harden test-gc-http-client
262fc9f 
This reduces the total number of requests from 500 to 300 and triggers
more requests in parallel. It also moves some function creation out
and waits with the first request until the server is listening.

Fixes: nodejs#22336

BridgeAR commented Aug 19, 2018

Copy link
Copy Markdown
Member Author

Rebased.

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

joyeecheung commented Aug 20, 2018

Copy link
Copy Markdown
Member

CI is green

joyeecheung commented Aug 20, 2018

Copy link
Copy Markdown
Member

Landed in 79642ae, thanks!

joyeecheung pushed a commit that referenced this pull request Aug 20, 2018
test: harden test-gc-http-client
79642ae 
This reduces the total number of requests from 500 to 300 and triggers
more requests in parallel. It also moves some function creation out
and waits with the first request until the server is listening.

PR-URL: #22373
Fixes: #22336
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
joyeecheung mentioned this pull request Aug 20, 2018
Closed
targos pushed a commit that referenced this pull request Aug 21, 2018
test: harden test-gc-http-client
73c9592 
This reduces the total number of requests from 500 to 300 and triggers
more requests in parallel. It also moves some function creation out
and waits with the first request until the server is listening.

PR-URL: #22373
Fixes: #22336
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Sep 3, 2018
test: harden test-gc-http-client
149c209 
This reduces the total number of requests from 500 to 300 and triggers
more requests in parallel. It also moves some function creation out
and waits with the first request until the server is listening.

PR-URL: #22373
Fixes: #22336
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos mentioned this pull request Sep 5, 2018
Merged
This was referenced Sep 6, 2018
Closed
Closed
BridgeAR deleted the harden-gc-test branch January 20, 2020 11:35
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

joyeecheung joyeecheung approved these changes
jasnell jasnell approved these changes
lpinca lpinca approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Investigate flaky parallel/test-gc-http-client

6 participants

Back | FazBrowse Home | New Git URL