test: simplify test-gc-{http-client,net}-* by lpinca · Pull Request #42782 · nodejs/node

lpinca · 2022-04-19T13:41:44Z

Instead of sending/creating a fixed number of requests/connections,
detect when GC has started and stop sending requests/creating
connections at that point.

Refs: 47ecf2060343
Refs: 7ce8403ef1a6

nodejs-github-bot · 2022-04-19T19:13:05Z

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

nodejs-github-bot · 2022-04-21T05:31:08Z

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

nodejs-github-bot · 2022-04-21T10:59:40Z

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

nodejs-github-bot · 2022-04-23T18:56:01Z

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

nodejs-github-bot · 2022-04-24T16:05:35Z

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

aduh95 · 2022-04-28T18:45:10Z

test/parallel/test-gc-http-client-connaborted.js

+    } else {
+      setImmediate(status);
+    }
+  } else {
+    setImmediate(status);
+  }


nit

Suggested change

} else {

setImmediate(status);

}

} else {

setImmediate(status);

}

return;

}

}

setImmediate(status);

This is only a cosmetic change that is inconsistent with the original refactor done in 47ecf2060343. I would prefer to not apply it.

Sure, feel free to ignore and land as is, LGTM anyway.

I'll apply it, but it's a little annoying to rerun CI for something like this.

aduh95 · 2022-04-28T18:46:00Z

test/parallel/test-gc-http-client-timeout.js

+    } else {
+      setImmediate(status);
+    }
+  } else {
+    setImmediate(status);
+  }


nit

Suggested change

} else {

setImmediate(status);

}

} else {

setImmediate(status);

}

return;

}

}

setImmediate(status);

aduh95 · 2022-04-28T18:46:15Z

test/parallel/test-gc-net-timeout.js

+    } else {
+      setImmediate(status);
+    }
+  } else {
+    setImmediate(status);
+  }


nit

Suggested change

} else {

setImmediate(status);

}

} else {

setImmediate(status);

}

return;

}

}

setImmediate(status);

Instead of sending/creating a fixed number of requests/connections, detect when GC has started and stop sending requests/creating connections at that point. Refs: nodejs@47ecf2060343 Refs: nodejs@7ce8403ef1a6

nodejs-github-bot · 2022-04-28T19:09:30Z

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

nodejs-github-bot · 2022-04-28T20:48:19Z

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

nodejs-github-bot · 2022-04-28T22:34:39Z

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

nodejs-github-bot · 2022-04-29T19:09:27Z

Commit Queue failed

- Loading data for nodejs/node/pull/42782
✔  Done loading data for nodejs/node/pull/42782
----------------------------------- PR info ------------------------------------
Title      test: simplify test-gc-{http-client,net}-* (#42782)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     lpinca:simplify/test-gc-xxx -> nodejs:master
Labels     test, author ready, needs-ci
Commits    1
 - test: simplify test-gc-{http-client,net}-*
Committers 1
 - Luigi Pinca 
PR-URL: https://github.com/nodejs/node/pull/42782
Refs: https://github.com/nodejs/node/commit/47ecf2060343
Refs: https://github.com/nodejs/node/commit/7ce8403ef1a6
Reviewed-By: James M Snell 
Reviewed-By: Antoine du Hamel 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/42782
Refs: https://github.com/nodejs/node/commit/47ecf2060343
Refs: https://github.com/nodejs/node/commit/7ce8403ef1a6
Reviewed-By: James M Snell 
Reviewed-By: Antoine du Hamel 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - test: simplify test-gc-{http-client,net}-*
   ℹ  This PR was created on Tue, 19 Apr 2022 13:41:44 GMT
   ✔  Approvals: 2
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/42782#pullrequestreview-947803586
   ✔  - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/42782#pullrequestreview-956901494
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-04-28T22:34:39Z: https://ci.nodejs.org/job/node-test-pull-request/43757/
- Querying data for job/node-test-pull-request/43757/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/2247249270

Instead of sending/creating a fixed number of requests/connections, detect when GC has started and stop sending requests/creating connections at that point. Refs: 47ecf2060343 Refs: 7ce8403ef1a6 PR-URL: #42782 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

lpinca · 2022-04-29T19:18:12Z

Landed in 02e0c17.

Instead of sending/creating a fixed number of requests/connections, detect when GC has started and stop sending requests/creating connections at that point. Refs: 47ecf2060343 Refs: 7ce8403ef1a6 PR-URL: #42782 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

Instead of sending/creating a fixed number of requests/connections, detect when GC has started and stop sending requests/creating connections at that point. Refs: nodejs/node@47ecf2060343 Refs: nodejs/node@7ce8403ef1a6 PR-URL: nodejs/node#42782 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Apr 19, 2022

lpinca force-pushed the simplify/test-gc-xxx branch 2 times, most recently from 59d8f04 to 5324b0b Compare April 19, 2022 13:48

lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 19, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 19, 2022

This comment was marked as outdated.

Sign in to view

lpinca force-pushed the simplify/test-gc-xxx branch from 5324b0b to 0d66d64 Compare April 19, 2022 18:51

lpinca changed the title test: simplify test-gc-* test: simplify test-gc-{http-client,net}* Apr 19, 2022

lpinca force-pushed the simplify/test-gc-xxx branch from 0d66d64 to 9fd4911 Compare April 19, 2022 18:57

lpinca changed the title test: simplify test-gc-{http-client,net}* test: simplify test-gc-{http-client,net}-* Apr 19, 2022

lpinca force-pushed the simplify/test-gc-xxx branch from 9fd4911 to 1b10c03 Compare April 19, 2022 18:59

lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 19, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 19, 2022

github-actions bot mentioned this pull request Apr 20, 2022

CI Reliability 2022-04-20 nodejs/reliability#257

Open

16 tasks

lpinca force-pushed the simplify/test-gc-xxx branch from 1b10c03 to 49ba47a Compare April 20, 2022 07:01

jasnell approved these changes Apr 20, 2022

View reviewed changes

github-actions bot mentioned this pull request Apr 21, 2022

CI Reliability 2022-04-21 nodejs/reliability#258

Open

22 tasks

lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 21, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 21, 2022

This was referenced Apr 22, 2022

CI Reliability 2022-04-22 nodejs/reliability#259

Open

CI Reliability 2022-04-23 nodejs/reliability#260

Open

github-actions bot mentioned this pull request Apr 24, 2022

CI Reliability 2022-04-24 nodejs/reliability#261

Open

21 tasks

This was referenced Apr 25, 2022

CI Reliability 2022-04-25 nodejs/reliability#262

Open

CI Reliability 2022-04-26 nodejs/reliability#263

Open

aduh95 approved these changes Apr 28, 2022

View reviewed changes

test: simplify test-gc-{http-client,net}-*

8518bd1

Instead of sending/creating a fixed number of requests/connections, detect when GC has started and stop sending requests/creating connections at that point. Refs: nodejs@47ecf2060343 Refs: nodejs@7ce8403ef1a6

lpinca force-pushed the simplify/test-gc-xxx branch from 8d240e1 to 8518bd1 Compare April 28, 2022 19:02

lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 28, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 28, 2022

github-actions bot mentioned this pull request Apr 29, 2022

CI Reliability 2022-04-29 nodejs/reliability#266

Open

21 tasks

lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 29, 2022

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 29, 2022

nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Apr 29, 2022

lpinca closed this Apr 29, 2022

This was referenced Apr 30, 2022

CI Reliability 2022-04-30 nodejs/reliability#267

Open

CI Reliability 2022-05-01 nodejs/reliability#268

Open

CI Reliability 2022-05-02 nodejs/reliability#269

Open

targos mentioned this pull request May 2, 2022

v18.1.0 release proposal #42943

Merged

github-actions bot mentioned this pull request May 3, 2022

CI Reliability 2022-05-03 nodejs/reliability#270

Open

33 tasks

lpinca deleted the simplify/test-gc-xxx branch May 10, 2022 08:17

juanarbol mentioned this pull request May 31, 2022

2022-05-XX, Version 16.16.0 'Gallium' (LTS) #43268

Closed

targos mentioned this pull request Aug 3, 2022

v16.17.0 release proposal #44098

Merged

Uh oh!

Conversation

lpinca commented Apr 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

nodejs-github-bot commented Apr 19, 2022

Uh oh!

nodejs-github-bot commented Apr 21, 2022

Uh oh!

nodejs-github-bot commented Apr 21, 2022

Uh oh!

nodejs-github-bot commented Apr 23, 2022

Uh oh!

nodejs-github-bot commented Apr 24, 2022

Uh oh!

aduh95 Apr 28, 2022

Choose a reason for hiding this comment

Uh oh!

lpinca Apr 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aduh95 Apr 28, 2022

Choose a reason for hiding this comment

Uh oh!

lpinca Apr 28, 2022

Choose a reason for hiding this comment

Uh oh!

aduh95 Apr 28, 2022

Choose a reason for hiding this comment

Uh oh!

aduh95 Apr 28, 2022

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Apr 28, 2022

Uh oh!

nodejs-github-bot commented Apr 28, 2022

Uh oh!

nodejs-github-bot commented Apr 28, 2022

Uh oh!

nodejs-github-bot commented Apr 29, 2022

Uh oh!

lpinca commented Apr 29, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lpinca commented Apr 19, 2022 •

edited

Loading

lpinca Apr 28, 2022 •

edited

Loading