Support parameterized swift-testing tests

Swift-testing emits a `test` event at the beginning of a test run that
enumerates all the parameterized test cases to be executed.

This patch waits for the `test` event and then generates
`vscode.TestItem`s for each parameterized test execution, parenting them
to their test. Then when we recieve a `runStarted` event we enqueue all
the tests along with the newly created parameterized `TestItem`s.

Before a new test run starts we remove the existing parameterized
`TestItems` so we can regenerate them, as there may be a different
number of parameterized tests run with every execution.

Author: plemarquand

src/SwiftTaskProvider.ts

@@ -187,6 +187,7 @@ export async function getBuildAllTask(folderContext: FolderContext): Promise<vsc
     if (!task) {
         throw Error("Build All Task does not exist");
     }
+
     return task;
 }
 
@@ -252,7 +253,7 @@ export function createSwiftTask(
     const fullCwd = config.cwd.fsPath;
 
     /* Currently there seems to be a bug in vscode where kicking off two tasks
-     with the same definition but different scopes messes with the task 
+     with the same definition but different scopes messes with the task
      completion code. When that is resolved we will go back to the code below
      where we only store the relative cwd instead of the full cwd
 

src/TestExplorer/TestDiscovery.ts

@@ -140,11 +140,11 @@ function deepMergeTestItemChildren(existingItem: vscode.TestItem, newItem: vscod
  * Updates the existing `vscode.TestItem` if it exists with the same ID as the `TestClass`,
  * otherwise creates an add a new one. The location on the returned vscode.TestItem is always updated.
  */
-function upsertTestItem(
+export function upsertTestItem(
     testController: vscode.TestController,
     testItem: TestClass,
     parent?: vscode.TestItem
-) {
+): vscode.TestItem {
     const collection = parent?.children ?? testController.items;
     const existingItem = collection.get(testItem.id);
     let newItem: vscode.TestItem;
@@ -161,6 +161,15 @@ function upsertTestItem(
             testItem.label,
             testItem.location?.uri
         );
+
+        // We want to keep existing children if they exist.
+        if (existingItem) {
+            const existingChildren: vscode.TestItem[] = [];
+            existingItem.children.forEach(child => {
+                existingChildren.push(child);
+            });
+            newItem.children.replace(existingChildren);
+        }
     } else {
         newItem = existingItem;
     }
@@ -192,4 +201,6 @@ function upsertTestItem(
     testItem.children.forEach(child => {
         upsertTestItem(testController, child, newItem);
     });
+
+    return newItem;
 }

src/TestExplorer/TestParsers/SwiftTestingOutputParser.ts

@@ -1,3 +1,4 @@
+import * as vscode from "vscode";
 import * as readline from "readline";
 import { Readable } from "stream";
 import {
@@ -6,6 +7,7 @@ import {
     WindowsNamedPipeReader,
 } from "./TestEventStreamReader";
 import { ITestRunState } from "./TestRunState";
+import { TestClass } from "../TestDiscovery";
 
 // All events produced by a swift-testing run will be one of these three types.
 export type SwiftTestEvent = MetadataRecord | TestRecord | EventRecord;
@@ -21,7 +23,7 @@ interface MetadataRecord extends VersionedRecord {
 
 interface TestRecord extends VersionedRecord {
     kind: "test";
-    payload: Test;
+    payload: TestSuite | TestFunction;
 }
 
 export type EventRecordPayload =
@@ -43,15 +45,23 @@ interface Metadata {
     [key: string]: object; // Currently unstructured content
 }
 
-interface Test {
-    kind: "suite" | "function" | "parameterizedFunction";
+interface TestBase {
     id: string;
     name: string;
     _testCases?: TestCase[];
     sourceLocation: SourceLocation;
 }
 
-interface TestCase {
+interface TestSuite extends TestBase {
+    kind: "suite";
+}
+
+interface TestFunction extends TestBase {
+    kind: "function";
+    isParameterized: boolean;
+}
+
+export interface TestCase {
     id: string;
     displayName: string;
 }
@@ -76,6 +86,11 @@ interface BaseEvent {
     testID: string;
 }
 
+interface TestCaseEvent {
+    sourceLocation: SourceLocation;
+    _testCase: TestCase;
+}
+
 interface TestStarted extends BaseEvent {
     kind: "testStarted";
 }
@@ -84,19 +99,19 @@ interface TestEnded extends BaseEvent {
     kind: "testEnded";
 }
 
-interface TestCaseStarted extends BaseEvent {
+interface TestCaseStarted extends BaseEvent, TestCaseEvent {
     kind: "testCaseStarted";
 }
 
-interface TestCaseEnded extends BaseEvent {
+interface TestCaseEnded extends BaseEvent, TestCaseEvent {
     kind: "testCaseEnded";
 }
 
 interface TestSkipped extends BaseEvent {
     kind: "testSkipped";
 }
 
-interface IssueRecorded extends BaseEvent {
+interface IssueRecorded extends BaseEvent, TestCaseEvent {
     kind: "issueRecorded";
     issue: {
         sourceLocation: SourceLocation;
@@ -115,6 +130,12 @@ export interface SourceLocation {
 
 export class SwiftTestingOutputParser {
     private completionMap = new Map<number, boolean>();
+    private testCaseMap = new Map<string, Map<string, TestCase>>();
+
+    constructor(
+        public testRunStarted: () => void,
+        public addParameterizedTestCase: (testClass: TestClass, parentIndex: number) => void
+    ) {}
 
     /**
      * Watches for test events on the named pipe at the supplied path.
@@ -155,31 +176,131 @@ export class SwiftTestingOutputParser {
         return !matches ? id : matches[1];
     }
 
+    private testCaseId(testId: string, testCaseId: string): string {
+        const testCase = this.testCaseMap.get(testId)?.get(testCaseId);
+        return testCase ? this.createTestCaseId(testCase) : testId;
+    }
+
+    // Test cases do not have a unique ID if their arguments are not serializable
+    // with Codable. If they aren't, their id appears as `argumentIDs: nil`, and we
+    // fall back to using the testCase display name as the test case ID. This isn't
+    // ideal because its possible to have multiple test cases with the same display name,
+    // but until we have a better solution for identifying test cases it will have to do.
+    // SEE: rdar://119522099.
+    private createTestCaseId(testCase: TestCase): string {
+        return testCase.id === "argumentIDs: nil" ? testCase.displayName : testCase.id;
+    }
+
+    private parameterizedFunctionTestCaseToTestClass(
+        testCase: TestCase,
+        location: vscode.Location,
+        index: number
+    ): TestClass {
+        return {
+            id: this.createTestCaseId(testCase),
+            label: testCase.displayName,
+            tags: [],
+            children: [],
+            style: "swift-testing",
+            location: location,
+            disabled: true,
+            sortText: `${index}`.padStart(8, "0"),
+        };
+    }
+
+    private buildTestCaseMapForParameterizedTest(record: TestRecord) {
+        const map = new Map<string, TestCase>();
+        (record.payload._testCases ?? []).forEach(testCase => {
+            map.set(this.createTestCaseId(testCase), testCase);
+        });
+        this.testCaseMap.set(record.payload.id, map);
+    }
+
     private parse(item: SwiftTestEvent, runState: ITestRunState) {
-        if (item.kind === "event") {
-            if (item.payload.kind === "testCaseStarted" || item.payload.kind === "testStarted") {
+        if (
+            item.kind === "test" &&
+            item.payload.kind === "function" &&
+            item.payload.isParameterized &&
+            item.payload._testCases
+        ) {
+            // Store a map of [Test ID, [Test Case ID, TestCase]] so we can quickly
+            // map an event.payload.testID back to a test case.
+            this.buildTestCaseMapForParameterizedTest(item);
+
+            const testName = this.testName(item.payload.id);
+            const testIndex = runState.getTestItemIndex(testName, undefined);
+            // If a test has test cases it is paramterized and we need to notify
+            // the caller that the TestClass should be added to the vscode.TestRun
+            // before it starts.
+            item.payload._testCases
+                .map((testCase, index) =>
+                    this.parameterizedFunctionTestCaseToTestClass(
+                        testCase,
+                        sourceLocationToVSCodeLocation(item.payload.sourceLocation),
+                        index
+                    )
+                )
+                .flatMap(testClass => (testClass ? [testClass] : []))
+                .forEach(testClass => this.addParameterizedTestCase(testClass, testIndex));
+        } else if (item.kind === "event") {
+            if (item.payload.kind === "runStarted") {
+                // Notify the runner that we've recieved all the test cases and
+                // are going to start running tests now.
+                this.testRunStarted();
+            } else if (item.payload.kind === "testStarted") {
                 const testName = this.testName(item.payload.testID);
                 const testIndex = runState.getTestItemIndex(testName, undefined);
                 runState.started(testIndex, item.payload.instant.absolute);
+            } else if (item.payload.kind === "testCaseStarted") {
+                const testID = this.testCaseId(
+                    item.payload.testID,
+                    this.createTestCaseId(item.payload._testCase)
+                );
+                const testName = this.testName(testID);
+                const testIndex = runState.getTestItemIndex(testName, undefined);
+                runState.started(testIndex, item.payload.instant.absolute);
             } else if (item.payload.kind === "testSkipped") {
                 const testName = this.testName(item.payload.testID);
                 const testIndex = runState.getTestItemIndex(testName, undefined);
                 runState.skipped(testIndex);
             } else if (item.payload.kind === "issueRecorded") {
-                const testName = this.testName(item.payload.testID);
+                const testID = this.testCaseId(
+                    item.payload.testID,
+                    this.createTestCaseId(item.payload._testCase)
+                );
+                const testName = this.testName(testID);
                 const testIndex = runState.getTestItemIndex(testName, undefined);
-                const sourceLocation = item.payload.issue.sourceLocation;
+                const location = sourceLocationToVSCodeLocation(item.payload.issue.sourceLocation);
                 item.payload.messages.forEach(message => {
-                    runState.recordIssue(testIndex, message.text, {
-                        file: sourceLocation._filePath,
-                        line: sourceLocation.line,
-                        column: sourceLocation.column,
-                    });
+                    runState.recordIssue(testIndex, message.text, location);
                 });
-            } else if (item.payload.kind === "testCaseEnded" || item.payload.kind === "testEnded") {
+
+                if (testID !== item.payload.testID) {
+                    const testName = this.testName(item.payload.testID);
+                    const testIndex = runState.getTestItemIndex(testName, undefined);
+                    item.payload.messages.forEach(message => {
+                        runState.recordIssue(testIndex, message.text, location);
+                    });
+                }
+            } else if (item.payload.kind === "testEnded") {
                 const testName = this.testName(item.payload.testID);
                 const testIndex = runState.getTestItemIndex(testName, undefined);
 
+                // When running a single test the testEnded and testCaseEnded events
+                // have the same ID, and so we'd end the same test twice.
+                if (this.completionMap.get(testIndex)) {
+                    return;
+                }
+                this.completionMap.set(testIndex, true);
+                runState.completed(testIndex, { timestamp: item.payload.instant.absolute });
+            } else if (item.payload.kind === "testCaseEnded") {
+                const testID = this.testCaseId(
+                    item.payload.testID,
+                    this.createTestCaseId(item.payload._testCase)
+                );
+                const testName = this.testName(testID);
+                const testIndex = runState.getTestItemIndex(testName, undefined);
+
                 // When running a single test the testEnded and testCaseEnded events
                 // have the same ID, and so we'd end the same test twice.
                 if (this.completionMap.get(testIndex)) {
@@ -191,3 +312,10 @@ export class SwiftTestingOutputParser {
         }
     }
 }
+
+function sourceLocationToVSCodeLocation(sourceLocation: SourceLocation): vscode.Location {
+    return new vscode.Location(
+        vscode.Uri.file(sourceLocation._filePath),
+        new vscode.Position(sourceLocation.line - 1, sourceLocation?.column ?? 0)
+    );
+}

src/TestExplorer/TestParsers/TestRunState.ts

@@ -1,4 +1,4 @@
-import { MarkdownString } from "vscode";
+import * as vscode from "vscode";
 
 /**
  * Interface for setting this test runs state
@@ -26,11 +26,12 @@ export interface ITestRunState {
     // otherwise the time passed is assumed to be the duration.
     completed(index: number, timing: { duration: number } | { timestamp: number }): void;
 
-    // record an issue against a test
+    // record an issue against a test.
+    // If a `testCase` is provided a new TestItem will be created under the TestItem at the supplied index.
     recordIssue(
         index: number,
-        message: string | MarkdownString,
-        location?: { file: string; line: number; column?: number }
+        message: string | vscode.MarkdownString,
+        location?: vscode.Location
     ): void;
 
     // set test index to have been skipped

src/TestExplorer/TestParsers/XCTestOutputParser.ts

@@ -12,6 +12,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+import * as vscode from "vscode";
 import { ITestRunState } from "./TestRunState";
 
 /** Regex for parsing XCTest output */
@@ -228,6 +229,10 @@ export class XCTestOutputParser {
         runState.failedTest = undefined;
     }
 
+    private sourceLocationToVSCodeLocation(file: string, lineNumber: number): vscode.Location {
+        return new vscode.Location(vscode.Uri.file(file), new vscode.Position(lineNumber - 1, 0));
+    }
+
     /** Start capture error message */
     private startErrorMessage(
         testIndex: number,
@@ -238,10 +243,11 @@ export class XCTestOutputParser {
     ) {
         // If we were already capturing an error record it and start a new one
         if (runState.failedTest) {
-            runState.recordIssue(testIndex, runState.failedTest.message, {
-                file: runState.failedTest.file,
-                line: runState.failedTest.lineNumber,
-            });
+            const location = this.sourceLocationToVSCodeLocation(
+                runState.failedTest.file,
+                runState.failedTest.lineNumber
+            );
+            runState.recordIssue(testIndex, runState.failedTest.message, location);
             runState.failedTest.complete = true;
         }
         runState.failedTest = {
@@ -269,10 +275,11 @@ export class XCTestOutputParser {
     ) {
         if (testIndex !== -1) {
             if (runState.failedTest) {
-                runState.recordIssue(testIndex, runState.failedTest.message, {
-                    file: runState.failedTest.file,
-                    line: runState.failedTest.lineNumber,
-                });
+                const location = this.sourceLocationToVSCodeLocation(
+                    runState.failedTest.file,
+                    runState.failedTest.lineNumber
+                );
+                runState.recordIssue(testIndex, runState.failedTest.message, location);
             } else {
                 runState.recordIssue(testIndex, "Failed");
             }