Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

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

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

@nodejs-github-bot
Copy link
Collaborator

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

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

@nodejs-github-bot
Copy link
Collaborator

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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@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 <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@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

@Linkgoron Linkgoron Linkgoron left review comments

@Trott Trott Trott approved these changes

@jasnell jasnell jasnell approved these changes

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