Skip to content

Commit 0c7ee91

Browse files
authored
Mark XCTest suites as passed/failed immediately on suite completion (#1031)
Add back some code that was removed that marked XCTest suites as passed/failed as soon as all their tests completed. Currently the TestExplorer shows the test suites as pending until all tests in the whole test run have completed. This is a regression from how the Test Explorer used to work, where suites were marked as passed/failed immediately upon suite completion. Issue: #1029
1 parent 6fb2ee8 commit 0c7ee91

File tree

5 files changed

+105
-14
lines changed

5 files changed

+105
-14
lines changed

src/TestExplorer/TestRunner.ts

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1193,12 +1193,40 @@ export class TestRunnerTestRunState implements ITestRunState {
11931193
// Nothing to do here
11941194
}
11951195
// passed suite
1196-
passedSuite() {
1197-
// Nothing to do here
1196+
passedSuite(name: string) {
1197+
// Regular runs don't provide the full suite name (Target.Suite)
1198+
// in the output, so reference the last passing/failing test item
1199+
// and derive the suite from that.
1200+
1201+
// However, when running a parallel test run the XUnit XML output
1202+
// provides the full suite name, and the `lastTestItem` set is not
1203+
// guarenteed to be in this suite due to the parallel nature of the run.
1204+
1205+
// If we can look the suite up by name then we're doing a parallel run
1206+
// and can mark it as passed, otherwise derive the suite from the last
1207+
// completed test item.
1208+
const suiteIndex = this.testRun.getTestIndex(name);
1209+
if (suiteIndex !== -1) {
1210+
this.testRun.passed(this.testRun.testItems[suiteIndex]);
1211+
} else {
1212+
const lastClassTestItem = this.lastTestItem?.parent;
1213+
if (lastClassTestItem && lastClassTestItem.id.endsWith(`.${name}`)) {
1214+
this.testRun.passed(lastClassTestItem);
1215+
}
1216+
}
11981217
}
11991218
// failed suite
1200-
failedSuite() {
1201-
// Nothing to do here
1219+
failedSuite(name: string) {
1220+
// See comment in `passedSuite` for more context.
1221+
const suiteIndex = this.testRun.getTestIndex(name);
1222+
if (suiteIndex !== -1) {
1223+
this.testRun.failed(this.testRun.testItems[suiteIndex], []);
1224+
} else {
1225+
const lastClassTestItem = this.lastTestItem?.parent;
1226+
if (lastClassTestItem && lastClassTestItem.id.endsWith(`.${name}`)) {
1227+
this.testRun.failed(lastClassTestItem, []);
1228+
}
1229+
}
12021230
}
12031231

12041232
recordOutput(index: number | undefined, output: string): void {

src/TestExplorer/TestXUnitParser.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,15 @@ export class TestXUnitParser {
6666
let failures = 0;
6767
let errors = 0;
6868
xUnit.testsuites.testsuite.forEach(testsuite => {
69+
const suiteFailures = parseInt(testsuite.$.failures);
70+
failures += suiteFailures;
6971
tests = tests + parseInt(testsuite.$.tests);
70-
failures += parseInt(testsuite.$.failures);
7172
errors += parseInt(testsuite.$.errors);
73+
74+
let className: string | undefined;
7275
testsuite.testcase.forEach(testcase => {
73-
const id = `${testcase.$.classname}/${testcase.$.name}`;
76+
className = testcase.$.classname;
77+
const id = `${className}/${testcase.$.name}`;
7478
const index = runState.getTestItemIndex(id);
7579

7680
// From 5.7 to 5.10 running with the --parallel option dumps the test results out
@@ -85,6 +89,14 @@ export class TestXUnitParser {
8589
}
8690
runState.completed(index, { duration: testcase.$.time });
8791
});
92+
93+
if (className !== undefined) {
94+
if (className && suiteFailures === 0) {
95+
runState.passedSuite(className);
96+
} else if (className) {
97+
runState.failedSuite(className);
98+
}
99+
}
88100
});
89101
return { tests: tests, failures: failures, errors: errors };
90102
}

test/suite/testexplorer/MockTestRunState.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,11 +130,13 @@ export class TestRunState implements ITestRunState {
130130
//
131131
}
132132
// passed suite
133-
passedSuite() {
134-
//
133+
passedSuite(name: string) {
134+
const index = this.testItemFinder.getIndex(name);
135+
this.testItemFinder.tests[index].status = TestStatus.passed;
135136
}
136137
// failed suite
137-
failedSuite() {
138-
//
138+
failedSuite(name: string) {
139+
const index = this.testItemFinder.getIndex(name);
140+
this.testItemFinder.tests[index].status = TestStatus.failed;
139141
}
140142
}

test/suite/testexplorer/TestExplorerIntegration.test.ts

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,10 @@ suite("Test Explorer Suite", function () {
278278
await eventPromise(testRun.onTestRunComplete);
279279

280280
assertTestResults(testRun, {
281-
passed: ["PackageTests.MixedXCTestSuite/testPassing"],
281+
passed: [
282+
"PackageTests.MixedXCTestSuite",
283+
"PackageTests.MixedXCTestSuite/testPassing",
284+
],
282285
});
283286
});
284287
});
@@ -293,7 +296,10 @@ suite("Test Explorer Suite", function () {
293296
);
294297

295298
assertTestResults(testRun, {
296-
passed: ["PackageTests.DebugReleaseTestSuite/testDebug"],
299+
passed: [
300+
"PackageTests.DebugReleaseTestSuite",
301+
"PackageTests.DebugReleaseTestSuite/testDebug",
302+
],
297303
});
298304
});
299305

@@ -308,7 +314,10 @@ suite("Test Explorer Suite", function () {
308314
);
309315

310316
assertTestResults(passingRun, {
311-
passed: ["PackageTests.DebugReleaseTestSuite/testRelease"],
317+
passed: [
318+
"PackageTests.DebugReleaseTestSuite",
319+
"PackageTests.DebugReleaseTestSuite/testRelease",
320+
],
312321
});
313322
});
314323

@@ -516,6 +525,10 @@ suite("Test Explorer Suite", function () {
516525
test: "PackageTests.MixedXCTestSuite/testFailing",
517526
issues: [xcTestFailureMessage],
518527
},
528+
{
529+
issues: [],
530+
test: "PackageTests.MixedXCTestSuite",
531+
},
519532
],
520533
});
521534
});
@@ -530,7 +543,10 @@ suite("Test Explorer Suite", function () {
530543
);
531544

532545
assertTestResults(testRun, {
533-
passed: ["PackageTests.PassingXCTestSuite/testPassing"],
546+
passed: [
547+
"PackageTests.PassingXCTestSuite",
548+
"PackageTests.PassingXCTestSuite/testPassing",
549+
],
534550
});
535551
});
536552

@@ -547,6 +563,10 @@ suite("Test Explorer Suite", function () {
547563
test: "PackageTests.FailingXCTestSuite/testFailing",
548564
issues: [xcTestFailureMessage],
549565
},
566+
{
567+
issues: [],
568+
test: "PackageTests.FailingXCTestSuite",
569+
},
550570
],
551571
});
552572
});
@@ -565,6 +585,10 @@ suite("Test Explorer Suite", function () {
565585
test: "PackageTests.MixedXCTestSuite/testFailing",
566586
issues: [xcTestFailureMessage],
567587
},
588+
{
589+
issues: [],
590+
test: "PackageTests.MixedXCTestSuite",
591+
},
568592
],
569593
});
570594
});

test/suite/testexplorer/XCTestOutputParser.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,31 @@ Test Case '-[MyTests.MyTests`;
263263
});
264264
});
265265

266+
test("Suite", () => {
267+
const testRunState = new TestRunState(["MyTests", "MyTests.MyTests/testPass"], true);
268+
const input = `Test Suite 'MyTests' started at 2024-08-26 13:19:25.325.
269+
Test Case '-[MyTests.MyTests testPass]' started.
270+
Test Case '-[MyTests.MyTests testPass]' passed (0.001 seconds).
271+
Test Suite 'MyTests' passed at 2024-08-26 13:19:25.328.
272+
Executed 1 test, with 0 failures (0 unexpected) in 0.001 (0.001) seconds
273+
`;
274+
outputParser.parseResult(input, testRunState);
275+
276+
assert.deepEqual(testRunState.tests, [
277+
{
278+
name: "MyTests",
279+
output: [],
280+
status: TestStatus.passed,
281+
},
282+
{
283+
name: "MyTests.MyTests/testPass",
284+
status: TestStatus.passed,
285+
timing: { duration: 0.001 },
286+
output: inputToTestOutput(input).slice(1, -2), // trim the suite text
287+
},
288+
]);
289+
});
290+
266291
suite("Diffs", () => {
267292
const testRun = (message: string, expected?: string, actual?: string) => {
268293
const testRunState = new TestRunState(["MyTests.MyTests/testFail"], true);

0 commit comments

Comments
 (0)