Skip to content

Commit

Permalink
Revert "repl: refactor tests to not rely on timing"
Browse files Browse the repository at this point in the history 
This reverts commit de848ac.

The commit broke multiline repl.

PR-URL: #18715
Refs: #17828
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
  • Loading branch information
@BridgeAR
BridgeAR committed Feb 12, 2018
1 parent 60c9ad7 commit 1fc373b
Show file tree
Hide file tree
Showing 18 changed files with 273 additions and 391 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 = createInternalRepl;
module.exports.createInternalRepl = createRepl;

// 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 createInternalRepl(env, opts, cb) {
function createRepl(env, opts, cb) {
if (typeof opts === 'function') {
cb = opts;
opts = null;
Expand Down
79 changes: 39 additions & 40 deletions lib/repl.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,6 @@ 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 @@ -154,7 +149,6 @@ function REPLServer(prompt,
}

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

self._domain = dom || domain.create();
self.useGlobal = !!useGlobal;
Expand All @@ -171,6 +165,41 @@ 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 @@ -302,6 +331,7 @@ function REPLServer(prompt,

if (awaitPromise && !err) {
let sigintListener;
pause();
let promise = result;
if (self.breakEvalOnSigint) {
const interrupt = new Promise((resolve, reject) => {
Expand All @@ -320,6 +350,7 @@ 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 @@ -329,6 +360,7 @@ 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 @@ -345,36 +377,6 @@ 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 @@ -403,7 +405,6 @@ function REPLServer(prompt,
top.clearBufferedCommand();
top.lines.level = [];
top.displayPrompt();
unpause();
});

if (!input && !output) {
Expand Down Expand Up @@ -592,7 +593,6 @@ 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,7 +605,6 @@ function REPLServer(prompt,
'(Press Control-D to exit.)\n');
self.clearBufferedCommand();
self.displayPrompt();
unpause();
return;
}

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

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

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

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

const putIn = new common.ArrayStream();


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);
}
});
};
repl.start('', putIn, null, true);

test1();

Expand All @@ -63,11 +48,6 @@ 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 @@ -86,11 +66,6 @@ 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: 25 additions & 42 deletions test/parallel/test-repl-context.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,57 +27,40 @@ function testContext(repl) {
repl.close();
}

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);
testContextSideEffects(repl.start({ input: stream, output: stream }));

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

// an assignment to '_' in the repl server
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;');
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);

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

'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 @@ -38,10 +41,11 @@ function testTerminalMode() {
stream.emit('data', '\u0004');
});

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

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

r2.on('exit', common.mustCall(function() {
r2.on('exit', 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 1fc373b

Please sign in to comment.