Skip to content

Commit

Permalink
Core: Update storage clear check to avoid QUnit.config.stats
Browse files Browse the repository at this point in the history
Reduce internal reliance on the assertion count via the discouraged
QUnit.config.stats and its associated caveats as described at
https://qunitjs.com/api/callbacks/QUnit.done/.

This patch removes the last remaining internal use of `config.stats`.
After this we only ever set it, or expose it (in one place)
to provide the discouraged parameter to QUnit.done() callbacks for
back-compat.

Also remove outdated mention of internal config.stats.bad being used by
the HTML Reporter. This is no longer the case as of today with
#1760 landed for QUnit 3.0.
  • Loading branch information
Krinkle committed Jun 11, 2024
1 parent 170acc0 commit bb5cf73
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 15 deletions.
20 changes: 11 additions & 9 deletions src/core/processing-queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,27 +194,29 @@ class ProcessingQueue {
const runEnd = runSuite.end(true);
emit('runEnd', runEnd);

const runtime = runEnd.runtime;
const passed = config.stats.all - config.stats.bad;

runLoggingCallbacks('done', {
// Use of "details" parameter to QUnit.done() is discouraged
// since QUnit 2.19 to solve the "assertion counting" gotchas
// documented at https://qunitjs.com/api/callbacks/QUnit.done/.
// Use QUnit.on('runEnd') instead.
//
// Kept for compatibility with ecosystem integrations
// such as Testem, which safely only the "runtime" field only.
passed,
// such as Testem, which safely use the "runtime" field only.
passed: config.stats.all - config.stats.bad,
failed: config.stats.bad,
total: config.stats.all,
runtime
runtime: runEnd.runtime
}).then(() => {
// Clear own storage items if all tests passed
if (storage && config.stats.bad === 0) {
// Clear our storage keys if run passed and there were no "todo" tests left.
//
// The check for "todo" is important, because after a passing run,
// it is useful for re-runs to quickly start with known "todo" tests.
// In test.js we inform storage about "todo" failures the same as
// any other failure. Clearing here after merely "passed" would defeat
// that logic.
if (storage && runEnd.status === 'passed' && !runEnd.testCounts.todo) {
for (let i = storage.length - 1; i >= 0; i--) {
const key = storage.key(i);

if (key.indexOf('qunit-test-') === 0) {
storage.removeItem(key);
}
Expand Down
10 changes: 4 additions & 6 deletions src/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -379,12 +379,10 @@ Test.prototype = {
module.stats.all += this.assertions.length;

for (let i = 0; i < this.assertions.length; i++) {
// A failing assertion will counts toward the HTML Reporter's
// "X assertions, Y failed" line even if it was inside a todo.
// Inverting this would be similarly confusing since all but the last
// passing assertion inside a todo test should be considered as good.
// These stats don't decide the outcome of anything, so counting them
// as failing seems the most intuitive.
// For legacy reasons, `config.stats` reflects raw assertion counts.
// This means all failures add to the "bad" count, even an expected
// failure inside a passing "todo" test.
// See also https://qunitjs.com/api/callbacks/QUnit.done/
if (!this.assertions[i].result) {
bad++;
config.stats.bad++;
Expand Down

0 comments on commit bb5cf73

Please sign in to comment.