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: improve known_issues/test-vm-timeout-escape-queuemicrotask #25503

Closed
wants to merge 1 commit into from
Closed

test: improve known_issues/test-vm-timeout-escape-queuemicrotask #25503

wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Jan 14, 2019

Improve known_issues/test-vm-timeout-escape-queuemicrotask to mitigate
CI failures on ubuntu1604-arm64. Failures are due to a race condition.
Use common.platformTimeout() to help, adjust timeout to make sure
queueMicrotasks() has a chance to run, and improve error message.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@Trott
test: improve known_issues/test-vm-timeout-escape-queuemicrotask
ea51df3 
Improve known_issues/test-vm-timeout-escape-queuemicrotask to mitigate
CI failures on ubuntu1604-arm64. Failures are due to a race condition.
Use `common.platformTimeout()` to help, adjust timeout to make sure
`queueMicrotasks()` has a chance to run, and improve error message.
@nodejs-github-bot
Copy link
Collaborator

@Trott sadly an error occured when I tried to trigger a build :(

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jan 14, 2019
@Trott
Copy link
Member Author

Trott commented Jan 14, 2019 •
edited by refack
Loading

CI against master hopefully showing this flaky on ubuntu1604-arm64: https://ci.nodejs.org/job/node-stress-single-test/2132/ - (0/1000 OK)

CI against this PR hopefully showing it reliable on ubuntu1604-arm64: https://ci.nodejs.org/job/node-stress-single-test/2133/

CI (scheduled): https://ci.nodejs.org/job/node-test-pull-request/20126/

@refack
Copy link
Contributor

refack commented Jan 14, 2019

Seems to be overwhelmingly better. As in pre-PR 0/1000 iterations passed, and post-PR ATM all iterations pass (151/151). Just to make sure, both tests run on test-packetnet-ubuntu1604-arm64-2. In this case I think it makes sense to fast track. Please 👍 if you concur.

@refack refack added known limitation Issues that are identified as known limitations. flaky-test Issues and PRs related to the tests with unstable failures on the CI. fast-track PRs that do not need to wait for 48 hours to land. labels Jan 14, 2019
@Trott Trott mentioned this pull request Jan 14, 2019
Closed
4 tasks
bcoe
bcoe approved these changes Jan 15, 2019
View reviewed changes
Copy link
Contributor

@bcoe bcoe left a comment

Choose a reason for hiding this comment

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

🎉 thanks, have been wrestling with this trying to get a few branches to pass.

@Trott
Copy link
Member Author

Trott commented Jan 15, 2019

Unrelated failure in another test on Windows. Re-running just Windows (because still waiting for AIX which hasn't even started yet): https://ci.nodejs.org/job/node-test-commit-windows-fanned/23965/

@Trott
Copy link
Member Author

Trott commented Jan 15, 2019

Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/20132/

@Trott
Copy link
Member Author

Trott commented Jan 15, 2019 •
edited by refack
Loading

Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/20133/

Three failures in a row on Windows for parallel/test-trace-events-fs-sync. Starting to worry about that one....

@Trott
Copy link
Member Author

Trott commented Jan 15, 2019

Did a full CI re-run and this time Windows passed: https://ci.nodejs.org/job/node-test-commit/25058/

Re-running FreeBSD (only failure, unrelated): https://ci.nodejs.org/job/node-test-commit-freebsd/23400/

@Trott
Copy link
Member Author

Trott commented Jan 15, 2019

FreeBSD re-run was green. Landing...

Landed in 27f6d04

@Trott Trott closed this Jan 15, 2019
Trott added a commit to Trott/io.js that referenced this pull request Jan 15, 2019
@Trott
test: improve known_issues/test-vm-timeout-escape-queuemicrotask
27f6d04 
Improve known_issues/test-vm-timeout-escape-queuemicrotask to mitigate
CI failures on ubuntu1604-arm64. Failures are due to a race condition.
Use `common.platformTimeout()` to help, adjust timeout to make sure
`queueMicrotasks()` has a chance to run, and improve error message.

PR-URL: nodejs#25503
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ben Coe <[email protected]>
addaleax pushed a commit that referenced this pull request Jan 15, 2019
@Trott @addaleax
test: improve known_issues/test-vm-timeout-escape-queuemicrotask
0b1a9aa 
Improve known_issues/test-vm-timeout-escape-queuemicrotask to mitigate
CI failures on ubuntu1604-arm64. Failures are due to a race condition.
Use `common.platformTimeout()` to help, adjust timeout to make sure
`queueMicrotasks()` has a chance to run, and improve error message.

PR-URL: #25503
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ben Coe <[email protected]>
This was referenced Jan 16, 2019
Closed
Closed
@gireeshpunathil gireeshpunathil mentioned this pull request Jan 16, 2019
Closed
@BridgeAR BridgeAR mentioned this pull request Jan 16, 2019
Merged
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 16, 2019
@Trott @BridgeAR
test: improve known_issues/test-vm-timeout-escape-queuemicrotask
5be6ce0 
Improve known_issues/test-vm-timeout-escape-queuemicrotask to mitigate
CI failures on ubuntu1604-arm64. Failures are due to a race condition.
Use `common.platformTimeout()` to help, adjust timeout to make sure
`queueMicrotasks()` has a chance to run, and improve error message.

PR-URL: nodejs#25503
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ben Coe <[email protected]>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request Jan 17, 2019
@Trott @BridgeAR
test: improve known_issues/test-vm-timeout-escape-queuemicrotask
d7bc03e 
Improve known_issues/test-vm-timeout-escape-queuemicrotask to mitigate
CI failures on ubuntu1604-arm64. Failures are due to a race condition.
Use `common.platformTimeout()` to help, adjust timeout to make sure
`queueMicrotasks()` has a chance to run, and improve error message.

PR-URL: nodejs#25503
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ben Coe <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2019
Merged
@Trott Trott deleted the more-microtask branch January 13, 2022 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@addaleax addaleax addaleax approved these changes

@bcoe bcoe bcoe approved these changes

@refack refack refack approved these changes

Assignees
No one assigned
Labels
fast-track PRs that do not need to wait for 48 hours to land. flaky-test Issues and PRs related to the tests with unstable failures on the CI. known limitation Issues that are identified as known limitations. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Trott @nodejs-github-bot @refack @bcoe @addaleax