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

test: move test-worker-init-failure to sequential by Trott · Pull Request #34740 · nodejs/node · GitHub

/ node Public

test: move test-worker-init-failure to sequential - #34740

Closed
Trott wants to merge 2 commits into
nodejs:masterfrom
Trott:work-ini-seq
ConversationCommits (2)ChecksFiles changed
Closed

test: move test-worker-init-failure to sequential#34740
Trott wants to merge 2 commits into
nodejs:masterfrom
Trott:work-ini-seq

Conversation

Trott commented Aug 11, 2020

Copy link
Copy Markdown
Member

(Includes the change in #34727 to avoid subsequent merge conflict. This supplants it.)

Unfortunately, the test is sensitive to resource constraints and is
unreliable on macOS in CI when in parallel.

Fixes: #34727

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

Trott added 2 commits August 11, 2020 07:18
test: remove error message checking in test-worker-init-failure
5cf4c45 
Let the check for the error code suffice and don't check for a
particular string in the message.

Fixes: nodejs#33759
test: move test-worker-init-failure to sequential
346d4fe 
Unfortunately, the test is sensitive to resource constraints and is
unreliable on macOS in CI when in parallel.

Fixes: nodejs#34727
nodejs-github-bot added the test Issues and PRs related to the tests. label Aug 11, 2020

Trott commented Aug 11, 2020

Copy link
Copy Markdown
Member Author

Stress test that will hopefully show the issue on master: https://ci.nodejs.org/job/node-stress-single-test/165/
Stress test that will hopefully show the issue fixed with this PR: https://ci.nodejs.org/job/node-stress-single-test/166/

addaleax commented Aug 11, 2020

Copy link
Copy Markdown
Member

Do we know why the test is sensitive to resource constraints?

Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 12, 2020

Trott commented Aug 12, 2020

Copy link
Copy Markdown
Member Author

Do we know why the test is sensitive to resource constraints?

I'm guessing because it opens a lot of files (128).

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 12, 2020

nodejs-github-bot commented Aug 12, 2020

Copy link
Copy Markdown
Collaborator

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

Trott commented Aug 12, 2020

Copy link
Copy Markdown
Member Author

Do we know why the test is sensitive to resource constraints?

I'm guessing because it opens a lot of files (128).

An alternative we could experiment with is halving all the hard-coded numbers.

gireeshpunathil commented Aug 12, 2020

Copy link
Copy Markdown
Member

IMO the sensitivity to resource constraints is by design in the test, and it actually helped (and helps) to detect unforeseen control paths that are not covered in code and test, so is best placed to run in /parallel.

Trott closed this Aug 14, 2020
Trott deleted the work-ini-seq branch April 14, 2022 11:29
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

No reviews

Assignees

No one assigned

Labels

test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL