Skip to content

Conversation

joyeecheung
Copy link
Member

There is actually a leak. The test doesn't exercise the right path to create a substantial enough object graph (e.g. accessing something that results in the loading of a binding). This does something more complicated in the test and moves it to known_issues until we find a fix.

Refs: #47353

There is actually a leak. The test doesn't exercise the right
path to create a substantial enough object graph (e.g.
accessing something that results in the loading of a binding).
This does something more complicated in the test and moves it
to known_issues until we find a fix.
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Apr 1, 2023
@anonrig
Copy link
Member

anonrig commented Apr 1, 2023

known_issues expect the test to fail, but it just throws OOM exception causing the test runner to fail. Can we update & fast-track this, to merge Ada pull-request?

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@legendecas legendecas added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. commit-queue Add this label to land a pull request using GitHub Actions. labels Apr 4, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 4, 2023
@nodejs-github-bot nodejs-github-bot merged commit 0361978 into nodejs:main Apr 4, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 0361978

RafaelGSS pushed a commit that referenced this pull request Apr 5, 2023
There is actually a leak. The test doesn't exercise the right
path to create a substantial enough object graph (e.g.
accessing something that results in the loading of a binding).
This does something more complicated in the test and moves it
to known_issues until we find a fix.

PR-URL: #47355
Refs: #47353
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 5, 2023
There is actually a leak. The test doesn't exercise the right
path to create a substantial enough object graph (e.g.
accessing something that results in the loading of a binding).
This does something more complicated in the test and moves it
to known_issues until we find a fix.

PR-URL: #47355
Refs: #47353
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Apr 6, 2023
RafaelGSS pushed a commit that referenced this pull request Apr 6, 2023
There is actually a leak. The test doesn't exercise the right
path to create a substantial enough object graph (e.g.
accessing something that results in the loading of a binding).
This does something more complicated in the test and moves it
to known_issues until we find a fix.

PR-URL: #47355
Refs: #47353
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
@RafaelGSS
Copy link
Member

Hi @joyeecheung! This commit didn't land cleanly on v19.x-staging because there are no test-shadow-realm-gc.js (due to commit drops). Can you please create a manual backport? I'm adding the backport-blocked label because it depends on #46809

legendecas pushed a commit to legendecas/node that referenced this pull request Apr 12, 2023
There is actually a leak. The test doesn't exercise the right
path to create a substantial enough object graph (e.g.
accessing something that results in the loading of a binding).
This does something more complicated in the test and moves it
to known_issues until we find a fix.

PR-URL: nodejs#47355
Refs: nodejs#47353
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 13, 2023
There is actually a leak. The test doesn't exercise the right
path to create a substantial enough object graph (e.g.
accessing something that results in the loading of a binding).
This does something more complicated in the test and moves it
to known_issues until we find a fix.

PR-URL: #47355
Refs: #47353
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
There is actually a leak. The test doesn't exercise the right
path to create a substantial enough object graph (e.g.
accessing something that results in the loading of a binding).
This does something more complicated in the test and moves it
to known_issues until we find a fix.

PR-URL: #47355
Refs: #47353
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
MoLow pushed a commit to MoLow/node that referenced this pull request Jul 6, 2023
There is actually a leak. The test doesn't exercise the right
path to create a substantial enough object graph (e.g.
accessing something that results in the loading of a binding).
This does something more complicated in the test and moves it
to known_issues until we find a fix.

PR-URL: nodejs#47355
Refs: nodejs#47353
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants