Skip to content

Conversation

@richardlau
Copy link
Member

@richardlaurichardlau commented Jan 19, 2023

Extend the Linux logic in test-child-process-exec-abortcontroller-promisified to all POSIX platforms.

Fixes: nodejs/build#3154
Refs: #37572 (comment)
Refs: #37518


This unblocks the SmartOS CI for Node.js 14 and 16 that has been failing since we migrated the smartos machines at the end of last year. I'm opening this on main (which hasn't been failing) so I can cherry pick the same fix to both v16.x-staging and v14.x-staging. My main aim here is to unblock the CI.

To clarify, the test itself passes but is leaving behind child processes, which cause the CI run to fail as it checks for left over node processes at the end of the test run.

cc @nodejs/releasers @bahamat

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Jan 19, 2023
@richardlaurichardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 19, 2023
Extend the Linux logic to all POSIX platforms in test-child-process-exec-abortcontroller-promisified.
@richardlaurichardlau removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 19, 2023
@nodejs-github-bot

This comment was marked as outdated.

@richardlaurichardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 19, 2023
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 19, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@richardlaurichardlau added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 20, 2023
@lpincalpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 20, 2023
Copy link
Member

@mhdawsonmhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@mhdawsonmhdawson added the fast-track PRs that do not need to wait for 48 hours to land. label Jan 20, 2023
@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 Jan 20, 2023
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/46276 ✔ Done loading data for nodejs/node/pull/46276 ----------------------------------- PR info ------------------------------------ Title test: avoid left behind child processes (#46276) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch richardlau:cpexecac -> nodejs:main Labels test, fast-track, author ready, needs-ci, lts-watch-v14.x, lts-watch-v16.x Commits 1 - test: avoid left behind child processes Committers 1 - Richard Lau PR-URL: https://github.com/nodejs/node/pull/46276 Fixes: https://github.com/nodejs/build/issues/3154 Refs: https://github.com/nodejs/node/issues/37518 Reviewed-By: Moshe Atlow Reviewed-By: Beth Griggs Reviewed-By: Ruy Adorno Reviewed-By: Colin Ihrig Reviewed-By: Luigi Pinca Reviewed-By: Michaël Zasso Reviewed-By: Michael Dawson ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/46276 Fixes: https://github.com/nodejs/build/issues/3154 Refs: https://github.com/nodejs/node/issues/37518 Reviewed-By: Moshe Atlow Reviewed-By: Beth Griggs Reviewed-By: Ruy Adorno Reviewed-By: Colin Ihrig Reviewed-By: Luigi Pinca Reviewed-By: Michaël Zasso Reviewed-By: Michael Dawson -------------------------------------------------------------------------------- ℹ This PR was created on Thu, 19 Jan 2023 16:48:37 GMT ✔ Approvals: 7 ✔ - Moshe Atlow (@MoLow): https://github.com/nodejs/node/pull/46276#pullrequestreview-1262194862 ✔ - Beth Griggs (@BethGriggs) (TSC): https://github.com/nodejs/node/pull/46276#pullrequestreview-1262204146 ✔ - Ruy Adorno (@ruyadorno): https://github.com/nodejs/node/pull/46276#pullrequestreview-1262207413 ✔ - Colin Ihrig (@cjihrig) (TSC): https://github.com/nodejs/node/pull/46276#pullrequestreview-1262429139 ✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/46276#pullrequestreview-1263218104 ✔ - Michaël Zasso (@targos) (TSC): https://github.com/nodejs/node/pull/46276#pullrequestreview-1263219222 ✔ - Michael Dawson (@mhdawson) (TSC): https://github.com/nodejs/node/pull/46276#pullrequestreview-1264226655 ℹ This PR is being fast-tracked ✖ The fast-track request requires at least one collaborator's approval (👍). ✔ Last GitHub CI successful ℹ Last Full PR CI on 2023-01-19T23:07:22Z: https://ci.nodejs.org/job/node-test-pull-request/49094/ - Querying data for job/node-test-pull-request/49094/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/3970755114

@aduh95aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. and removed fast-track PRs that do not need to wait for 48 hours to land. commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jan 22, 2023
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 22, 2023
@nodejs-github-botnodejs-github-bot merged commit 2df72fe into nodejs:mainJan 22, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 2df72fe

richardlau added a commit to richardlau/node-1 that referenced this pull request Jan 22, 2023
Extend the Linux logic to all POSIX platforms in test-child-process-exec-abortcontroller-promisified. PR-URL: nodejs#46276Fixes: nodejs/build#3154 Refs: nodejs#37518 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Ruy Adorno <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: James M Snell <[email protected]>
richardlau added a commit that referenced this pull request Jan 22, 2023
Extend the Linux logic to all POSIX platforms in test-child-process-exec-abortcontroller-promisified. PR-URL: #46276Fixes: nodejs/build#3154 Refs: #37518 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Ruy Adorno <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: James M Snell <[email protected]>
@richardlaurichardlau deleted the cpexecac branch January 30, 2023 12:21
ruyadorno pushed a commit that referenced this pull request Feb 1, 2023
Extend the Linux logic to all POSIX platforms in test-child-process-exec-abortcontroller-promisified. PR-URL: #46276Fixes: nodejs/build#3154 Refs: #37518 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Ruy Adorno <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: James M Snell <[email protected]>
@ruyadornoruyadorno mentioned this pull request Feb 1, 2023
juanarbol pushed a commit that referenced this pull request Mar 3, 2023
Extend the Linux logic to all POSIX platforms in test-child-process-exec-abortcontroller-promisified. PR-URL: #46276Fixes: nodejs/build#3154 Refs: #37518 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Ruy Adorno <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: James M Snell <[email protected]>
@juanarboljuanarbol mentioned this pull request Mar 3, 2023
juanarbol pushed a commit that referenced this pull request Mar 5, 2023
Extend the Linux logic to all POSIX platforms in test-child-process-exec-abortcontroller-promisified. PR-URL: #46276Fixes: nodejs/build#3154 Refs: #37518 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Ruy Adorno <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: James M Snell <[email protected]>
oraluben pushed a commit to oraluben/node that referenced this pull request Mar 14, 2023
Extend the Linux logic to all POSIX platforms in test-child-process-exec-abortcontroller-promisified. PR-URL: nodejs/node#46276Fixes: nodejs/build#3154 Refs: nodejs/node#37518 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Ruy Adorno <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: James M Snell <[email protected]>
oraluben pushed a commit to oraluben/node that referenced this pull request Mar 17, 2023
Extend the Linux logic to all POSIX platforms in test-child-process-exec-abortcontroller-promisified. PR-URL: nodejs/node#46276Fixes: nodejs/build#3154 Refs: nodejs/node#37518 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Ruy Adorno <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: James M Snell <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.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.

node-daily-v14.x-staging is failing on smartos

12 participants

@richardlau@nodejs-github-bot@mhdawson@ruyadorno@jasnell@lpinca@targos@cjihrig@MoLow@BethGriggs@RafaelGSS@aduh95