FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: fix test-cluster-send-handle-large-payload by Trott · Pull Request #19311 · nodejs/node · GitHub

/ node Public

test: fix test-cluster-send-handle-large-payload - #19311

Closed
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:json-error
ConversationCommits (1)ChecksFiles changed
Closed

test: fix test-cluster-send-handle-large-payload#19311
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:json-error

Conversation

Trott commented Mar 13, 2018

Copy link
Copy Markdown
Member

test-cluster-send-handle-large-payload is susceptible to failure if it
is competing for resources. Move to sequential directory so it is not
competing with other tests.

Fixes: #14844

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

test: fix test-cluster-send-handle-large-payload
2403bfb 
test-cluster-send-handle-large-payload is susceptible to failure if it
is competing for resources. Move to `sequential` directory so it is not
competing with other tests.

Fixes: nodejs#14844
nodejs-github-bot added the test Issues and PRs related to the tests. label Mar 13, 2018

Trott commented Mar 13, 2018

Copy link
Copy Markdown
Member Author

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

santigimeno commented Mar 13, 2018

Copy link
Copy Markdown
Member

This will make the test less flaky but the freeBSD issue will remain.

Trott commented Mar 13, 2018

Copy link
Copy Markdown
Member Author

This will make the test less flaky but the freeBSD issue will remain.

@santigimeno Stress tests on FreeBSD show that there the test does not fail except when run in parallel (that is, with -j and --repeat set appropriately). In other words, I believe this will in fact fix the issue on FreeBSD (or at least make it vastly less frequent).

Trott added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Mar 13, 2018

santigimeno commented Mar 13, 2018

Copy link
Copy Markdown
Member

Yeah, I don't disagree. I meant that the underlying issue will still be there though less visible.

Trott commented Mar 13, 2018

Copy link
Copy Markdown
Member Author

Yeah, I don't disagree. I meant that the underlying issue will still be there though less visible.

Oh, I see. Yes. If we can find a way to trigger the issue reliably without consuming a ton of resources, we could create a known_issues test for it....

Trott commented Mar 15, 2018

Copy link
Copy Markdown
Member Author

Landed in 5e23b65

Trott closed this Mar 15, 2018
Trott added a commit to Trott/io.js that referenced this pull request Mar 15, 2018
test: fix test-cluster-send-handle-large-payload
5e23b65 
test-cluster-send-handle-large-payload is susceptible to failure if it
is competing for resources. Move to `sequential` directory so it is not
competing with other tests.

Fixes: nodejs#14844

PR-URL: nodejs#19311
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
targos pushed a commit that referenced this pull request Mar 17, 2018
test: fix test-cluster-send-handle-large-payload
03feca3 
test-cluster-send-handle-large-payload is susceptible to failure if it
is competing for resources. Move to `sequential` directory so it is not
competing with other tests.

Fixes: #14844

PR-URL: #19311
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
targos mentioned this pull request Mar 18, 2018
Merged
MylesBorins pushed a commit that referenced this pull request Mar 20, 2018
test: fix test-cluster-send-handle-large-payload
9aa5090 
test-cluster-send-handle-large-payload is susceptible to failure if it
is competing for resources. Move to `sequential` directory so it is not
competing with other tests.

Fixes: #14844

PR-URL: #19311
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
mhdawson mentioned this pull request Jun 7, 2018
Closed
4 tasks
MylesBorins pushed a commit that referenced this pull request Jul 31, 2018
test: fix test-cluster-send-handle-large-payload
093b777 
test-cluster-send-handle-large-payload is susceptible to failure if it
is competing for resources. Move to `sequential` directory so it is not
competing with other tests.

Fixes: #14844

PR-URL: #19311
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
rvagg pushed a commit that referenced this pull request Aug 16, 2018
test: fix test-cluster-send-handle-large-payload
0517cd8 
test-cluster-send-handle-large-payload is susceptible to failure if it
is competing for resources. Move to `sequential` directory so it is not
competing with other tests.

Fixes: #14844

PR-URL: #19311
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins mentioned this pull request Aug 16, 2018
Merged
Trott deleted the json-error branch January 13, 2022 22:48
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
danbev danbev approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. 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 parallel/test-cluster-send-handle-large-payload failure on FreeBSD

5 participants

Back | FazBrowse Home | New Git URL