From 58199b1f7a5a6cc5205810c1560422dae73986cd Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 30 Sep 2015 22:18:36 -0700 Subject: [PATCH 1/3] 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. --- lib/timers.js | 22 ++++++++-------------- test/parallel/test-timers-active.js | 28 ++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 14 deletions(-) create mode 100644 test/parallel/test-timers-active.js diff --git a/lib/timers.js b/lib/timers.js index aa9f316253e692..5a0a8ab271e549 100644 --- 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 index 00000000000000..8366a522116f38 --- /dev/null +++ b/test/parallel/test-timers-active.js @@ -0,0 +1,28 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const active = require('timers').active; + +// active() should not create a timer for these +var legitTimers = [ + { _idleTimeout: 0 }, + { _idleTimeout: 1 } +]; + +legitTimers.forEach(function(legit) { + active(legit); + // active() should mutate these objects + assert.notDeepEqual(Object.keys(legit), ['_idleTimeout']); +}); + + +// active() should not create a timer for these +var bogusTimers = [ + { _idleTimeout: -1 } +]; + +bogusTimers.forEach(function(bogus) { + active(bogus); + // active() should not mutate these objects + assert.deepEqual(Object.keys(bogus), ['_idleTimeout']); +}); From 42712930c71a3f70feeb26d28336f6a49e0e8530 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 5 Oct 2015 21:13:22 -0700 Subject: [PATCH 2/3] fixup: improve test --- test/parallel/test-timers-active.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-timers-active.js b/test/parallel/test-timers-active.js index 8366a522116f38..cf8f904de96089 100644 --- a/test/parallel/test-timers-active.js +++ b/test/parallel/test-timers-active.js @@ -10,9 +10,13 @@ var legitTimers = [ ]; legitTimers.forEach(function(legit) { + const savedTimeout = legit._idleTimeout; active(legit); // active() should mutate these objects - assert.notDeepEqual(Object.keys(legit), ['_idleTimeout']); + assert(legit._idleTimeout === savedTimeout); + assert(Number.isInteger(legit._idleStart)); + assert(legit._idleNext); + assert(legit._idlePrev); }); @@ -22,7 +26,8 @@ var bogusTimers = [ ]; bogusTimers.forEach(function(bogus) { + const savedTimeout = bogus._idleTimeout; active(bogus); // active() should not mutate these objects - assert.deepEqual(Object.keys(bogus), ['_idleTimeout']); + assert.deepStrictEqual(bogus, {_idleTimeout: savedTimeout}); }); From 17e09ecc8057e92fdbb6ae9360e5df8e04cd78b0 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 5 Oct 2015 21:22:39 -0700 Subject: [PATCH 3/3] typo fix --- test/parallel/test-timers-active.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-timers-active.js b/test/parallel/test-timers-active.js index cf8f904de96089..9162406848d195 100644 --- a/test/parallel/test-timers-active.js +++ b/test/parallel/test-timers-active.js @@ -3,7 +3,7 @@ const common = require('../common'); const assert = require('assert'); const active = require('timers').active; -// active() should not create a timer for these +// active() should create timers for these var legitTimers = [ { _idleTimeout: 0 }, { _idleTimeout: 1 }