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: cover dgram handle send failures #13158

Merged
merged 1 commit into from
May 25, 2017
Merged

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented May 22, 2017

This commit adds test coverage for the case where a dgram socket successfully binds, but the handle's send() function fails.

This was previously covered, but the recent churn in the dgram tests seems to have undone that coverage.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label May 22, 2017
@mscdex mscdex added the dgram Issues and PRs related to the dgram subsystem / UDP. label May 22, 2017
return errCode;
};

socket.send('foo', port, 'localhost', callback);
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it doesn't really matter in this case, but maybe common.localhostIPv4

assert.strictEqual(err.syscall, 'send');
assert.strictEqual(err.address, 'localhost');
assert.strictEqual(err.port, port);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add an assertion for the message format. Something like:

assert.strictEqual(
    err.message,
    `${err.syscall} ${err.code} ${err.address}:${err.port} ` +
    `- Local (${err.localAddress}:${err.localPort})`
  );

Silly but cheap.

@cjihrig
Copy link
Contributor Author

cjihrig commented May 25, 2017

This commit adds test coverage for the case where a dgram socket
successfully binds, but the handle's send() function fails.

PR-URL: nodejs#13158
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@cjihrig cjihrig merged commit 06757cd into nodejs:master May 25, 2017
@cjihrig cjihrig deleted the coverage branch May 25, 2017 18:03
jasnell pushed a commit that referenced this pull request May 25, 2017
This commit adds test coverage for the case where a dgram socket
successfully binds, but the handle's send() function fails.

PR-URL: #13158
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
jasnell pushed a commit that referenced this pull request May 28, 2017
This commit adds test coverage for the case where a dgram socket
successfully binds, but the handle's send() function fails.

PR-URL: #13158
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@jasnell jasnell mentioned this pull request May 28, 2017
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
3 tasks
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.

7 participants