Skip to content

test: increase bufsize in child process write test - #13626

Closed
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:is-that-freedom-rock-well-turn-it-up
Closed

test: increase bufsize in child process write test#13626
Trott wants to merge 1 commit into
nodejs:masterfrom
Trott:is-that-freedom-rock-well-turn-it-up

Conversation

@Trott

Copy link
Copy Markdown
Member

test-child-process-stdio-big-write-end was failing on ubuntu1604-arm64
because the while loop that was supposed to fill up the buffer ended up
being an infinite loop.

This increases the size of the writes in the loop by 1K until the buffer
fills up.

Fixes: #13603

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

test child_process

test-child-process-stdio-big-write-end was failing on ubuntu1604-arm64
because the while loop that was supposed to fill up the buffer ended up
being an infinite loop.
This increases the size of the writes in the loop by 1K until the buffer
fills up.
Fixes: nodejs#13603
@TrottTrott added child_process Issues and PRs related to the child_process subsystem. test Issues and PRs related to the tests. labels Jun 12, 2017
@Trott
Trottforce-pushed the is-that-freedom-rock-well-turn-it-up branch from 77aab91 to 30c1e00CompareJune 12, 2017 04:14
@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Jun 12, 2017
@Trott

Copy link
Copy Markdown
MemberAuthor

@Trott

Copy link
Copy Markdown
MemberAuthor

ubuntu1604-arm64 is green in the CI! Victory! (Unless something else comes back red...)

@Trott

Copy link
Copy Markdown
MemberAuthor

I'd like to expedite this to get CI back to green.

/cc @nodejs/testing

And a rare /cc @isaacs because they wrote the test originally and may know reasons this approach may be inadvisable.

@Trott

Copy link
Copy Markdown
MemberAuthor

CI is green!

do {
buf = Buffer.alloc(BUFSIZE, '.');
sent += BUFSIZE;
bufsize += 1024;

@refackrefackJun 12, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Now it definitely looks like a /pummel/...
But:

ok 184 parallel/test-child-process-stdio-big-write-end
---
duration_ms: 0.332

🤷‍♂️

@refack

Copy link
Copy Markdown
Contributor

I'd like to expedite this to get CI back to green.

👍

@gibfahn

Copy link
Copy Markdown
Member

test-child-process-stdio-big-write-end was failing on ubuntu1604-arm64
because the while loop that was supposed to fill up the buffer ended up
being an infinite loop.

Probably a stupid question, but why exactly does the buffer fill become an infinite loop on this machine? Is it because there's so much RAM?

@tniessen

Copy link
Copy Markdown
Member

@gibfahn This is not necessarily caused by too much RAM, but the child process might just clear the buffer too quickly. I think Linux uses something around 64k as the pipe buffer (not all the RAM!).

@Trott

Copy link
Copy Markdown
MemberAuthor

Landed in b71d677

@TrottTrott closed this Jun 12, 2017
Trott added a commit to Trott/io.js that referenced this pull request Jun 12, 2017
test-child-process-stdio-big-write-end was failing on ubuntu1604-arm64
because the while loop that was supposed to fill up the buffer ended up
being an infinite loop.
This increases the size of the writes in the loop by 1K until the buffer
fills up.
PR-URL: nodejs#13626Fixes: nodejs#13603
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
addaleax pushed a commit that referenced this pull request Jun 17, 2017
test-child-process-stdio-big-write-end was failing on ubuntu1604-arm64
because the while loop that was supposed to fill up the buffer ended up
being an infinite loop.
This increases the size of the writes in the loop by 1K until the buffer
fills up.
PR-URL: #13626Fixes: #13603
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@addaleaxaddaleax mentioned this pull request Jun 17, 2017
addaleax pushed a commit that referenced this pull request Jun 21, 2017
test-child-process-stdio-big-write-end was failing on ubuntu1604-arm64
because the while loop that was supposed to fill up the buffer ended up
being an infinite loop.
This increases the size of the writes in the loop by 1K until the buffer
fills up.
PR-URL: #13626Fixes: #13603
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@addaleaxaddaleax mentioned this pull request Jun 21, 2017
MylesBorins pushed a commit that referenced this pull request Jul 17, 2017
test-child-process-stdio-big-write-end was failing on ubuntu1604-arm64
because the while loop that was supposed to fill up the buffer ended up
being an infinite loop.
This increases the size of the writes in the loop by 1K until the buffer
fills up.
PR-URL: #13626Fixes: #13603
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorinsMylesBorins mentioned this pull request Jul 18, 2017
@Trott
Trott deleted the is-that-freedom-rock-well-turn-it-up branch January 13, 2022 22:45
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

child_processIssues and PRs related to the child_process subsystem.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test: investigate flaky test-child-process-stdio-big-write-end

8 participants

@Trott@refack@gibfahn@tniessen@cjihrig@aqrln@MylesBorins@nodejs-github-bot