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: move test-benchmark-buffer to sequential #15373

Closed
wants to merge 1 commit into from
Closed

test: move test-benchmark-buffer to sequential #15373

wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Sep 12, 2017

test-benchmark-buffer has been observed to timeout on CI on SmartOS.
Move the test to sequential so it is not competing with other tests
for resources.

Fixes: #15372

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

test benchmark buffer

@Trott
test: move test-benchmark-buffer to sequential
5d698bc 
`test-benchmark-buffer` has been observed to timeout on CI on SmartOS.
Move the test to `sequential` so it is not competing with other tests
for resources.

Fixes: nodejs#15372
@Trott Trott added the test Issues and PRs related to the tests. label Sep 12, 2017
@Trott Trott mentioned this pull request Sep 12, 2017
Closed
@mscdex mscdex added benchmark Issues and PRs related to the benchmark subsystem. buffer Issues and PRs related to the buffer subsystem. labels Sep 12, 2017
@refack refack mentioned this pull request Sep 12, 2017
Closed
3 tasks
@Trott
Copy link
Member Author

Trott commented Sep 12, 2017

Stress test (started by @mscdex) to see if it's not a problem with the test itself: https://ci.nodejs.org/job/node-stress-single-test/1409/

If that stress test comes back green, that would suggest that moving the test to sequential is a solution (or at least an effective workaround).

If that stress test comes back red, that would suggest that moving the test to sequential would not eliminate the test flakiness (although it would likely reduce test flakiness).

@refack
Copy link
Contributor

refack commented Sep 13, 2017

Cross-ref: https://ci.nodejs.org/job/node-stress-single-test/1409/ was stopped after 19h (OK: 1934 NOT OK: 0). I'd say it's stable on it's own.

@jasnell
Copy link
Member

jasnell commented Sep 14, 2017

This needs a rebase.

@jasnell
Copy link
Member

jasnell commented Sep 14, 2017

..nvm .. I rebased on landing...

jasnell pushed a commit that referenced this pull request Sep 14, 2017
@Trott @jasnell
test: move test-benchmark-buffer to sequential
082c434 
`test-benchmark-buffer` has been observed to timeout on CI on SmartOS.
Move the test to `sequential` so it is not competing with other tests
for resources.

PR-URL: #15373
Fixes: #15372
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Sep 14, 2017

Landed in 082c434

@jasnell jasnell closed this Sep 14, 2017
addaleax pushed a commit to addaleax/ayo that referenced this pull request Sep 17, 2017
@Trott @addaleax
test: move test-benchmark-buffer to sequential
d2722fb 
`test-benchmark-buffer` has been observed to timeout on CI on SmartOS.
Move the test to `sequential` so it is not competing with other tests
for resources.

PR-URL: nodejs/node#15373
Fixes: nodejs/node#15372
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
jasnell pushed a commit that referenced this pull request Sep 20, 2017
@Trott @jasnell
test: move test-benchmark-buffer to sequential
4d26c68 
`test-benchmark-buffer` has been observed to timeout on CI on SmartOS.
Move the test to `sequential` so it is not competing with other tests
for resources.

PR-URL: #15373
Fixes: #15372
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell jasnell mentioned this pull request Sep 20, 2017
Merged
Qard pushed a commit to Qard/ayo that referenced this pull request Sep 21, 2017
@Trott
test: move test-benchmark-buffer to sequential
9f8e9b1 
`test-benchmark-buffer` has been observed to timeout on CI on SmartOS.
Move the test to `sequential` so it is not competing with other tests
for resources.

PR-URL: nodejs/node#15373
Fixes: nodejs/node#15372
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott Trott deleted the mv-buffer-to-seq branch January 13, 2022 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@refack refack refack approved these changes

@jasnell jasnell jasnell approved these changes

@evanlucas evanlucas evanlucas approved these changes

@cjihrig cjihrig cjihrig approved these changes

@hiroppy hiroppy hiroppy approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

Assignees
No one assigned
Labels
benchmark Issues and PRs related to the benchmark subsystem. buffer Issues and PRs related to the buffer subsystem. 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 parallel/test-benchmark-buffer.js on SmartOS
9 participants
@Trott @refack @jasnell @evanlucas @cjihrig @hiroppy @BridgeAR @mscdex @MylesBorins