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

test: make IsolateData per-isolate in cctest by joyeecheung · Pull Request #48450 · nodejs/node · GitHub

/ node Public

test: make IsolateData per-isolate in cctest - #48450

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
joyeecheung:isolatedata-cctest
Jun 25, 2023
Merged

test: make IsolateData per-isolate in cctest#48450
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
joyeecheung:isolatedata-cctest

Conversation

Copy link
Copy Markdown
Member

This ensures that we only create one IsolateData for each isolate inthe cctest, since IsolateData are meant to be per-isolate. We need to make the isolate and isolate_data static in the test fixtures as a result, similar to how the event loops and array buffer allocators are managed in the
NodeZeroIsolateTestFixture but it is fine because gtest ensures that the Setup() and TearDown() of the fixtures are always run in order and would never overlap in one process.

This ensures that we only create one IsolateData for each isolate
inthe cctest, since IsolateData are meant to be per-isolate.
We need to make the isolate and isolate_data static in the
test fixtures as a result, similar to how the event loops and
array buffer allocators are managed in the
NodeZeroIsolateTestFixture but it is fine because gtest ensures
that the Setup() and TearDown() of the fixtures are always run
in order and would never overlap in one process.
nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Jun 13, 2023
joyeecheung requested a review from addaleax June 13, 2023 18:02

Copy link
Copy Markdown
Member Author

cc @nodejs/cpp-reviewers @nodejs/startup

legendecas added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jun 25, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 25, 2023

Copy link
Copy Markdown
Collaborator

legendecas added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 25, 2023
nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 25, 2023
nodejs-github-bot merged commit 42d8143 into nodejs:main Jun 25, 2023

Copy link
Copy Markdown
Collaborator

Landed in 42d8143

RafaelGSS pushed a commit that referenced this pull request Jul 3, 2023
This ensures that we only create one IsolateData for each isolate
inthe cctest, since IsolateData are meant to be per-isolate.
We need to make the isolate and isolate_data static in the
test fixtures as a result, similar to how the event loops and
array buffer allocators are managed in the
NodeZeroIsolateTestFixture but it is fine because gtest ensures
that the Setup() and TearDown() of the fixtures are always run
in order and would never overlap in one process.

PR-URL: #48450
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
RafaelGSS mentioned this pull request Jul 3, 2023
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
This ensures that we only create one IsolateData for each isolate
inthe cctest, since IsolateData are meant to be per-isolate.
We need to make the isolate and isolate_data static in the
test fixtures as a result, similar to how the event loops and
array buffer allocators are managed in the
NodeZeroIsolateTestFixture but it is fine because gtest ensures
that the Setup() and TearDown() of the fixtures are always run
in order and would never overlap in one process.

PR-URL: nodejs#48450
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
This ensures that we only create one IsolateData for each isolate
inthe cctest, since IsolateData are meant to be per-isolate.
We need to make the isolate and isolate_data static in the
test fixtures as a result, similar to how the event loops and
array buffer allocators are managed in the
NodeZeroIsolateTestFixture but it is fine because gtest ensures
that the Setup() and TearDown() of the fixtures are always run
in order and would never overlap in one process.

PR-URL: nodejs#48450
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
ruyadorno pushed a commit that referenced this pull request Sep 10, 2023
This ensures that we only create one IsolateData for each isolate
inthe cctest, since IsolateData are meant to be per-isolate.
We need to make the isolate and isolate_data static in the
test fixtures as a result, similar to how the event loops and
array buffer allocators are managed in the
NodeZeroIsolateTestFixture but it is fine because gtest ensures
that the Setup() and TearDown() of the fixtures are always run
in order and would never overlap in one process.

PR-URL: #48450
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
ruyadorno mentioned this pull request Sep 10, 2023
ruyadorno pushed a commit that referenced this pull request Sep 13, 2023
This ensures that we only create one IsolateData for each isolate
inthe cctest, since IsolateData are meant to be per-isolate.
We need to make the isolate and isolate_data static in the
test fixtures as a result, similar to how the event loops and
array buffer allocators are managed in the
NodeZeroIsolateTestFixture but it is fine because gtest ensures
that the Setup() and TearDown() of the fixtures are always run
in order and would never overlap in one process.

PR-URL: #48450
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. 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.

3 participants


Back | FazBrowse Home | New Git URL