Skip to content

Commit

Permalink
readline: use Date.now() and move test to parallel
Browse files Browse the repository at this point in the history
The readline module wants a truthy time while using Timer.now() doesn't
necessarily guarantee that early on in the process' life. It also
doesn't actually resolve the timing issues experienced in an earlier
issue. Instead, this PR fixes the related tests and moves them back
to parallel.

Refs: nodejs#14674
  • Loading branch information
apapirovski committed Feb 4, 2018
1 parent 47a984a commit eda075e
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 114 deletions.
13 changes: 7 additions & 6 deletions lib/readline.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ const {
kClearScreenDown
} = CSI;

const { now } = process.binding('timer_wrap').Timer;

const kHistorySize = 30;
const kMincrlfDelay = 100;
// \r\n, \n, or \r followed by something other than \n
Expand Down Expand Up @@ -409,9 +407,10 @@ Interface.prototype._normalWrite = function(b) {
if (b === undefined) {
return;
}
const now = Date.now();
var string = this._decoder.write(b);
if (this._sawReturnAt &&
now() - this._sawReturnAt <= this.crlfDelay) {
now - this._sawReturnAt <= this.crlfDelay) {
string = string.replace(/^\n/, '');
this._sawReturnAt = 0;
}
Expand All @@ -424,7 +423,7 @@ Interface.prototype._normalWrite = function(b) {
this._line_buffer = null;
}
if (newPartContainsEnding) {
this._sawReturnAt = string.endsWith('\r') ? now() : 0;
this._sawReturnAt = string.endsWith('\r') ? now : 0;

// got one or more newlines; process into "line" events
var lines = string.split(lineEnding);
Expand Down Expand Up @@ -911,20 +910,22 @@ Interface.prototype._ttyWrite = function(s, key) {
} else {
/* No modifier keys used */

const now = Date.now();

// \r bookkeeping is only relevant if a \n comes right after.
if (this._sawReturnAt && key.name !== 'enter')
this._sawReturnAt = 0;

switch (key.name) {
case 'return': // carriage return, i.e. \r
this._sawReturnAt = now();
this._sawReturnAt = now;
this._line();
break;

case 'enter':
// When key interval > crlfDelay
if (this._sawReturnAt === 0 ||
now() - this._sawReturnAt > this.crlfDelay) {
now - this._sawReturnAt > this.crlfDelay) {
this._line();
}
this._sawReturnAt = 0;
Expand Down
73 changes: 73 additions & 0 deletions test/parallel/test-readline-interface.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ class FakeInput extends EventEmitter {
end() {}
}

const crlfDelay = common.platformTimeout(1000);

function isWarned(emitter) {
for (const name in emitter) {
const listeners = emitter[name];
Expand Down Expand Up @@ -869,3 +871,74 @@ function isWarned(emitter) {
assert.strictEqual(rl._prompt, '$ ');
}
});

[ true, false ].forEach(function(terminal) {
// sending multiple newlines at once that does not end with a new line
// and a `end` event(last line is)

// \r\n should emit one line event, not two
{
const fi = new FakeInput();
const rli = new readline.Interface(
{
input: fi,
output: fi,
terminal: terminal,
crlfDelay
}
);
const expectedLines = ['foo', 'bar', 'baz', 'bat'];
let callCount = 0;
rli.on('line', function(line) {
assert.strictEqual(line, expectedLines[callCount]);
callCount++;
});
fi.emit('data', expectedLines.join('\r\n'));
assert.strictEqual(callCount, expectedLines.length - 1);
rli.close();
}

// \r\n should emit one line event when split across multiple writes.
{
const fi = new FakeInput();
const rli = new readline.Interface({
input: fi,
output: fi,
terminal: terminal,
crlfDelay
});
const expectedLines = ['foo', 'bar', 'baz', 'bat'];
let callCount = 0;
rli.on('line', function(line) {
assert.strictEqual(line, expectedLines[callCount]);
callCount++;
});
expectedLines.forEach(function(line) {
fi.emit('data', `${line}\r`);
fi.emit('data', '\n');
});
assert.strictEqual(callCount, expectedLines.length);
rli.close();
}

// Emit one line event when the delay between \r and \n is
// over the default crlfDelay but within the setting value.
{
const fi = new FakeInput();
const delay = 125;
const rli = new readline.Interface({
input: fi,
output: fi,
terminal: terminal,
crlfDelay
});
let callCount = 0;
rli.on('line', () => callCount++);
fi.emit('data', '\r');
setTimeout(common.mustCall(() => {
fi.emit('data', '\n');
assert.strictEqual(callCount, 1);
rli.close();
}), delay);
}
});
108 changes: 0 additions & 108 deletions test/sequential/test-readline-interface.js

This file was deleted.

0 comments on commit eda075e

Please sign in to comment.