Skip to content

Commit

Permalink
repl: refactor tests to not rely on timing
Browse files Browse the repository at this point in the history 
Tests relying on synchronous timing have been migrated to use events.

PR-URL: #17828
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
  • Loading branch information
@bmeck @BridgeAR
bmeck authored and BridgeAR committed Feb 10, 2018
1 parent 6007a9c commit de848ac
Show file tree
Hide file tree
Showing 18 changed files with 391 additions and 273 deletions.
4 changes: 2 additions & 2 deletions lib/internal/repl.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const os = require('os');
const util = require('util');
const debug = util.debuglog('repl');
module.exports = Object.create(REPL);
module.exports.createInternalRepl = createRepl;
module.exports.createInternalRepl = createInternalRepl;

// XXX(chrisdickinson): The 15ms debounce value is somewhat arbitrary.
// The debounce is to guard against code pasted into the REPL.
Expand All @@ -19,7 +19,7 @@ function _writeToOutput(repl, message) {
repl._refreshLine();
}

function createRepl(env, opts, cb) {
function createInternalRepl(env, opts, cb) {
if (typeof opts === 'function') {
cb = opts;
opts = null;
Expand Down
79 changes: 40 additions & 39 deletions lib/repl.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ writer.options = Object.assign({},

exports._builtinLibs = internalModule.builtinLibs;

const sep = '\u0000\u0000\u0000';
const regExMatcher = new RegExp(`^${sep}(.*)${sep}(.*)${sep}(.*)${sep}(.*)` +
`${sep}(.*)${sep}(.*)${sep}(.*)${sep}(.*)` +
`${sep}(.*)$`);

function REPLServer(prompt,
stream,
eval_,
Expand Down Expand Up @@ -149,6 +154,7 @@ function REPLServer(prompt,
}

var self = this;
replMap.set(self, self);

self._domain = dom || domain.create();
self.useGlobal = !!useGlobal;
Expand All @@ -165,41 +171,6 @@ function REPLServer(prompt,
self.rli = this;

const savedRegExMatches = ['', '', '', '', '', '', '', '', '', ''];
const sep = '\u0000\u0000\u0000';
const regExMatcher = new RegExp(`^${sep}(.*)${sep}(.*)${sep}(.*)${sep}(.*)` +
`${sep}(.*)${sep}(.*)${sep}(.*)${sep}(.*)` +
`${sep}(.*)$`);

eval_ = eval_ || defaultEval;

// Pause taking in new input, and store the keys in a buffer.
const pausedBuffer = [];
let paused = false;
function pause() {
paused = true;
}
function unpause() {
if (!paused) return;
paused = false;
let entry;
while (entry = pausedBuffer.shift()) {
const [type, payload] = entry;
switch (type) {
case 'key': {
const [d, key] = payload;
self._ttyWrite(d, key);
break;
}
case 'close':
self.emit('exit');
break;
}
if (paused) {
break;
}
}
}

function defaultEval(code, context, file, cb) {
var err, result, script, wrappedErr;
var wrappedCmd = false;
Expand Down Expand Up @@ -331,7 +302,6 @@ function REPLServer(prompt,

if (awaitPromise && !err) {
let sigintListener;
pause();
let promise = result;
if (self.breakEvalOnSigint) {
const interrupt = new Promise((resolve, reject) => {
Expand All @@ -350,7 +320,6 @@ function REPLServer(prompt,
prioritizedSigintQueue.delete(sigintListener);

finishExecution(undefined, result);
unpause();
}, (err) => {
// Remove prioritized SIGINT listener if it was not called.
prioritizedSigintQueue.delete(sigintListener);
Expand All @@ -360,7 +329,6 @@ function REPLServer(prompt,
Object.defineProperty(err, 'stack', { value: '' });
}

unpause();
if (err && process.domain) {
debug('not recoverable, send to domain');
process.domain.emit('error', err);
Expand All @@ -377,6 +345,36 @@ function REPLServer(prompt,
}
}

eval_ = eval_ || defaultEval;

// Pause taking in new input, and store the keys in a buffer.
const pausedBuffer = [];
let paused = false;
function pause() {
paused = true;
}
function unpause() {
if (!paused) return;
paused = false;
let entry;
while (entry = pausedBuffer.shift()) {
const [type, payload] = entry;
switch (type) {
case 'key': {
const [d, key] = payload;
self._ttyWrite(d, key);
break;
}
case 'close':
self.emit('exit');
break;
}
if (paused) {
break;
}
}
}

self.eval = self._domain.bind(eval_);

self._domain.on('error', function debugDomainError(e) {
Expand Down Expand Up @@ -405,6 +403,7 @@ function REPLServer(prompt,
top.clearBufferedCommand();
top.lines.level = [];
top.displayPrompt();
unpause();
});

if (!input && !output) {
Expand Down Expand Up @@ -593,6 +592,7 @@ function REPLServer(prompt,
const evalCmd = self[kBufferedCommandSymbol] + cmd + '\n';

debug('eval %j', evalCmd);
pause();
self.eval(evalCmd, self.context, 'repl', finish);

function finish(e, ret) {
Expand All @@ -605,6 +605,7 @@ function REPLServer(prompt,
'(Press Control-D to exit.)\n');
self.clearBufferedCommand();
self.displayPrompt();
unpause();
return;
}

Expand Down Expand Up @@ -642,6 +643,7 @@ function REPLServer(prompt,

// Display prompt again
self.displayPrompt();
unpause();
}
});

Expand Down Expand Up @@ -724,7 +726,6 @@ exports.start = function(prompt,
ignoreUndefined,
replMode);
if (!exports.repl) exports.repl = repl;
replMap.set(repl, repl);
return repl;
};

Expand Down
27 changes: 26 additions & 1 deletion test/parallel/test-repl-autolibs.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,22 @@ const repl = require('repl');
common.globalCheck = false;

const putIn = new common.ArrayStream();
repl.start('', putIn, null, true);


const replserver = repl.start('', putIn, null, true);
const callbacks = [];
const $eval = replserver.eval;
replserver.eval = function(code, context, file, cb) {
const expected = callbacks.shift();
return $eval.call(this, code, context, file, (...args) => {
try {
expected(cb, ...args);
} catch (e) {
console.error(e);
process.exit(1);
}
});
};

test1();

Expand All @@ -48,6 +63,11 @@ function test1() {
}
};
assert(!gotWrite);
callbacks.push(common.mustCall((cb, err, result) => {
assert.ifError(err);
assert.strictEqual(result, require('fs'));
cb(err, result);
}));
putIn.run(['fs']);
assert(gotWrite);
}
Expand All @@ -66,6 +86,11 @@ function test2() {
const val = {};
global.url = val;
assert(!gotWrite);
callbacks.push(common.mustCall((cb, err, result) => {
assert.ifError(err);
assert.strictEqual(result, val);
cb(err, result);
}));
putIn.run(['url']);
assert(gotWrite);
}
67 changes: 42 additions & 25 deletions test/parallel/test-repl-context.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,40 +27,57 @@ function testContext(repl) {
repl.close();
}

testContextSideEffects(repl.start({ input: stream, output: stream }));
const replserver = repl.start({ input: stream, output: stream });
const callbacks = [];
const $eval = replserver.eval;
replserver.eval = function(code, context, file, cb) {
const expected = callbacks.shift();
return $eval.call(this, code, context, file, (...args) => {
try {
expected(cb, ...args);
} catch (e) {
console.error(e);
process.exit(1);
}
});
};
testContextSideEffects(replserver);

function testContextSideEffects(server) {
assert.ok(!server.underscoreAssigned);
assert.strictEqual(server.lines.length, 0);

// an assignment to '_' in the repl server
server.write('_ = 500;\n');
assert.ok(server.underscoreAssigned);
assert.strictEqual(server.lines.length, 1);
assert.strictEqual(server.lines[0], '_ = 500;');
assert.strictEqual(server.last, 500);
callbacks.push(common.mustCall((cb, ...args) => {
assert.ok(server.underscoreAssigned);
assert.strictEqual(server.last, 500);
cb(...args);
assert.strictEqual(server.lines.length, 1);
assert.strictEqual(server.lines[0], '_ = 500;');

// use the server to create a new context
const context = server.createContext();
// use the server to create a new context
const context = server.createContext();

// ensure that creating a new context does not
// have side effects on the server
assert.ok(server.underscoreAssigned);
assert.strictEqual(server.lines.length, 1);
assert.strictEqual(server.lines[0], '_ = 500;');
assert.strictEqual(server.last, 500);
// ensure that creating a new context does not
// have side effects on the server
assert.ok(server.underscoreAssigned);
assert.strictEqual(server.lines.length, 1);
assert.strictEqual(server.lines[0], '_ = 500;');
assert.strictEqual(server.last, 500);

// reset the server context
server.resetContext();
assert.ok(!server.underscoreAssigned);
assert.strictEqual(server.lines.length, 0);
// reset the server context
server.resetContext();
assert.ok(!server.underscoreAssigned);
assert.strictEqual(server.lines.length, 0);

// ensure that assigning to '_' in the new context
// does not change the value in our server.
assert.ok(!server.underscoreAssigned);
vm.runInContext('_ = 1000;\n', context);
// ensure that assigning to '_' in the new context
// does not change the value in our server.
assert.ok(!server.underscoreAssigned);
vm.runInContext('_ = 1000;\n', context);

assert.ok(!server.underscoreAssigned);
assert.strictEqual(server.lines.length, 0);
server.close();
assert.ok(!server.underscoreAssigned);
assert.strictEqual(server.lines.length, 0);
server.close();
}));
server.write('_ = 500;\n');
}
18 changes: 4 additions & 14 deletions test/parallel/test-repl-end-emits-exit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@

'use strict';
const common = require('../common');
const assert = require('assert');
const repl = require('repl');
let terminalExit = 0;
let regularExit = 0;

// Create a dummy stream that does nothing
const stream = new common.ArrayStream();
Expand All @@ -41,11 +38,10 @@ function testTerminalMode() {
stream.emit('data', '\u0004');
});

r1.on('exit', function() {
r1.on('exit', common.mustCall(function() {
// should be fired from the simulated ^D keypress
terminalExit++;
testRegularMode();
});
}));
}

function testRegularMode() {
Expand All @@ -59,17 +55,11 @@ function testRegularMode() {
stream.emit('end');
});

r2.on('exit', function() {
r2.on('exit', common.mustCall(function() {
// should be fired from the simulated 'end' event
regularExit++;
});
}));
}

process.on('exit', function() {
assert.strictEqual(terminalExit, 1);
assert.strictEqual(regularExit, 1);
});


// start
testTerminalMode();
Loading

0 comments on commit de848ac

Please sign in to comment.