Skip to content

Commit b7a95a9

Browse files
committed
CLI: Fix TAP compliance for test plan when using QUnit.test.only()
We correctly prevent registration, counting, and execution of non-only tests after the first QUnit.test.only call. And we correctly undo the execution and registration of any QUnit.test calls before the first QUnit.test.only call. But, we did not clear the test *count*. This left the test count emitted in `runEnd` event, and thus in the "1..X" test plan printed by the TAP reporter, inaccurate. This has implications for TAP-based test runners (such as QTap) as those rely on this being accurate to declare a run as finished, stop waiting for results, and stop the browser process.
1 parent 9efcfe1 commit b7a95a9

File tree

7 files changed

+49
-17
lines changed

7 files changed

+49
-17
lines changed

src/core/module.js

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,41 @@ function processModule (name, options, scope, modifiers = {}) {
153153
}
154154
}
155155

156+
/**
157+
* Clear the SuiteReport tree of all tests and leave only current module as child suite
158+
*
159+
* This should be called before defining the first module.only() or test.only()
160+
* because otherwise:
161+
* - `runEnd.testCounts` is too high.
162+
* - UI (HtmlReporter) and TAP (TapReporter) display totals too high.
163+
* - Test runners like QTap might timeout because the TAP plan
164+
* would be printed as "1..9" even if only 2 tests are run,
165+
* which means tap-finished will wait for 3-9.
166+
*/
167+
export function clearSuiteReports (currentModule) {
168+
let childSuite = null;
169+
let suiteReport = currentModule.suiteReport;
170+
while (suiteReport) {
171+
suiteReport.tests.length = 0;
172+
const i = suiteReport.childSuites.indexOf(childSuite);
173+
if (i === -1) {
174+
suiteReport.childSuites.length = 0;
175+
} else {
176+
// Reduce in-place to just currentModule.suiteReport or its intermediary
177+
suiteReport.childSuites.splice(0, i);
178+
suiteReport.childSuites.splice(1);
179+
}
180+
181+
if (suiteReport === globalSuiteReport) {
182+
suiteReport = null;
183+
} else {
184+
childSuite = suiteReport;
185+
currentModule = currentModule.parentModule;
186+
suiteReport = (currentModule && currentModule.suiteReport) || globalSuiteReport;
187+
}
188+
}
189+
}
190+
156191
let focused = false; // indicates that the "only" filter was used
157192

158193
export function module (name, options, scope) {
@@ -165,15 +200,9 @@ module.only = function (...args) {
165200
if (!focused) {
166201
// Upon the first module.only() call,
167202
// delete any and all previously registered modules and tests.
168-
//
169-
// TODO: This does not clear SuiteReport, which means the empty modules are
170-
// left behind and wrongly reported as "skipped" (skipped == total), and
171-
// deleted tests wrongly count toward runEnd.testCounts as passing test
172-
// (this.getFailedAssertions().length === 0).
173-
// This is why /test/cli/fixtures/only-test-only-module-mix.tap.txt reports
174-
// 1..3 instead of 1..1.
175203
config.modules.length = 0;
176204
config.queue.length = 0;
205+
clearSuiteReports(config.currentModule);
177206

178207
// Ignore any tests declared after this block within the same
179208
// module parent. https://github.com/qunitjs/qunit/issues/1645

src/core/test.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import dump from './dump.js';
55
import { emit } from './events.js';
66
import { globalThis, setTimeout, clearTimeout, StringMap } from './globals.js';
77
import Logger from './logger.js';
8+
import { clearSuiteReports } from './module.js';
89
import Promise from './promise.js';
910
import TestReport from './reports/test.js';
1011
import { extractStacktrace, sourceFromStacktrace } from './stacktrace.js';
@@ -912,6 +913,8 @@ function addOnlyTest (settings) {
912913
}
913914
if (!focused) {
914915
config.queue.length = 0;
916+
clearSuiteReports(config.currentModule);
917+
915918
focused = true;
916919
}
917920

test/cli/fixtures/only-module-flat.tap.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ not ok 1 module B > test B # TODO
1212
...
1313
ok 2 module B > test C # SKIP
1414
ok 3 module B > test D
15-
1..4
16-
# pass 2
15+
1..3
16+
# pass 1
1717
# skip 1
1818
# todo 1
1919
# fail 0

test/cli/fixtures/only-module-then-test.tap.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33

44
TAP version 13
55
ok 1 module A > module B > test B
6-
1..2
7-
# pass 2
6+
1..1
7+
# pass 1
88
# skip 0
99
# todo 0
1010
# fail 0

test/cli/fixtures/only-module.tap.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ ok 3 module B > Only this module should run > normal test
1616
ok 4 module D > test D
1717
ok 5 module E > module F > test F
1818
ok 6 module E > test E
19-
1..8
20-
# pass 6
19+
1..6
20+
# pass 4
2121
# skip 1
2222
# todo 1
2323
# fail 0

test/cli/fixtures/only-test-only-module-mix.tap.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ not ok 1 global failure
99
Error: No tests were run.
1010
...
1111
Bail out! Error: No tests were run.
12-
1..3
13-
# pass 2
12+
1..1
13+
# pass 0
1414
# skip 0
1515
# todo 0
1616
# fail 1

test/cli/fixtures/only-test.tap.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
TAP version 13
55
ok 1 run this test
66
ok 2 all tests with only run
7-
1..3
8-
# pass 3
7+
1..2
8+
# pass 2
99
# skip 0
1010
# todo 0
1111
# fail 0

0 commit comments

Comments
 (0)