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: fix test-debug-signal-cluster.js flakyness #8568

Closed
wants to merge 3 commits into from
Closed
Changes from 1 commit
Commits
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
66 changes: 39 additions & 27 deletions test/parallel/test-debug-signal-cluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const common = require('../common');
const assert = require('assert');
const spawn = require('child_process').spawn;
const os = require('os');
const path = require('path');

const port = common.PORT;
Expand All @@ -11,13 +12,24 @@ const args = [`--debug-port=${port}`, serverPath];
const options = { stdio: ['inherit', 'inherit', 'pipe', 'ipc'] };
const child = spawn(process.execPath, args, options);

const outputLines = [];
var waitingForDebuggers = false;
var expectedContent = [
'Starting debugger agent.',
'Debugger listening on 127.0.0.1:' + (port + 0),
'Starting debugger agent.',
'Debugger listening on 127.0.0.1:' + (port + 1),
'Starting debugger agent.',
'Debugger listening on 127.0.0.1:' + (port + 2),
].join(os.EOL);
expectedContent += os.EOL; // the last line also contains an EOL character

var debuggerAgentsOutput = '';
var debuggerAgentsStarted = false;

var pids;

child.stderr.on('data', function(data) {
const lines = data.toString().replace(/\r/g, '').trim().split('\n');
const childStderrOutputString = data.toString();
const lines = childStderrOutputString.replace(/\r/g, '').trim().split('\n');

lines.forEach(function(line) {
console.log('> ' + line);
Expand All @@ -30,24 +42,26 @@ child.stderr.on('data', function(data) {
pids = msg.pids;
console.error('got pids %j', pids);

waitingForDebuggers = true;
process._debugProcess(child.pid);
debuggerAgentsStarted = true;
});

child.send({
type: 'getpids'
});
} else if (waitingForDebuggers) {
outputLines.push(line);
}

});
if (outputLines.length === expectedLines.length)
onNoMoreLines();

if (debuggerAgentsStarted) {
debuggerAgentsOutput += childStderrOutputString;
if (debuggerAgentsOutput.length === expectedContent.length) {
onNoMoreDebuggerAgentsOutput();
}
}
});

function onNoMoreLines() {
assertOutputLines();
function onNoMoreDebuggerAgentsOutput() {
assertDebuggerAgentsOutput();
process.exit();
}

Expand All @@ -63,21 +77,19 @@ process.on('exit', function onExit() {
});
});

const expectedLines = [
'Starting debugger agent.',
'Debugger listening on 127.0.0.1:' + (port + 0),
'Starting debugger agent.',
'Debugger listening on 127.0.0.1:' + (port + 1),
'Starting debugger agent.',
'Debugger listening on 127.0.0.1:' + (port + 2),
];

function assertOutputLines() {
// Do not assume any particular order of output messages,
// since workers can take different amout of time to
// start up
outputLines.sort();
expectedLines.sort();
function assertDebuggerAgentsOutput() {
// Workers can take different amout of time to start up, and child processes'
// output may be interleaved arbitrarily. Moreover, child processes' output
// may be written using an arbitrary number of system calls, and no assumption
// on buffering or atomicity of output should be made. Thus, we process the
// output of all chid processes' debugger agents character by character, and
Copy link
Member

Choose a reason for hiding this comment

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

nit: spotted a comment typo

Copy link
Author

Choose a reason for hiding this comment

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

Good catch, thanks!

// remove each character from the set of expected characters. Once all the
// output from all debugger agents has been processed, we consider that we got
// the content we expected if there's no character left in the initial
// expected content.
debuggerAgentsOutput.split('').forEach(function gotChar(char) {
expectedContent = expectedContent.replace(char, '');
});

assert.deepStrictEqual(outputLines, expectedLines);
assert.equal(expectedContent, '');
Copy link
Member

@Trott Trott Sep 16, 2016

Choose a reason for hiding this comment

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

Nit: assert.strictEqual() (not necessary, just a preference, hence the Nit, ignore if you have strong contrary feelings)

Copy link
Author

Choose a reason for hiding this comment

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

Good catch, thanks!

}