diff --git a/src/core/processing-queue.js b/src/core/processing-queue.js index 324df180f..ab7323370 100644 --- a/src/core/processing-queue.js +++ b/src/core/processing-queue.js @@ -194,9 +194,6 @@ 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 @@ -204,17 +201,22 @@ class ProcessingQueue { // 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); } diff --git a/src/test.js b/src/test.js index b03d5f138..31fcf9f5b 100644 --- a/src/test.js +++ b/src/test.js @@ -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++;