Skip to content

Commit 1fc373b

Browse files
BridgeARBridgeAR
committed
Revert "repl: refactor tests to not rely on timing"
This reverts commit de848ac. The commit broke multiline repl. PR-URL: nodejs#18715 Refs: nodejs#17828 Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
1 parent 60c9ad7 commit 1fc373b

18 files changed

+273
-391
lines changed

‎lib/internal/repl.js

+2-2
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 = createInternalRepl;
11+
module.exports.createInternalRepl = createRepl;
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 createInternalRepl(env, opts, cb) {
22+
function createRepl(env, opts, cb) {
2323
if (typeof opts === 'function') {
2424
cb = opts;
2525
opts = null;

‎lib/repl.js

+39-40
Original file line numberDiff line numberDiff line change
@@ -109,11 +109,6 @@ 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-
117112
function REPLServer(prompt,
118113
stream,
119114
eval_,
@@ -154,7 +149,6 @@ function REPLServer(prompt,
154149
}
155150

156151
var self = this;
157-
replMap.set(self, self);
158152

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

173167
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+
174203
function defaultEval(code, context, file, cb) {
175204
var err, result, script, wrappedErr;
176205
var wrappedCmd = false;
@@ -302,6 +331,7 @@ function REPLServer(prompt,
302331

303332
if (awaitPromise && !err) {
304333
let sigintListener;
334+
pause();
305335
let promise = result;
306336
if (self.breakEvalOnSigint) {
307337
const interrupt = new Promise((resolve, reject) => {
@@ -320,6 +350,7 @@ function REPLServer(prompt,
320350
prioritizedSigintQueue.delete(sigintListener);
321351

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

363+
unpause();
332364
if (err && process.domain) {
333365
debug('not recoverable, send to domain');
334366
process.domain.emit('error', err);
@@ -345,36 +377,6 @@ function REPLServer(prompt,
345377
}
346378
}
347379

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-
378380
self.eval = self._domain.bind(eval_);
379381

380382
self._domain.on('error', function debugDomainError(e) {
@@ -403,7 +405,6 @@ function REPLServer(prompt,
403405
top.clearBufferedCommand();
404406
top.lines.level = [];
405407
top.displayPrompt();
406-
unpause();
407408
});
408409

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

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

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

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

644643
// Display prompt again
645644
self.displayPrompt();
646-
unpause();
647645
}
648646
});
649647

@@ -726,6 +724,7 @@ exports.start = function(prompt,
726724
ignoreUndefined,
727725
replMode);
728726
if (!exports.repl) exports.repl = repl;
727+
replMap.set(repl, repl);
729728
return repl;
730729
};
731730

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

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

3131
const putIn = new common.ArrayStream();
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-
};
32+
repl.start('', putIn, null, true);
4833

4934
test1();
5035

@@ -63,11 +48,6 @@ function test1() {
6348
}
6449
};
6550
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-
}));
7151
putIn.run(['fs']);
7252
assert(gotWrite);
7353
}
@@ -86,11 +66,6 @@ function test2() {
8666
const val = {};
8767
global.url = val;
8868
assert(!gotWrite);
89-
callbacks.push(common.mustCall((cb, err, result) => {
90-
assert.ifError(err);
91-
assert.strictEqual(result, val);
92-
cb(err, result);
93-
}));
9469
putIn.run(['url']);
9570
assert(gotWrite);
9671
}

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

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

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

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

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

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

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);
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);
6752

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

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);
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);
7762

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

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

+14-4
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@
2121

2222
'use strict';
2323
const common = require('../common');
24+
const assert = require('assert');
2425
const repl = require('repl');
26+
let terminalExit = 0;
27+
let regularExit = 0;
2528

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

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

4751
function testRegularMode() {
@@ -55,11 +59,17 @@ function testRegularMode() {
5559
stream.emit('end');
5660
});
5761

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

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

6474
// start
6575
testTerminalMode();

0 commit comments

Comments
 (0)