Skip to content

Commit

Permalink
test,dgram: add tests for setBroadcast()
Browse files Browse the repository at this point in the history
The only tests for `setBroadcast()` (from the `dgram` module) were in
`test/internet` which means they almost never get run. This adds a
minimal test that can check JS-land functionality in `test/parallel`.

I also expanded a comment and did some minor formatting on the existing
`test/internet` test. If there were an easy and reliable way to check
for the BROADCAST flag on an interface, it's possible that a version of
the test could be moved to `test/sequential` or `test/parallel` once it
was modified to only use internal networks.

PR-URL: nodejs#6750
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
Trott committed May 16, 2016
1 parent 5f31b7e commit 78520fa
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 3 deletions.
8 changes: 5 additions & 3 deletions test/internet/test-dgram-broadcast-multi-process.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ if (common.inFreeBSDJail) {
return;
}

// take the first non-internal interface as the address for binding
// Take the first non-internal interface as the address for binding.
// Ideally, this should check for whether or not an interface is set up for
// BROADCAST and favor internal/private interfaces.
get_bindAddress: for (var name in networkInterfaces) {
var interfaces = networkInterfaces[name];
for (var i = 0; i < interfaces.length; i++) {
Expand Down Expand Up @@ -209,7 +211,7 @@ if (process.argv[2] === 'child') {

receivedMessages.push(buf);

process.send({ message: buf.toString() });
process.send({message: buf.toString()});

if (receivedMessages.length == messages.length) {
process.nextTick(function() {
Expand All @@ -228,7 +230,7 @@ if (process.argv[2] === 'child') {
});

listenSocket.on('listening', function() {
process.send({ listening: true });
process.send({listening: true});
});

listenSocket.bind(common.PORT);
Expand Down
39 changes: 39 additions & 0 deletions test/parallel/test-dgram-setBroadcast.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
'use strict';

const common = require('../common');
const assert = require('assert');
const dgram = require('dgram');

const setup = () => {
return dgram.createSocket({type: 'udp4', reuseAddr: true});
};

const teardown = (socket) => {
if (socket.close)
socket.close();
};

const runTest = (testCode, expectError) => {
const socket = setup();
const assertion = expectError ? assert.throws : assert.doesNotThrow;
const wrapped = () => { testCode(socket); };
assertion(wrapped, expectError);
teardown(socket);
};

// Should throw EBADF if socket is never bound.
runTest((socket) => { socket.setBroadcast(true); }, /EBADF/);

// Should not throw if broadcast set to false after binding.
runTest((socket) => {
socket.bind(common.PORT, common.localhostIPv4, () => {
socket.setBroadcast(false);
});
});

// Should not throw if broadcast set to true after binding.
runTest((socket) => {
socket.bind(common.PORT, common.localhostIPv4, () => {
socket.setBroadcast(true);
});
});

0 comments on commit 78520fa

Please sign in to comment.