Skip to content

Commit

Permalink
[BUGFIX] Check the incremented count for limit
Browse files Browse the repository at this point in the history
The prior implementation was checking `count` without incrementing it,
and the implementation which wanted to log the limit message on the
100th message was logging it on the 101st. Additionally, the check for
presence of the option.id was not correct.
  • Loading branch information
mixonic committed Jun 4, 2021
1 parent 83db9a7 commit 73d1ec0
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 15 deletions.
29 changes: 17 additions & 12 deletions tests/acceptance/workflow-config-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,11 @@ module('workflow config', function (hooks) {
});

test('deprecation limits each id to 100 console.logs', (assert) => {
assert.expect(104);
let limit = 100;

let message = 'log-id';
let id = 'ember.workflow';
let message = 'log-me';
let id = 'first-and-unique-to-limit-test';
let options = {
id,
since: '2.0.0',
Expand All @@ -100,13 +101,14 @@ module('workflow config', function (hooks) {
expected,
'deprecation logs'
);
} else if (count === limit + 1) {
assert.equal(
passedMessage,
'To avoid console overflow, this deprecation will not be logged any more in this run.'
);
} else {
assert.ok(false, 'No further logging expected');
}
if (count === limit) {
window.Testem.handleConsoleMessage = function (passedMessage) {
assert.equal(
passedMessage,
'To avoid console overflow, this deprecation will not be logged any more in this run.'
);
};
}
};

Expand All @@ -115,10 +117,10 @@ module('workflow config', function (hooks) {
deprecate(message, false, options);
}

assert.equal(count, limit + 1, 'logged 101 times, including final notice');
assert.equal(count, limit, 'logged 100 times, including final notice');

let secondMessage = 'log-id2';
let secondId = 'ember.workflow2';
let secondMessage = 'log-me';
let secondId = 'second-and-unique-to-limit-test';
let secondOptions = {
id: secondId,
since: '2.0.0',
Expand All @@ -135,6 +137,9 @@ module('workflow config', function (hooks) {
secondExpected,
'second deprecation logs'
);
window.Testem.handleConsoleMessage = function () {
assert.ok(false, 'No further logging expected');
};
};

deprecate(secondMessage, false, secondOptions);
Expand Down
1 change: 0 additions & 1 deletion tests/dummy/config/deprecation-workflow.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,5 @@ self.deprecationWorkflow.config = {
{ matchMessage: 'silence-me', handler: 'silence' },
{ matchMessage: 'log-me', handler: 'log' },
{ matchMessage: 'throw-me', handler: 'throw' },
{ matchId: 'ember.workflow', handler: 'log' },
],
};
4 changes: 2 additions & 2 deletions vendor/ember-cli-deprecation-workflow/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ const LOG_LIMIT = 100;
// no-op
break;
case 'log': {
let key = options && options.id || message;
let key = (options && options.id) || message;
let count = self.deprecationWorkflow.logCounts[key] || 0;
self.deprecationWorkflow.logCounts[key] = count + 1;
self.deprecationWorkflow.logCounts[key] = ++count;

if (count <= LOG_LIMIT) {
console.warn('DEPRECATION: ' + message);
Expand Down

0 comments on commit 73d1ec0

Please sign in to comment.