Skip to content

test: fix test-http-server-keepalive-req-gc - #53292

Closed
targos wants to merge 1 commit into
nodejs:mainfrom
targos:robust-gc
Closed

test: fix test-http-server-keepalive-req-gc#53292
targos wants to merge 1 commit into
nodejs:mainfrom
targos:robust-gc

Conversation

@targos

Copy link
Copy Markdown
Member

This changes adds a second explicit gc call in the test. Without this
call, the test relies on gc eventually happening based, since the
first call doesn't free the object.
Relying on gc to eventually happen prevents changing GC heuristics
unrelated to this test.
The gc call is async; otherwise doing multiple sync GCs doesn't free
the object.

This changes adds a second explicit gc call in the test. Without this
call, the test relies on gc eventually happening based, since the
first call doesn't free the object.
Relying on gc to eventually happen prevents changing GC heuristics
unrelated to this test.
The gc call is async; otherwise doing multiple sync GCs doesn't free
the object.
@targostargos added the commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. label Jun 3, 2024
@targos

Copy link
Copy Markdown
MemberAuthor

Adapted from v8#186

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Jun 3, 2024
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@targos

Copy link
Copy Markdown
MemberAuthor

@nodejs/v8-update

@targostargos added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 10, 2024
@nodejs-github-botnodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jun 10, 2024
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/53292
✔ Done loading data for nodejs/node/pull/53292
----------------------------------- PR info ------------------------------------
Title test: fix test-http-server-keepalive-req-gc (#53292)
Author Michaël Zasso (@targos)
Branch targos:robust-gc -> nodejs:main
Labels test, needs-ci, commit-queue-rebase
Commits 1
- test: fix test-http-server-keepalive-req-gc
Committers 1
- Michaël Zasso PR-URL: https://github.com/nodejs/node/pull/53292
Reviewed-By: Luigi Pinca ------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/53292
Reviewed-By: Luigi Pinca --------------------------------------------------------------------------------
ℹ This PR was created on Mon, 03 Jun 2024 08:40:03 GMT
✔ Approvals: 1
✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/53292#pullrequestreview-2105182781
✘ This PR needs to wait 0 more hours to land (or 0 hours if there is one more approval)
⚠ GitHub cannot link the author of 'test: fix test-http-server-keepalive-req-gc' to their GitHub account.
⚠ Please suggest them to take a look at https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork
✔ Last GitHub CI successful
ℹ Last Full PR CI on 2024-06-06T05:03:41Z: https://ci.nodejs.org/job/node-test-pull-request/59670/
- Querying data for job/node-test-pull-request/59670/
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/9444846586

@targostargos added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jun 10, 2024
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 10, 2024
nodejs-github-bot pushed a commit that referenced this pull request Jun 10, 2024
This changes adds a second explicit gc call in the test. Without this
call, the test relies on gc eventually happening based, since the
first call doesn't free the object.
Relying on gc to eventually happen prevents changing GC heuristics
unrelated to this test.
The gc call is async; otherwise doing multiple sync GCs doesn't free
the object.
PR-URL: #53292
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in db09f62...2dea6a4

@targos
targos deleted the robust-gc branch June 10, 2024 09:41
targos pushed a commit that referenced this pull request Jun 20, 2024
This changes adds a second explicit gc call in the test. Without this
call, the test relies on gc eventually happening based, since the
first call doesn't free the object.
Relying on gc to eventually happen prevents changing GC heuristics
unrelated to this test.
The gc call is async; otherwise doing multiple sync GCs doesn't free
the object.
PR-URL: #53292
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
soophoo pushed a commit to soophoo/node that referenced this pull request Jun 20, 2024
This changes adds a second explicit gc call in the test. Without this
call, the test relies on gc eventually happening based, since the
first call doesn't free the object.
Relying on gc to eventually happen prevents changing GC heuristics
unrelated to this test.
The gc call is async; otherwise doing multiple sync GCs doesn't free
the object.
PR-URL: nodejs#53292
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@targostargos mentioned this pull request Jun 25, 2024
marco-ippolito pushed a commit that referenced this pull request Jul 19, 2024
This changes adds a second explicit gc call in the test. Without this
call, the test relies on gc eventually happening based, since the
first call doesn't free the object.
Relying on gc to eventually happen prevents changing GC heuristics
unrelated to this test.
The gc call is async; otherwise doing multiple sync GCs doesn't free
the object.
PR-URL: #53292
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@marco-ippolitomarco-ippolito mentioned this pull request Jul 19, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commit-queue-rebaseAdd this label to allow the Commit Queue to land a PR in several commits.needs-ciPRs that need a full CI run.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@targos@nodejs-github-bot@lpinca@etiennep-chromium