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: refactor test-dgram-send-callback-recursive #5079

Conversation

santigimeno
Copy link
Member

Just send 10 messages recursively and check that the send calls are
asynchronous by asserting that a setImmediate callback has been called
in-between. It avoids a race condition in the test where the recursive
limit is reached without having received at least 10 messages.

I was sometimes getting this error in OS X:

/Users/sgimeno/node/node/test/parallel/test-dgram-send-callback-recursive.js:15
    throw new Error('infinite loop detected');
    ^

Error: infinite loop detected
    at SendWrap.onsend [as callback] (/Users/sgimeno/node/node/test/parallel/test-dgram-send-callback-recursive.js:15:11)
    at SendWrap.afterSend [as oncomplete] (dgram.js:383:8)

I've verified this test fails with node before the commit where this test was introduced: 18d457b and passes after this commit: b5cd2f0 when the libuv implementation of udp send was made asynchronous.

@jasnell jasnell added lts-watch-v4.x test Issues and PRs related to the tests. dgram Issues and PRs related to the dgram subsystem / UDP. labels Feb 4, 2016
@r-52
Copy link
Contributor

r-52 commented Feb 4, 2016

client.send(
chunk, 0, chunk.length, common.PORT, common.localhostIPv4, onsend);
} else {
assert.equal(async, true, 'Send should be asynchronous.');
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use strictEqual() here.

@cjihrig
Copy link
Contributor

cjihrig commented Feb 4, 2016

LGTM with one comment.

Just send 10 messages recursively and check that the send calls are
asynchronous by asserting that a `setImmediate` callback has been called
in-between. It avoids a race condition in the test when the recursive
limit is reached without having received at least 10 messages.
@santigimeno santigimeno force-pushed the test-dgram-send-callback-recursive branch from f6c274b to 9616b88 Compare February 4, 2016 19:22
@santigimeno
Copy link
Member Author

Updated. Thanks!

@mcollina
Copy link
Member

LGTM

mcollina pushed a commit to mcollina/node that referenced this pull request Feb 26, 2016
Just send 10 messages recursively and check that the send calls are
asynchronous by asserting that a `setImmediate` callback has been called
in-between. It avoids a race condition in the test when the recursive
limit is reached without having received at least 10 messages.

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#5079
@mcollina
Copy link
Member

Landed in 8872840.

@mcollina mcollina closed this Feb 26, 2016
rvagg pushed a commit that referenced this pull request Feb 27, 2016
Just send 10 messages recursively and check that the send calls are
asynchronous by asserting that a `setImmediate` callback has been called
in-between. It avoids a race condition in the test when the recursive
limit is reached without having received at least 10 messages.

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: #5079
rvagg pushed a commit that referenced this pull request Feb 27, 2016
Just send 10 messages recursively and check that the send calls are
asynchronous by asserting that a `setImmediate` callback has been called
in-between. It avoids a race condition in the test when the recursive
limit is reached without having received at least 10 messages.

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: #5079
@Fishrock123 Fishrock123 mentioned this pull request Mar 1, 2016
5 tasks
MylesBorins pushed a commit that referenced this pull request Mar 1, 2016
Just send 10 messages recursively and check that the send calls are
asynchronous by asserting that a `setImmediate` callback has been called
in-between. It avoids a race condition in the test when the recursive
limit is reached without having received at least 10 messages.

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: #5079
MylesBorins pushed a commit that referenced this pull request Mar 1, 2016
Just send 10 messages recursively and check that the send calls are
asynchronous by asserting that a `setImmediate` callback has been called
in-between. It avoids a race condition in the test when the recursive
limit is reached without having received at least 10 messages.

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: #5079
MylesBorins pushed a commit that referenced this pull request Mar 2, 2016
Just send 10 messages recursively and check that the send calls are
asynchronous by asserting that a `setImmediate` callback has been called
in-between. It avoids a race condition in the test when the recursive
limit is reached without having received at least 10 messages.

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: #5079
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dgram Issues and PRs related to the dgram subsystem / UDP. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants