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

test: improve test-child-process-fork-dgram #8697

Closed
wants to merge 1 commit into from
Closed

test: improve test-child-process-fork-dgram #8697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 0 additions & 3 deletions test/parallel/parallel.status
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,3 @@ test-fs-watch-encoding : FAIL, PASS

#being worked under https://github.com/nodejs/node/issues/7973
test-stdio-closed : PASS, FLAKY

#covered by https://github.com/nodejs/node/issues/8271
test-child-process-fork-dgram : PASS, FLAKY
32 changes: 25 additions & 7 deletions test/parallel/test-child-process-fork-dgram.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
* Because it's not really possible to predict how the messages will be
* distributed among the parent and the child processes, we keep sending
* messages until both the parent and the child received at least one
* message. The worst case scenario is when either one never receives
* a message. In this case the test runner will timeout after 60 secs
* and the test will fail.
* message. When either the parent or child receives a message we close
* the server on that side so the next message is guaranteed to be
* received by the other.
*/

const common = require('../common');
Expand All @@ -26,22 +26,30 @@ if (common.isWindows) {

var server;
if (process.argv[2] === 'child') {
var serverClosed = false;
process.on('message', function removeMe(msg, clusterServer) {
if (msg === 'server') {
server = clusterServer;

server.on('message', function() {
process.send('gotMessage');
// got a message so close the server to make sure
// the parent also gets a message
serverClosed = true;
server.close();
});

} else if (msg === 'stop') {
server.close();
process.removeListener('message', removeMe);
if (!serverClosed) {
server.close();
}
}
});

} else {
server = dgram.createSocket('udp4');
var serverPort = null;
var client = dgram.createSocket('udp4');
var child = fork(__filename, ['child']);

Expand All @@ -52,9 +60,13 @@ if (process.argv[2] === 'child') {

server.on('message', function(msg, rinfo) {
parentGotMessage = true;
// got a message so close the server to make sure
// the child also gets a message
server.close();
});

server.on('listening', function() {
serverPort = server.address().port;
child.send('server', server);

child.once('message', function(msg) {
Expand All @@ -66,13 +78,16 @@ if (process.argv[2] === 'child') {
sendMessages();
});

var iterations = 0;
var sendMessages = function() {
var timer = setInterval(function() {
iterations++;

client.send(
msg,
0,
msg.length,
server.address().port,
serverPort,
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to add the serverPort variable? It only seems to be used here, and makes things a bit more complicated.

Copy link
Member Author

Choose a reason for hiding this comment

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

Its needed because the server is closed before we have stopped sending. Therefore, you can't call server.address().port. I found this the hard way while building up the changeset.

'127.0.0.1',
function(err) {
if (err) throw err;
Expand All @@ -83,7 +98,8 @@ if (process.argv[2] === 'child') {
* Both the parent and the child got at least one message,
* test passed, clean up everyting.
*/
if (parentGotMessage && childGotMessage) {
if ((parentGotMessage && childGotMessage) ||
((iterations / 1000) > 45)) {
clearInterval(timer);
shutdown();
}
Expand All @@ -94,7 +110,9 @@ if (process.argv[2] === 'child') {
var shutdown = function() {
child.send('stop');

server.close();
if (!parentGotMessage) {
server.close();
}
client.close();
};

Expand Down