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: disable warnings to fix flaky test #40739

Closed
wants to merge 1 commit into from
Closed

test: disable warnings to fix flaky test #40739

wants to merge 1 commit into from

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Nov 6, 2021

Fixes: #40738

@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 Nov 6, 2021
@aduh95 aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 6, 2021
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 6, 2021
@nodejs-github-bot

This comment has been minimized.

Sign in to view
targos
targos approved these changes Nov 6, 2021
View reviewed changes
Copy link
Member

@targos targos left a comment

Choose a reason for hiding this comment

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

LGTM, but do you know why the test doesn't fail all the time on all platforms?

@targos targos added the fast-track PRs that do not need to wait for 48 hours to land. label Nov 6, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2021

Fast-track has been requested by @targos. Please 👍 to approve.

@nodejs-github-bot
Copy link
Collaborator

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

@aduh95
Copy link
Contributor Author

aduh95 commented Nov 6, 2021 •
edited
Loading

LGTM, but do you know why the test doesn't fail all the time on all platforms?

Note sure, it could be related to something like a race condition between the warning being emitted asynchronously and the process crashing.

@github-actions github-actions bot mentioned this pull request Nov 7, 2021
Open
46 tasks
@nodejs-github-bot
Copy link
Collaborator

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

@Mesteery Mesteery added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 7, 2021
@github-actions github-actions bot mentioned this pull request Nov 8, 2021
Open
46 tasks
@nodejs-github-bot
Copy link
Collaborator

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

@targos targos mentioned this pull request Nov 8, 2021
Closed
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@targos
Copy link
Member

targos commented Nov 8, 2021

Landed in a420da6

@targos targos closed this Nov 8, 2021
targos pushed a commit that referenced this pull request Nov 8, 2021
@aduh95 @targos
test: disable warnings to fix flaky test
a420da6 
Fixes: #40738

PR-URL: #40739
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request Nov 8, 2021
@aduh95 @targos
test: disable warnings to fix flaky test
e58cce4 
Fixes: #40738

PR-URL: #40739
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@targos targos mentioned this pull request Nov 8, 2021
Merged
@aduh95 aduh95 deleted the fix-flaky-json-warning-in-message branch November 8, 2021 18:00
This was referenced Nov 9, 2021
Open
Open
Open
This was referenced Nov 12, 2021
Open
Open
@danielleadams
Copy link
Contributor

@aduh95 I think this is related to #40250. Do you mind backporting this to v16.x-staging too?

aduh95 added a commit to aduh95/node that referenced this pull request Jan 30, 2022
@aduh95
test: disable warnings to fix flaky test
8272b87 
Fixes: nodejs#40738

PR-URL: nodejs#40739
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@aduh95 aduh95 mentioned this pull request Jan 30, 2022
Closed
aduh95 added a commit to aduh95/node that referenced this pull request Jan 31, 2022
@aduh95
test: disable warnings to fix flaky test
0d728a3 
Fixes: nodejs#40738

PR-URL: nodejs#40739
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Feb 1, 2022
@aduh95 @danielleadams
test: disable warnings to fix flaky test
b4fea14 
Fixes: #40738

PR-URL: #40739
Backport-PR-URL: #41776
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@danielleadams danielleadams mentioned this pull request Feb 1, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@targos targos targos approved these changes

@lpinca lpinca lpinca 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. fast-track PRs that do not need to wait for 48 hours to land. 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 - esm_display_syntax_error_import_json_named_export
6 participants
@aduh95 @nodejs-github-bot @targos @danielleadams @lpinca @Mesteery