GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — test: improve `UV_THREADPOOL_SIZE` tests on `.env` by anonrig · Pull Request #49213 · nodejs/node

Conversation

@anonrig

@anonrig anonrig commented Aug 17, 2023

Copy link
Copy Markdown
Member

Follow up after @tniessen's pull request #49165

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to Node.js core tests and test infrastructure. labels Aug 17, 2023
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 17, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 17, 2023
@nodejs-github-bot

nodejs-github-bot commented Aug 17, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53403/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
atlowChemi
atlowChemi approved these changes Aug 17, 2023
View reviewed changes
Comment thread test/parallel/test-dotenv-node-options.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
@anonrig anonrig mentioned this pull request Aug 17, 2023
.env file support issue tracker #49148
Open
8 tasks done
@github-actions github-actions Bot mentioned this pull request Aug 18, 2023
CI Reliability 2023-08-18 nodejs/reliability#640
Open
32 tasks
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 18, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 18, 2023
atlowChemi
atlowChemi approved these changes Aug 19, 2023
View reviewed changes
@anonrig anonrig added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Aug 19, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 19, 2023
@nodejs-github-bot

nodejs-github-bot commented Aug 19, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53447/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
This was referenced Aug 21, 2023
CI Reliability 2023-08-21 nodejs/reliability#641
Open
CI Reliability 2023-08-22 nodejs/reliability#642
Open
CI Reliability 2023-08-23 nodejs/reliability#643
Open
@aduh95

aduh95 commented Aug 26, 2023

Copy link
Copy Markdown
Contributor

parallel.test-dotenv-node-options is failing on a few platforms
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@aduh95 aduh95 removed the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Aug 26, 2023
@tniessen

tniessen commented Aug 27, 2023

Copy link
Copy Markdown
Member

Looking at the error messages, this is likely due to some platforms not running the node-api tests, either not at all or not on the same machine that runs this test. Does that sound right @richardlau? If that's the case, perhaps just skip the test if the file is missing.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@richardlau

richardlau commented Aug 29, 2023

Copy link
Copy Markdown
Member

Looking at the error messages, this is likely due to some platforms not running the node-api tests, either not at all or not on the same machine that runs this test. Does that sound right @richardlau?

Yes, that is correct. If the test needs the addon from test/node-api/test_uv_threadpool_size it should live in test/node-api/test_uv_threadpool_size.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@tniessen

tniessen commented Aug 29, 2023

Copy link
Copy Markdown
Member

I agree that that makes sense, but FWIW, there is precedent for skipping parts of tests when a shared library is missing:

node/test/addons/openssl-test-engine/test.js

Lines 34 to 38 in 9cd70f4

const engine = path.join(__dirname,
`/build/${common.buildType}/testsetengine.engine`);
if (!fs.existsSync(engine))
common.skip('no engine');

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@richardlau

richardlau commented Aug 29, 2023

Copy link
Copy Markdown
Member

@tniessen In that particular case I think it's because the engine is not compiled on all platforms:

node/test/addons/openssl-test-engine/binding.gyp

Lines 8 to 11 in 9cd70f4

['(OS=="mac" or OS=="linux") and '
'node_use_openssl=="true" and '
'node_shared=="false" and '
'node_shared_openssl=="false"', {

That is not the case for test/node-api/test_uv_threadpool_size, which is compiled for all platforms.
tniessen reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@anonrig
anonrig force-pushed the test-dotenv-improve branch from e4c08f4 to e24414c Compare August 29, 2023 18:49
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 29, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Aug 29, 2023
@nodejs-github-bot

nodejs-github-bot commented Aug 29, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53628/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
anonrig
anonrig commented Aug 30, 2023
View reviewed changes
Comment thread test/node-api/test_uv_threadpool_size/node-options.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
@GeoffreyBooth GeoffreyBooth added the cli Issues and PRs related to the Node.js command-line interface. label Sep 2, 2023
@anonrig

anonrig commented Oct 31, 2023

Copy link
Copy Markdown
Member Author

@anonrig any thoughts on my findings/suggestions?

Thank you @StefanStojanovic. I've applied your suggestions.
StefanStojanovic reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@anonrig anonrig added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 31, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 31, 2023
@nodejs-github-bot

nodejs-github-bot commented Oct 31, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/55361/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Oct 31, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/55370/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot mentioned this pull request Nov 1, 2023
CI Reliability 2023-11-01 nodejs/reliability#704
Open
24 tasks
@nodejs-github-bot

nodejs-github-bot commented Nov 1, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/55373/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
StefanStojanovic
StefanStojanovic approved these changes Nov 1, 2023
View reviewed changes

@StefanStojanovic StefanStojanovic left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
This was referenced Nov 2, 2023
CI Reliability 2023-11-02 nodejs/reliability#705
Open
CI Reliability 2023-11-03 nodejs/reliability#706
Open
@anonrig anonrig added the commit-queue PRs queued for automated landing through the Commit Queue. label Nov 3, 2023
@nodejs-github-bot nodejs-github-bot added commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. and removed commit-queue PRs queued for automated landing through the Commit Queue. labels Nov 3, 2023
@nodejs-github-bot

nodejs-github-bot commented Nov 3, 2023

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/49213
✔  Done loading data for nodejs/node/pull/49213
----------------------------------- PR info ------------------------------------
Title      test: improve `UV_THREADPOOL_SIZE` tests on `.env` (#49213)
Author     Yagiz Nizipli  (@anonrig)
Branch     anonrig:test-dotenv-improve -> nodejs:main
Labels     test, cli, needs-ci
Commits    1
 - test: improve `UV_THREADPOOL_SIZE` tests on `.env`
Committers 1
 - Yagiz Nizipli 
PR-URL: https://github.com/nodejs/node/pull/49213
Reviewed-By: Chemi Atlow 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/49213
Reviewed-By: Chemi Atlow 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - test: improve `UV_THREADPOOL_SIZE` tests on `.env`
   ℹ  This PR was created on Thu, 17 Aug 2023 14:19:12 GMT
   ✔  Approvals: 1
   ✔  - Chemi Atlow (@atlowChemi): https://github.com/nodejs/node/pull/49213#pullrequestreview-1585715138
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2023-11-01T01:47:03Z: https://ci.nodejs.org/job/node-test-pull-request/55373/
- Querying data for job/node-test-pull-request/55373/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/6748150235
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
GeoffreyBooth
GeoffreyBooth approved these changes Nov 3, 2023
View reviewed changes
@anonrig anonrig added commit-queue PRs queued for automated landing through the Commit Queue. and removed commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. labels Nov 3, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Nov 3, 2023
@nodejs-github-bot
nodejs-github-bot merged commit 4dbb017 into nodejs:main Nov 3, 2023
@nodejs-github-bot

nodejs-github-bot commented Nov 3, 2023

Copy link
Copy Markdown
Collaborator

Landed in 4dbb017
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
This was referenced Nov 4, 2023
CI Reliability 2023-11-04 nodejs/reliability#707
Open
CI Reliability 2023-11-05 nodejs/reliability#708
Open
CI Reliability 2023-11-06 nodejs/reliability#709
Open
CI Reliability 2023-11-07 nodejs/reliability#710
Open
targos pushed a commit that referenced this pull request Nov 11, 2023
@anonrig @targos
test: improve UV_THREADPOOL_SIZE tests on .env
00a8ecd 
PR-URL: #49213
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
@targos targos mentioned this pull request Nov 12, 2023
v21.2.0 release proposal #50681
Merged
targos pushed a commit that referenced this pull request Nov 14, 2023
@anonrig @targos
test: improve UV_THREADPOOL_SIZE tests on .env
8487cac 
PR-URL: #49213
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
@anonrig @UlisesGascon
test: improve UV_THREADPOOL_SIZE tests on .env
41644ee 
PR-URL: #49213
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
@UlisesGascon UlisesGascon mentioned this pull request Dec 12, 2023
v20.11.0 proposal #51124
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tniessen tniessen tniessen left review comments
@GeoffreyBooth GeoffreyBooth GeoffreyBooth approved these changes
@StefanStojanovic StefanStojanovic StefanStojanovic approved these changes
@atlowChemi atlowChemi atlowChemi approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

cli Issues and PRs related to the Node.js command-line interface. needs-ci PRs that need a full CI run. test Issues and PRs related to Node.js core tests and test infrastructure.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@anonrig @nodejs-github-bot @aduh95 @tniessen @richardlau @StefanStojanovic @GeoffreyBooth @atlowChemi
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.