lib: remove redundant code, add tests in timers.js

insert() is only called from one place where there is already a check
that msecs is greater than or equal to zero, so do not repeat the check
inside insert().

timers.active() is not documented and should not be exposed, but since
it is exposed for now, let's test it.

PR-URL: https://github.com/nodejs/node/pull/3143
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

diff --git a/lib/timers.js b/lib/timers.js
index aa9f316..5a0a8ab 100644 (file)
--- a/lib/timers.js
+++ b/lib/timers.js
@@ -23,14 +23,17 @@ const TIMEOUT_MAX = 2147483647; // 2^31-1
 // value = list
 var lists = {};
 
+
+// call this whenever the item is active (not idle)
+// it will reset its timeout.
 // the main function - creates lists on demand and the watchers associated
 // with them.
-function insert(item, msecs) {
-  item._idleStart = Timer.now();
-  item._idleTimeout = msecs;
-
+exports.active = function(item) {
+  const msecs = item._idleTimeout;
   if (msecs < 0) return;
 
+  item._idleStart = Timer.now();
+
   var list;
 
   if (lists[msecs]) {
@@ -48,7 +51,7 @@ function insert(item, msecs) {
 
   L.append(list, item);
   assert(!L.isEmpty(list)); // list is not empty
-}
+};
 
 function listOnTimeout() {
   var msecs = this.msecs;
@@ -156,15 +159,6 @@ exports.enroll = function(item, msecs) {
 };
 
 
-// call this whenever the item is active (not idle)
-// it will reset its timeout.
-exports.active = function(item) {
-  var msecs = item._idleTimeout;
-  if (msecs >= 0)
-    insert(item, msecs);
-};
-
-
 /*
  * DOM-style timers
  */

diff --git a/test/parallel/test-timers-active.js b/test/parallel/test-timers-active.js
new file mode 100644 (file)
index 0000000..9162406
--- /dev/null
+++ b/test/parallel/test-timers-active.js
@@ -0,0 +1,33 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const active = require('timers').active;
+
+// active() should create timers for these
+var legitTimers = [
+  { _idleTimeout: 0 },
+  { _idleTimeout: 1 }
+];
+
+legitTimers.forEach(function(legit) {
+  const savedTimeout = legit._idleTimeout;
+  active(legit);
+  // active() should mutate these objects
+  assert(legit._idleTimeout === savedTimeout);
+  assert(Number.isInteger(legit._idleStart));
+  assert(legit._idleNext);
+  assert(legit._idlePrev);
+});
+
+
+// active() should not create a timer for these
+var bogusTimers = [
+  { _idleTimeout: -1 }
+];
+
+bogusTimers.forEach(function(bogus) {
+  const savedTimeout = bogus._idleTimeout;
+  active(bogus);
+  // active() should not mutate these objects
+  assert.deepStrictEqual(bogus, {_idleTimeout: savedTimeout});
+});