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.

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

test: fix cluster-disconnect-handles flakiness #4009

Conversation

santigimeno
Copy link
Member

Sometimes the test was timing out because the worker process remained stuck in
the breakpoint and didn't exit. This could happen because the continue was sent
before the breakpoint was set. If that's the case, with this change, a new
continue command is sent so the worker process can end.

It fixes for me the following error I was receiving in a Jessie 64bit box:

Path: parallel/test-cluster-disconnect-handles
Debugger listening on port 12446
Command: out/Release/node --expose_internals /home/sgimeno/node/node/test/parallel/test-cluster-disconnect-handles.js
--- TIMEOUT ---

const req = protocol.serialize({ command: 'continue' });
debugClient.write(req);
}
});
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be better to rewrite the protocol.onResponse logic to wait for the break event first before sending the continue command?

Thanks for looking into this, by the way. Looks like this might fix #3907.

@mscdex mscdex added cluster Issues and PRs related to the cluster subsystem. test Issues and PRs related to the tests. labels Nov 24, 2015
@santigimeno santigimeno force-pushed the fix-test-cluster-disconnect-handles branch from 8301a40 to d90a4f5 Compare November 25, 2015 09:21
@santigimeno
Copy link
Member Author

@bnoordhuis updated using protocol.onResponse. Thanks!

Sometimes the test was timing out because the worker process remained stuck in
the breakpoint and didn't exit. This could happen because the continue was sent
before the breakpoint was set. If that's the case, with this change, a new
continue command is sent so the worker process can end.
@santigimeno santigimeno force-pushed the fix-test-cluster-disconnect-handles branch from d90a4f5 to 1226417 Compare November 25, 2015 10:36
@bnoordhuis
Copy link
Member

@bnoordhuis
Copy link
Member

Thanks Santiago, landed in 8677627.

@bnoordhuis bnoordhuis closed this Dec 1, 2015
bnoordhuis pushed a commit that referenced this pull request Dec 1, 2015
Sometimes the test was timing out because the worker process remained
stuck in the breakpoint and didn't exit. This could happen because the
continue was sent before the breakpoint was set. If that's the case,
with this change, a new continue command is sent so the worker process
can end.

PR-URL: #4009
Reviewed-By: Ben Noordhuis <[email protected]>
rvagg pushed a commit that referenced this pull request Dec 5, 2015
Sometimes the test was timing out because the worker process remained
stuck in the breakpoint and didn't exit. This could happen because the
continue was sent before the breakpoint was set. If that's the case,
with this change, a new continue command is sent so the worker process
can end.

PR-URL: #4009
Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 15, 2015
Sometimes the test was timing out because the worker process remained
stuck in the breakpoint and didn't exit. This could happen because the
continue was sent before the breakpoint was set. If that's the case,
with this change, a new continue command is sent so the worker process
can end.

PR-URL: #4009
Reviewed-By: Ben Noordhuis <[email protected]>
@jasnell jasnell mentioned this pull request Dec 17, 2015
jasnell pushed a commit that referenced this pull request Dec 17, 2015
Sometimes the test was timing out because the worker process remained
stuck in the breakpoint and didn't exit. This could happen because the
continue was sent before the breakpoint was set. If that's the case,
with this change, a new continue command is sent so the worker process
can end.

PR-URL: #4009
Reviewed-By: Ben Noordhuis <[email protected]>
jasnell pushed a commit that referenced this pull request Dec 23, 2015
Sometimes the test was timing out because the worker process remained
stuck in the breakpoint and didn't exit. This could happen because the
continue was sent before the breakpoint was set. If that's the case,
with this change, a new continue command is sent so the worker process
can end.

PR-URL: #4009
Reviewed-By: Ben Noordhuis <[email protected]>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
Sometimes the test was timing out because the worker process remained
stuck in the breakpoint and didn't exit. This could happen because the
continue was sent before the breakpoint was set. If that's the case,
with this change, a new continue command is sent so the worker process
can end.

PR-URL: nodejs#4009
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cluster Issues and PRs related to the cluster subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants