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

repl: refactor tests to not rely on timing #17828

Closed
wants to merge 1 commit into from
Closed

repl: refactor tests to not rely on timing #17828

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
4 changes: 2 additions & 2 deletions lib/internal/repl.js
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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