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: split heap snapshot limit tests #37189

Merged
merged 1 commit into from
Feb 2, 2021
Merged

test: split heap snapshot limit tests #37189

merged 1 commit into from
Feb 2, 2021

Conversation

Trott
Copy link
Member

@Trott Trott commented Feb 2, 2021

test/parallel/test-heapsnapshot-near-heap-limit.js is timing out in CI
on low-memory and slow-CPU devices. Split off the worker test to its own
test file to allow the test to finish in time.

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Feb 2, 2021
@nodejs-github-bot
Copy link
Collaborator

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

@Trott
Copy link
Member Author

Trott commented Feb 2, 2021

I'm leaving this in Draft mode until CI passes, but if it does, I'd like to fast-track this to fix CI. Please 👍 here to approve.

@Trott Trott added 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. labels Feb 2, 2021
This was referenced Feb 2, 2021
Closed
Closed
@Trott Trott marked this pull request as ready for review February 2, 2021 15:02
@Trott
Copy link
Member Author

Trott commented Feb 2, 2021

I've taken it out of draft mode so people can approve it to facilitate fast-tracking.

@RaisinTen
Copy link
Contributor

nit: maybe add this in the commit message while landing

Fixes: https://github.com/nodejs/node/issues/36961

@Trott
Copy link
Member Author

Trott commented Feb 2, 2021

nit: maybe add this in the commit message while landing

Fixes: https://github.com/nodejs/node/issues/36961

I don't think it will fix the macOS issue there. This only addresses the Raspberry Pi issue, which is a timeout. So maybe Refs: instead of Fixes: but otherwise, yeah, sure, let's add that when landing.

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

Trott commented Feb 2, 2021

CI is green. Landing.

@Trott
test: split heap snapshot limit tests
84e41d2 
test/parallel/test-heapsnapshot-near-heap-limit.js is timing out in CI
on low-memory and slow-CPU devices. Split off the worker test to its own
test file to allow the test to finish in time.

Refs: nodejs#36961
PR-URL: nodejs#37189
Reviewed-By: Ash Cripps <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@Trott Trott merged commit 84e41d2 into nodejs:master Feb 2, 2021
@Trott
Copy link
Member Author

Trott commented Feb 2, 2021

Landed in 84e41d2

@Trott Trott deleted the heap-test-fix branch February 2, 2021 17:26
danielleadams pushed a commit that referenced this pull request Feb 16, 2021
@Trott @danielleadams
test: split heap snapshot limit tests
13fe17c 
test/parallel/test-heapsnapshot-near-heap-limit.js is timing out in CI
on low-memory and slow-CPU devices. Split off the worker test to its own
test file to allow the test to finish in time.

Refs: #36961
PR-URL: #37189
Reviewed-By: Ash Cripps <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
This was referenced Feb 16, 2021
Closed
Merged
targos pushed a commit that referenced this pull request Aug 8, 2021
@Trott @targos
test: split heap snapshot limit tests
8625edb 
test/parallel/test-heapsnapshot-near-heap-limit.js is timing out in CI
on low-memory and slow-CPU devices. Split off the worker test to its own
test file to allow the test to finish in time.

Refs: #36961
PR-URL: #37189
Reviewed-By: Ash Cripps <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
BethGriggs pushed a commit that referenced this pull request Aug 12, 2021
@Trott @BethGriggs
test: split heap snapshot limit tests
5a79f6f 
test/parallel/test-heapsnapshot-near-heap-limit.js is timing out in CI
on low-memory and slow-CPU devices. Split off the worker test to its own
test file to allow the test to finish in time.

Refs: #36961
PR-URL: #37189
Reviewed-By: Ash Cripps <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
MylesBorins pushed a commit that referenced this pull request Aug 31, 2021
@Trott @MylesBorins
test: split heap snapshot limit tests
971d5be 
test/parallel/test-heapsnapshot-near-heap-limit.js is timing out in CI
on low-memory and slow-CPU devices. Split off the worker test to its own
test file to allow the test to finish in time.

Refs: #36961
PR-URL: #37189
Reviewed-By: Ash Cripps <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
@targos targos mentioned this pull request Sep 4, 2021
Merged
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
@Trott @foxxyz
test: split heap snapshot limit tests
0586567 
test/parallel/test-heapsnapshot-near-heap-limit.js is timing out in CI
on low-memory and slow-CPU devices. Split off the worker test to its own
test file to allow the test to finish in time.

Refs: nodejs#36961
PR-URL: nodejs#37189
Reviewed-By: Ash Cripps <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AshCripps AshCripps AshCripps approved these changes

@aduh95 aduh95 aduh95 approved these changes

@richardlau richardlau richardlau approved these changes

@jasnell jasnell jasnell approved these changes

@panva panva panva 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. flaky-test Issues and PRs related to the tests with unstable failures on the CI. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@Trott @nodejs-github-bot @RaisinTen @panva @jasnell @richardlau @aduh95 @AshCripps @targos