Skip to content

Fix flakey XCTest parallel test failure reporting #1343

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 13, 2025
Merged

Fix flakey XCTest parallel test failure reporting #1343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 37 additions & 2 deletions src/TestExplorer/TestParsers/XCTestOutputParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,9 @@ export class ParallelXCTestOutputParser implements IXCTestOutputParser {
public parseResult(output: string, runState: ITestRunState) {
// From 5.7 to 5.10 running with the --parallel option dumps the test results out
// to the console with no newlines, so it isn't possible to distinguish where errors
// begin and end. Consequently we can't record them, and so we manually mark them
// as passed or failed here with a manufactured issue.
// begin and end. Consequently we can't record them. For these versions we rely on the
// generated xunit XML, which we can parse and mark tests as passed or failed here with
// manufactured issues.
// Don't attempt to parse the console output of parallel tests between 5.7 and 5.10
// as it doesn't have newlines. You might get lucky and find the output is split
// in the right spot, but more often than not we wont be able to parse it.
Expand All @@ -112,8 +113,42 @@ export class ParallelXCTestOutputParser implements IXCTestOutputParser {

/* eslint-disable @typescript-eslint/no-unused-vars */
class ParallelXCTestRunStateProxy implements ITestRunState {
// Note this must remain stateless as its recreated on
// every `parseResult` call in `ParallelXCTestOutputParser`
constructor(private runState: ITestRunState) {}

get excess(): typeof this.runState.excess {
return this.runState.excess;
}

set excess(value: typeof this.runState.excess) {
this.runState.excess = value;
}

get activeSuite(): typeof this.runState.activeSuite {
return this.runState.activeSuite;
}

set activeSuite(value: typeof this.runState.activeSuite) {
this.runState.activeSuite = value;
}

get pendingSuiteOutput(): typeof this.runState.pendingSuiteOutput {
return this.runState.pendingSuiteOutput;
}

set pendingSuiteOutput(value: typeof this.runState.pendingSuiteOutput) {
this.runState.pendingSuiteOutput = value;
}

get failedTest(): typeof this.runState.failedTest {
return this.runState.failedTest;
}

set failedTest(value: typeof this.runState.failedTest) {
this.runState.failedTest = value;
}

getTestItemIndex(id: string, filename: string | undefined): number {
return this.runState.getTestItemIndex(id, filename);
}
Expand Down
16 changes: 9 additions & 7 deletions src/TestExplorer/TestXUnitParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
//===----------------------------------------------------------------------===//

import * as xml2js from "xml2js";
import { TestRunnerTestRunState } from "./TestRunner";
import { OutputChannel } from "vscode";
import { ITestRunState } from "./TestParsers/TestRunState";
import { SwiftOutputChannel } from "../ui/SwiftOutputChannel";

export interface TestResults {
tests: number;
Expand Down Expand Up @@ -49,8 +49,8 @@ export class TestXUnitParser {

async parse(
buffer: string,
runState: TestRunnerTestRunState,
outputChannel: OutputChannel
runState: ITestRunState,
outputChannel: SwiftOutputChannel
): Promise<TestResults | undefined> {
const xml = await xml2js.parseStringPromise(buffer);
try {
Expand All @@ -63,7 +63,8 @@ export class TestXUnitParser {
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
async parseXUnit(xUnit: XUnit, runState: TestRunnerTestRunState): Promise<TestResults> {
async parseXUnit(xUnit: XUnit, runState: ITestRunState): Promise<TestResults> {
// eslint-disable-next-line no-console
let tests = 0;
let failures = 0;
let errors = 0;
Expand All @@ -77,7 +78,7 @@ export class TestXUnitParser {
testsuite.testcase.forEach(testcase => {
className = testcase.$.classname;
const id = `${className}/${testcase.$.name}`;
const index = runState.getTestItemIndex(id);
const index = runState.getTestItemIndex(id, undefined);

// From 5.7 to 5.10 running with the --parallel option dumps the test results out
// to the console with no newlines, so it isn't possible to distinguish where errors
Expand All @@ -86,7 +87,8 @@ export class TestXUnitParser {
if (!!testcase.failure && !this.hasMultiLineParallelTestOutput) {
runState.recordIssue(
index,
testcase.failure.shift()?.$.message ?? "Test Failed"
testcase.failure.shift()?.$.message ?? "Test Failed",
false
);
}
runState.completed(index, { duration: testcase.$.time });
Expand Down
16 changes: 8 additions & 8 deletions test/integration-tests/testexplorer/TestExplorerIntegration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ suite("Test Explorer Suite", function () {
});
});

test(`Runs failing test (${runProfile})`, async function () {
test(`swift-testing Runs failing test (${runProfile})`, async function () {
const testRun = await runTest(
testExplorer,
runProfile,
Expand All @@ -585,7 +585,7 @@ suite("Test Explorer Suite", function () {
});
});

test(`Runs Suite (${runProfile})`, async function () {
test(`swift-testing Runs Suite (${runProfile})`, async function () {
const testRun = await runTest(
testExplorer,
runProfile,
Expand All @@ -610,7 +610,7 @@ suite("Test Explorer Suite", function () {
});
});

test(`Runs parameterized test (${runProfile})`, async function () {
test(`swift-testing Runs parameterized test (${runProfile})`, async function () {
const testId = "PackageTests.parameterizedTest(_:)";
const testRun = await runTest(testExplorer, runProfile, testId);

Expand Down Expand Up @@ -660,7 +660,7 @@ suite("Test Explorer Suite", function () {
assert.deepEqual(unrunnableChildren, [true, true, true]);
});

test(`Runs Suite (${runProfile})`, async function () {
test(`swift-testing Runs Suite (${runProfile})`, async function () {
const testRun = await runTest(
testExplorer,
runProfile,
Expand All @@ -685,7 +685,7 @@ suite("Test Explorer Suite", function () {
});
});

test(`Runs All (${runProfile})`, async function () {
test(`swift-testing Runs All (${runProfile})`, async function () {
const testRun = await runTest(
testExplorer,
runProfile,
Expand Down Expand Up @@ -724,7 +724,7 @@ suite("Test Explorer Suite", function () {
});

suite(`XCTests (${runProfile})`, () => {
test("Runs passing test", async function () {
test(`XCTest Runs passing test (${runProfile})`, async function () {
const testRun = await runTest(
testExplorer,
runProfile,
Expand All @@ -739,7 +739,7 @@ suite("Test Explorer Suite", function () {
});
});

test("Runs failing test", async function () {
test(`XCTest Runs failing test (${runProfile})`, async function () {
const testRun = await runTest(
testExplorer,
runProfile,
Expand All @@ -760,7 +760,7 @@ suite("Test Explorer Suite", function () {
});
});

test("Runs Suite", async function () {
test(`XCTest Runs Suite (${runProfile})`, async function () {
const testRun = await runTest(
testExplorer,
runProfile,
Expand Down
9 changes: 8 additions & 1 deletion test/integration-tests/testexplorer/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,11 @@ export function assertTestResults(
skipped: (state.skipped ?? []).sort(),
errored: (state.errored ?? []).sort(),
unknown: 0,
}
},
`
Build Output:
${testRun.runState.output.join("\n")}
`
);
}

Expand Down Expand Up @@ -280,6 +284,9 @@ export async function runTest(
const testItems = await gatherTests(testExplorer.controller, ...tests);
const request = new vscode.TestRunRequest(testItems);

// The first promise is the return value, the second promise builds and runs
// the tests, populating the TestRunProxy with results and blocking the return
// of that TestRunProxy until the test run is complete.
return (
await Promise.all([
eventPromise(testExplorer.onCreateTestRun),
Expand Down