From ac53ee5b3e910e6cd35f2395852d6d7859cfd8be Mon Sep 17 00:00:00 2001 From: Dominic Farolino Date: Sun, 17 Jun 2018 22:38:36 -0700 Subject: [PATCH 1/2] console: console.countReset() should emit warning The Console Standard specifies that console.countReset() should emit some type of a warning when given a label that has no previous account associated with it. This PR brings node's implementation of console.countReset() up-to-spec and adds a test asserting that a warning is emitted. Fixes: https://github.com/nodejs/node/issues/20524 --- lib/console.js | 11 +++++++---- test/parallel/test-console.js | 6 ++++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/console.js b/lib/console.js index d42ca496eb15de..65d7e81f151de3 100644 --- a/lib/console.js +++ b/lib/console.js @@ -107,6 +107,7 @@ function Console(options /* or: stdout, stderr, ignoreErrors = true */) { if (typeof colorMode !== 'boolean' && colorMode !== 'auto') throw new ERR_INVALID_ARG_VALUE('colorMode', colorMode); + // Corresponds to https://console.spec.whatwg.org/#count-map this[kCounts] = new Map(); this[kColorMode] = colorMode; @@ -308,12 +309,14 @@ Console.prototype.count = function count(label = 'default') { this.log(`${label}: ${count}`); }; -// Not yet defined by the https://console.spec.whatwg.org, but -// proposed to be added and currently implemented by Edge. Having -// the ability to reset counters is important to help prevent -// the counter from being a memory leak. +// Defined by: https://console.spec.whatwg.org/#countreset Console.prototype.countReset = function countReset(label = 'default') { const counts = this[kCounts]; + if (!counts.has(label)) { + process.emitWarning(`Count for '${label}' does not exist`); + return; + } + counts.delete(`${label}`); }; diff --git a/test/parallel/test-console.js b/test/parallel/test-console.js index e6446bbbe47d17..c103bfdc129841 100644 --- a/test/parallel/test-console.js +++ b/test/parallel/test-console.js @@ -35,14 +35,16 @@ if (common.isMainThread) { common.expectWarning( 'Warning', [ - ['No such label \'nolabel\' for console.timeEnd()', common.noWarnCode], + ['Count for \'noLabel\' does not exist', common.noWarnCode], ['No such label \'nolabel\' for console.timeLog()', common.noWarnCode], + ['No such label \'nolabel\' for console.timeEnd()', common.noWarnCode], ['Label \'test\' already exists for console.time()', common.noWarnCode] ] ); -console.timeEnd('nolabel'); +console.countReset('noLabel'); console.timeLog('nolabel'); +console.timeEnd('nolabel'); console.time('label'); console.timeEnd('label'); From 033f41e7fb40e95becca3c040ca6593d450a3936 Mon Sep 17 00:00:00 2001 From: Dominic Farolino Date: Wed, 4 Jul 2018 15:58:50 -0700 Subject: [PATCH 2/2] Finish rebase + update console label --- test/parallel/test-console.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/parallel/test-console.js b/test/parallel/test-console.js index c103bfdc129841..6f06ea0954001f 100644 --- a/test/parallel/test-console.js +++ b/test/parallel/test-console.js @@ -36,15 +36,15 @@ common.expectWarning( 'Warning', [ ['Count for \'noLabel\' does not exist', common.noWarnCode], - ['No such label \'nolabel\' for console.timeLog()', common.noWarnCode], - ['No such label \'nolabel\' for console.timeEnd()', common.noWarnCode], + ['No such label \'noLabel\' for console.timeLog()', common.noWarnCode], + ['No such label \'noLabel\' for console.timeEnd()', common.noWarnCode], ['Label \'test\' already exists for console.time()', common.noWarnCode] ] ); console.countReset('noLabel'); -console.timeLog('nolabel'); -console.timeEnd('nolabel'); +console.timeLog('noLabel'); +console.timeEnd('noLabel'); console.time('label'); console.timeEnd('label'); @@ -247,6 +247,6 @@ common.hijackStderr(common.mustCall(function(data) { // stderr.write will catch sync error, so use `process.nextTick` here process.nextTick(function() { - assert.strictEqual(data.includes('nolabel'), true); + assert.strictEqual(data.includes('noLabel'), true); }); }));