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.

Sign up for GitHub

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

Jump to bottom

test: refactor test-dgram-broadcast-multi-process #26846

Closed
wants to merge 2 commits into from
Closed

test: refactor test-dgram-broadcast-multi-process #26846

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1e61515
test: refactor test-dgram-broadcast-multi-process
Trott Mar 21, 2019
59d7171
fixup! test: refactor test-dgram-broadcast-multi-process
Trott Mar 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 18 additions & 22 deletions test/internet/test-dgram-broadcast-multi-process.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@ const assert = require('assert');
const dgram = require('dgram');
const util = require('util');
const networkInterfaces = require('os').networkInterfaces();
const fork = require('child_process').fork;
const { fork } = require('child_process');
const LOCAL_BROADCAST_HOST = '255.255.255.255';
const TIMEOUT = common.platformTimeout(5000);
const messages = [
Buffer.from('First message to send'),
Buffer.from('Second message to send'),
Buffer.from('Third message to send'),
Buffer.from('Fourth message to send')
Buffer.from('Fourth message to send'),
Copy link
Member

Choose a reason for hiding this comment

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

why add a comma

Copy link
Member Author

Choose a reason for hiding this comment

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

From https://eslint.org/docs/rules/comma-dangle:

Trailing commas simplify adding and removing items to objects and arrays, since only the lines you are modifying must be touched. Another argument in favor of trailing commas is that it improves the clarity of diffs when an item is added or removed from an object or array:

Screen Shot 2019-03-21 at 8 30 39 PM

Copy link
Member

Choose a reason for hiding this comment

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

tks

];

let bindAddress = null;
Expand Down Expand Up @@ -65,7 +65,7 @@ if (process.argv[2] !== 'child') {
let timer = null;

// Exit the test if it doesn't succeed within TIMEOUT
timer = setTimeout(function() {
timer = setTimeout(() => {
console.error('[PARENT] Responses were not received within %d ms.',
TIMEOUT);
console.error('[PARENT] Fail');
Expand All @@ -84,7 +84,7 @@ if (process.argv[2] !== 'child') {
worker.messagesReceived = [];

// Handle the death of workers
worker.on('exit', function(code, signal) {
worker.on('exit', (code, signal) => {
// Don't consider this the true death if the worker
// has finished successfully
// or if the exit code is 0
Expand All @@ -98,6 +98,8 @@ if (process.argv[2] !== 'child') {
dead,
listeners);

assert.notStrictEqual(signal, null);

if (dead === listeners) {
console.error('[PARENT] All workers have died.');
console.error('[PARENT] Fail');
Expand All @@ -108,7 +110,7 @@ if (process.argv[2] !== 'child') {
}
});

worker.on('message', function(msg) {
worker.on('message', (msg) => {
if (msg.listening) {
listening += 1;

Expand All @@ -132,12 +134,12 @@ if (process.argv[2] !== 'child') {
'required number of ' +
'messages. Will now compare.');

Object.keys(workers).forEach(function(pid) {
Object.keys(workers).forEach((pid) => {
const worker = workers[pid];

let count = 0;

worker.messagesReceived.forEach(function(buf) {
worker.messagesReceived.forEach((buf) => {
for (let i = 0; i < messages.length; ++i) {
if (buf.toString() === messages[i].toString()) {
count++;
Expand Down Expand Up @@ -170,11 +172,11 @@ if (process.argv[2] !== 'child') {
// Bind the address explicitly for sending
// INADDR_BROADCAST to only one interface
sendSocket.bind(common.PORT, bindAddress);
sendSocket.on('listening', function() {
sendSocket.on('listening', () => {
sendSocket.setBroadcast(true);
});

sendSocket.on('close', function() {
sendSocket.on('close', () => {
console.error('[PARENT] sendSocket closed');
});

Expand All @@ -192,7 +194,7 @@ if (process.argv[2] !== 'child') {
buf.length,
common.PORT,
LOCAL_BROADCAST_HOST,
function(err) {
(err) => {
assert.ifError(err);
console.error('[PARENT] sent %s to %s:%s',
util.inspect(buf.toString()),
Expand All @@ -204,7 +206,7 @@ if (process.argv[2] !== 'child') {
};

function killSubprocesses(subprocesses) {
Object.keys(subprocesses).forEach(function(key) {
Object.keys(subprocesses).forEach((key) => {
const subprocess = subprocesses[key];
subprocess.kill();
});
Expand All @@ -218,7 +220,7 @@ if (process.argv[2] === 'child') {
reuseAddr: true
});

listenSocket.on('message', function(buf, rinfo) {
listenSocket.on('message', (buf, rinfo) => {
// Receive udp messages only sent from parent
if (rinfo.address !== bindAddress) return;

Expand All @@ -232,24 +234,18 @@ if (process.argv[2] === 'child') {
process.send({ message: buf.toString() });

if (receivedMessages.length === messages.length) {
process.nextTick(function() {
listenSocket.close();
});
process.nextTick(() => { listenSocket.close(); });
}
});

listenSocket.on('close', function() {
listenSocket.on('close', () => {
// HACK: Wait to exit the process to ensure that the parent
// process has had time to receive all messages via process.send()
// This may be indicative of some other issue.
setTimeout(function() {
process.exit();
}, 1000);
setTimeout(() => { process.exit(); }, 1000);
});

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

listenSocket.bind(common.PORT);
}