Skip to content

test: fix flaky test-child-process-exec-abortcontroller-promisified#37572

Closed
aduh95 wants to merge 1 commit intonodejs:masterfrom
aduh95:fix-flaky-windows
Closed

test: fix flaky test-child-process-exec-abortcontroller-promisified#37572
aduh95 wants to merge 1 commit intonodejs:masterfrom
aduh95:fix-flaky-windows

Conversation

@aduh95
Copy link
Contributor

@aduh95 aduh95 commented Mar 2, 2021

Fixes: #37568

@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 Mar 2, 2021
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 2, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/36451/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 2, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/36452/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 2, 2021 •
edited by Trott
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/36456/ ✔️

@aduh95 aduh95 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 3, 2021
@Trott
Copy link
Member

Trott commented Mar 4, 2021

@nodejs/testing

@Trott Trott added flaky-test Issues and PRs related to the tests with unstable failures on the CI. and removed needs-ci PRs that need a full CI run. labels Mar 4, 2021
@aduh95
Copy link
Contributor Author

aduh95 commented Mar 4, 2021 •
edited
Loading

@Trott, I've seen you removed the needs-ci label. FYI when a PR is labelled needs-ci, it tells ncu to look for a green Jenkins CI. It's OK to remove the label here – if the label's not there, ncu browses through the list of files, and tries to determine if a passing Jenkins CI is necessary, which a test file definitely does – but I wanted to point out that the label may actually be useful to keep on some PR.

@Trott
Copy link
Member

Trott commented Mar 4, 2021

@aduh95 You're right that I misunderstood what the label means. I wonder if we might consider renaming labels whose only purpose is to signal information to ncu. For example, perhaps they should all have ncu- as a prefix, so ncu-needs-ci or something? Anyway, I'll go re-add that label now.

@Trott Trott added the needs-ci PRs that need a full CI run. label Mar 4, 2021
@aduh95 aduh95 force-pushed the fix-flaky-windows branch from 5a576c8 to cf59fd0 Compare March 4, 2021 21:41
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 4, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/36476/

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 5, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/36484/

@jasnell
Copy link
Member

jasnell commented Mar 5, 2021

Landed in 4fde7dc

@jasnell jasnell closed this Mar 5, 2021
jasnell pushed a commit that referenced this pull request Mar 5, 2021
@aduh95 @jasnell
test: fix flaky test-child-process-exec-abortcontroller-promisified
4fde7dc 
Fixes: #37568

PR-URL: #37572
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@aduh95 aduh95 deleted the fix-flaky-windows branch March 5, 2021 18:09
danielleadams pushed a commit that referenced this pull request Mar 16, 2021
@aduh95 @danielleadams
test: fix flaky test-child-process-exec-abortcontroller-promisified
0d04b6c 
Fixes: #37568

PR-URL: #37572
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@danielleadams danielleadams mentioned this pull request Mar 16, 2021
Merged
targos pushed a commit that referenced this pull request Sep 7, 2021
@aduh95 @targos
test: fix flaky test-child-process-exec-abortcontroller-promisified
64fb928 
Fixes: #37568

PR-URL: #37572
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@richardlau richardlau mentioned this pull request Jan 19, 2023
Closed
@richardlau richardlau mentioned this pull request Jan 19, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@richardlau richardlau richardlau left review comments

@jasnell jasnell jasnell approved these changes

@Trott Trott Trott approved these changes

+1 more reviewer

@Linkgoron Linkgoron Linkgoron left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. flaky-test Issues and PRs related to the tests with unstable failures on the CI. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

investigate flaky test-child-process-exec-abortcontroller-promisified on Windows CI

7 participants

@aduh95 @nodejs-github-bot @Trott @jasnell @Linkgoron @richardlau @targos