readline: use Date.now() and move test to parallel

apapirovski · MylesBorins · commit 3f73b7ff1c88 · 2018-02-20T19:27:05.000-05:00
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: #14674 PR-URL: #18563 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
diff --git a/lib/readline.js b/lib/readline.js
@@ -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
@@ -411,7 +409,7 @@ Interface.prototype._normalWrite = function(b) {
   }
   var string = this._decoder.write(b);
   if (this._sawReturnAt &&
-      now() - this._sawReturnAt <= this.crlfDelay) {
+      Date.now() - this._sawReturnAt <= this.crlfDelay) {
     string = string.replace(/^\n/, '');
     this._sawReturnAt = 0;
   }
@@ -424,7 +422,7 @@ Interface.prototype._normalWrite = function(b) {
     this._line_buffer = null;
   }
   if (newPartContainsEnding) {
-    this._sawReturnAt = string.endsWith('\r') ? now() : 0;
+    this._sawReturnAt = string.endsWith('\r') ? Date.now() : 0;
 
     // got one or more newlines; process into "line" events
     var lines = string.split(lineEnding);
@@ -917,14 +915,14 @@ Interface.prototype._ttyWrite = function(s, key) {
 
     switch (key.name) {
       case 'return':  // carriage return, i.e. \r
-        this._sawReturnAt = now();
+        this._sawReturnAt = Date.now();
         this._line();
         break;
 
       case 'enter':
         // When key interval > crlfDelay
         if (this._sawReturnAt === 0 ||
-            now() - this._sawReturnAt > this.crlfDelay) {
+            Date.now() - this._sawReturnAt > this.crlfDelay) {
           this._line();
         }
         this._sawReturnAt = 0;
diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js
@@ -869,3 +869,80 @@ function isWarned(emitter) {
     assert.strictEqual(rl._prompt, '$ ');
   }
 });
+
+// For the purposes of the following tests, we do not care about the exact
+// value of crlfDelay, only that the behaviour conforms to what's expected.
+// Setting it to Infinity allows the test to succeed even under extreme
+// CPU stress.
+const crlfDelay = Infinity;
+
+[ 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);
+  }
+});
diff --git a/test/sequential/test-readline-interface.js b/test/sequential/test-readline-interface.js
