Skip to content

Commit de848ac

Browse files
bmeckBridgeAR
bmeck
authored and
BridgeAR
committed
repl: refactor tests to not rely on timing
Tests relying on synchronous timing have been migrated to use events. PR-URL: #17828 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
1 parent 6007a9c commit de848ac

18 files changed

+391
-273
lines changed

‎lib/internal/repl.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const os = require('os');
88
const util = require('util');
99
const debug = util.debuglog('repl');
1010
module.exports = Object.create(REPL);
11-
module.exports.createInternalRepl = createRepl;
11+
module.exports.createInternalRepl = createInternalRepl;
1212

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

22-
function createRepl(env, opts, cb) {
22+
function createInternalRepl(env, opts, cb) {
2323
if (typeof opts === 'function') {
2424
cb = opts;
2525
opts = null;

‎lib/repl.js‎

Lines changed: 40 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,11 @@ writer.options = Object.assign({},
109109

110110
exports._builtinLibs = internalModule.builtinLibs;
111111

112+
const sep = '\u0000\u0000\u0000';
113+
const regExMatcher = new RegExp(`^${sep}(.*)${sep}(.*)${sep}(.*)${sep}(.*)` +
114+
`${sep}(.*)${sep}(.*)${sep}(.*)${sep}(.*)` +
115+
`${sep}(.*)$`);
116+
112117
function REPLServer(prompt,
113118
stream,
114119
eval_,
@@ -149,6 +154,7 @@ function REPLServer(prompt,
149154
}
150155

151156
var self = this;
157+
replMap.set(self, self);
152158

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

167173
const savedRegExMatches = ['', '', '', '', '', '', '', '', '', ''];
168-
const sep = '\u0000\u0000\u0000';
169-
const regExMatcher = new RegExp(`^${sep}(.*)${sep}(.*)${sep}(.*)${sep}(.*)` +
170-
`${sep}(.*)${sep}(.*)${sep}(.*)${sep}(.*)` +
171-
`${sep}(.*)$`);
172-
173-
eval_ = eval_ || defaultEval;
174-
175-
// Pause taking in new input, and store the keys in a buffer.
176-
const pausedBuffer = [];
177-
let paused = false;
178-
function pause() {
179-
paused = true;
180-
}
181-
function unpause() {
182-
if (!paused) return;
183-
paused = false;
184-
let entry;
185-
while (entry = pausedBuffer.shift()) {
186-
const [type, payload] = entry;
187-
switch (type) {
188-
case 'key': {
189-
const [d, key] = payload;
190-
self._ttyWrite(d, key);
191-
break;
192-
}
193-
case 'close':
194-
self.emit('exit');
195-
break;
196-
}
197-
if (paused) {
198-
break;
199-
}
200-
}
201-
}
202-
203174
function defaultEval(code, context, file, cb) {
204175
var err, result, script, wrappedErr;
205176
var wrappedCmd = false;
@@ -331,7 +302,6 @@ function REPLServer(prompt,
331302

332303
if (awaitPromise && !err) {
333304
let sigintListener;
334-
pause();
335305
let promise = result;
336306
if (self.breakEvalOnSigint) {
337307
const interrupt = new Promise((resolve, reject) => {
@@ -350,7 +320,6 @@ function REPLServer(prompt,
350320
prioritizedSigintQueue.delete(sigintListener);
351321

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

363-
unpause();
364332
if (err && process.domain) {
365333
debug('not recoverable, send to domain');
366334
process.domain.emit('error', err);
@@ -377,6 +345,36 @@ function REPLServer(prompt,
377345
}
378346
}
379347

348+
eval_ = eval_ || defaultEval;
349+
350+
// Pause taking in new input, and store the keys in a buffer.
351+
const pausedBuffer = [];
352+
let paused = false;
353+
function pause() {
354+
paused = true;
355+
}
356+
function unpause() {
357+
if (!paused) return;
358+
paused = false;
359+
let entry;
360+
while (entry = pausedBuffer.shift()) {
361+
const [type, payload] = entry;
362+
switch (type) {
363+
case 'key': {
364+
const [d, key] = payload;
365+
self._ttyWrite(d, key);
366+
break;
367+
}
368+
case 'close':
369+
self.emit('exit');
370+
break;
371+
}
372+
if (paused) {
373+
break;
374+
}
375+
}
376+
}
377+
380378
self.eval = self._domain.bind(eval_);
381379

382380
self._domain.on('error', function debugDomainError(e) {
@@ -405,6 +403,7 @@ function REPLServer(prompt,
405403
top.clearBufferedCommand();
406404
top.lines.level = [];
407405
top.displayPrompt();
406+
unpause();
408407
});
409408

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

595594
debug('eval %j', evalCmd);
595+
pause();
596596
self.eval(evalCmd, self.context, 'repl', finish);
597597

598598
function finish(e, ret) {
@@ -605,6 +605,7 @@ function REPLServer(prompt,
605605
'(Press Control-D to exit.)\n');
606606
self.clearBufferedCommand();
607607
self.displayPrompt();
608+
unpause();
608609
return;
609610
}
610611

@@ -642,6 +643,7 @@ function REPLServer(prompt,
642643

643644
// Display prompt again
644645
self.displayPrompt();
646+
unpause();
645647
}
646648
});
647649

@@ -724,7 +726,6 @@ exports.start = function(prompt,
724726
ignoreUndefined,
725727
replMode);
726728
if (!exports.repl) exports.repl = repl;
727-
replMap.set(repl, repl);
728729
return repl;
729730
};
730731

‎test/parallel/test-repl-autolibs.js‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,22 @@ const repl = require('repl');
2929
common.globalCheck = false;
3030

3131
const putIn = new common.ArrayStream();
32-
repl.start('', putIn, null, true);
32+
33+
34+
const replserver = repl.start('', putIn, null, true);
35+
const callbacks = [];
36+
const $eval = replserver.eval;
37+
replserver.eval = function(code, context, file, cb) {
38+
const expected = callbacks.shift();
39+
return $eval.call(this, code, context, file, (...args) => {
40+
try {
41+
expected(cb, ...args);
42+
} catch (e) {
43+
console.error(e);
44+
process.exit(1);
45+
}
46+
});
47+
};
3348

3449
test1();
3550

@@ -48,6 +63,11 @@ function test1() {
4863
}
4964
};
5065
assert(!gotWrite);
66+
callbacks.push(common.mustCall((cb, err, result) => {
67+
assert.ifError(err);
68+
assert.strictEqual(result, require('fs'));
69+
cb(err, result);
70+
}));
5171
putIn.run(['fs']);
5272
assert(gotWrite);
5373
}
@@ -66,6 +86,11 @@ function test2() {
6686
const val = {};
6787
global.url = val;
6888
assert(!gotWrite);
89+
callbacks.push(common.mustCall((cb, err, result) => {
90+
assert.ifError(err);
91+
assert.strictEqual(result, val);
92+
cb(err, result);
93+
}));
6994
putIn.run(['url']);
7095
assert(gotWrite);
7196
}

‎test/parallel/test-repl-context.js‎

Lines changed: 42 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -27,40 +27,57 @@ function testContext(repl) {
2727
repl.close();
2828
}
2929

30-
testContextSideEffects(repl.start({ input: stream, output: stream }));
30+
const replserver = repl.start({ input: stream, output: stream });
31+
const callbacks = [];
32+
const $eval = replserver.eval;
33+
replserver.eval = function(code, context, file, cb) {
34+
const expected = callbacks.shift();
35+
return $eval.call(this, code, context, file, (...args) => {
36+
try {
37+
expected(cb, ...args);
38+
} catch (e) {
39+
console.error(e);
40+
process.exit(1);
41+
}
42+
});
43+
};
44+
testContextSideEffects(replserver);
3145

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

3650
// an assignment to '_' in the repl server
37-
server.write('_ = 500;\n');
38-
assert.ok(server.underscoreAssigned);
39-
assert.strictEqual(server.lines.length, 1);
40-
assert.strictEqual(server.lines[0], '_ = 500;');
41-
assert.strictEqual(server.last, 500);
51+
callbacks.push(common.mustCall((cb, ...args) => {
52+
assert.ok(server.underscoreAssigned);
53+
assert.strictEqual(server.last, 500);
54+
cb(...args);
55+
assert.strictEqual(server.lines.length, 1);
56+
assert.strictEqual(server.lines[0], '_ = 500;');
4257

43-
// use the server to create a new context
44-
const context = server.createContext();
58+
// use the server to create a new context
59+
const context = server.createContext();
4560

46-
// ensure that creating a new context does not
47-
// have side effects on the server
48-
assert.ok(server.underscoreAssigned);
49-
assert.strictEqual(server.lines.length, 1);
50-
assert.strictEqual(server.lines[0], '_ = 500;');
51-
assert.strictEqual(server.last, 500);
61+
// ensure that creating a new context does not
62+
// have side effects on the server
63+
assert.ok(server.underscoreAssigned);
64+
assert.strictEqual(server.lines.length, 1);
65+
assert.strictEqual(server.lines[0], '_ = 500;');
66+
assert.strictEqual(server.last, 500);
5267

53-
// reset the server context
54-
server.resetContext();
55-
assert.ok(!server.underscoreAssigned);
56-
assert.strictEqual(server.lines.length, 0);
68+
// reset the server context
69+
server.resetContext();
70+
assert.ok(!server.underscoreAssigned);
71+
assert.strictEqual(server.lines.length, 0);
5772

58-
// ensure that assigning to '_' in the new context
59-
// does not change the value in our server.
60-
assert.ok(!server.underscoreAssigned);
61-
vm.runInContext('_ = 1000;\n', context);
73+
// ensure that assigning to '_' in the new context
74+
// does not change the value in our server.
75+
assert.ok(!server.underscoreAssigned);
76+
vm.runInContext('_ = 1000;\n', context);
6277

63-
assert.ok(!server.underscoreAssigned);
64-
assert.strictEqual(server.lines.length, 0);
65-
server.close();
78+
assert.ok(!server.underscoreAssigned);
79+
assert.strictEqual(server.lines.length, 0);
80+
server.close();
81+
}));
82+
server.write('_ = 500;\n');
6683
}

‎test/parallel/test-repl-end-emits-exit.js‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,7 @@
2121

2222
'use strict';
2323
const common = require('../common');
24-
const assert = require('assert');
2524
const repl = require('repl');
26-
let terminalExit = 0;
27-
let regularExit = 0;
2825

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

44-
r1.on('exit', function() {
41+
r1.on('exit', common.mustCall(function() {
4542
// should be fired from the simulated ^D keypress
46-
terminalExit++;
4743
testRegularMode();
48-
});
44+
}));
4945
}
5046

5147
function testRegularMode() {
@@ -59,17 +55,11 @@ function testRegularMode() {
5955
stream.emit('end');
6056
});
6157

62-
r2.on('exit', function() {
58+
r2.on('exit', common.mustCall(function() {
6359
// should be fired from the simulated 'end' event
64-
regularExit++;
65-
});
60+
}));
6661
}
6762

68-
process.on('exit', function() {
69-
assert.strictEqual(terminalExit, 1);
70-
assert.strictEqual(regularExit, 1);
71-
});
72-
7363

7464
// start
7565
testTerminalMode();

0 commit comments

Comments
 (0)