Skip to content

Commit

Permalink
console: console.countReset() should emit warning
Browse files Browse the repository at this point in the history
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: #20524

PR-URL: #21649
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
domfarolino authored and jasnell committed Jul 12, 2018
1 parent 85b0f16 commit d4164ca
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 9 deletions.
11 changes: 7 additions & 4 deletions lib/console.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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}`);
};

Expand Down
12 changes: 7 additions & 5 deletions test/parallel/test-console.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,16 @@ if (common.isMainThread) {
common.expectWarning(
'Warning',
[
['No such label \'nolabel\' for console.timeEnd()', common.noWarnCode],
['No such label \'nolabel\' for console.timeLog()', 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.timeLog('nolabel');
console.countReset('noLabel');
console.timeLog('noLabel');
console.timeEnd('noLabel');

console.time('label');
console.timeEnd('label');
Expand Down Expand Up @@ -245,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);
});
}));

0 comments on commit d4164ca

Please sign in to comment.