Support parameterized swift-testing tests (#797)

* 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

@@ -24,6 +24,8 @@ export interface TestClass extends Omit<Omit<LSPTestItem, "location">, "children
     children: TestClass[];
 }
 
+export const runnableTag = new vscode.TestTag("runnable");
+
 /**
  * Update Test Controller TestItems based off array of TestClasses.
  *
@@ -78,8 +80,10 @@ export function updateTests(
         ) {
             const collection = testItem.parent ? testItem.parent.children : testController.items;
 
-            // TODO: This needs to take in to account parameterized tests with no URI, when they're added.
-            if (testItem.children.size === 0) {
+            if (
+                testItem.children.size === 0 ||
+                testItemHasParameterizedTestResultChildren(testItem)
+            ) {
                 collection.delete(testItem.id);
             }
         }
@@ -96,6 +100,21 @@ export function updateTests(
     });
 }
 
+/**
+ * Returns true if all children have no URI.
+ * This indicates the test item is parameterized and the children are the results.
+ */
+function testItemHasParameterizedTestResultChildren(testItem: vscode.TestItem) {
+    return (
+        testItem.children.size > 0 &&
+        reduceTestItemChildren(
+            testItem.children,
+            (acc, child) => acc || child.uri !== undefined,
+            false
+        ) === false
+    );
+}
+
 /**
  * Create a lookup of the incoming tests we can compare to the existing list of tests
  * to produce a list of tests that are no longer present. If a filterFile is specified we
@@ -140,11 +159,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 +180,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;
     }
@@ -174,6 +202,11 @@ function upsertTestItem(
 
     // Manually add the test style as a tag so we can filter by test type.
     newItem.tags = [{ id: testItem.style }, ...testItem.tags];
+
+    if (testItem.disabled === false) {
+        newItem.tags = [...newItem.tags, runnableTag];
+    }
+
     newItem.label = testItem.label;
     newItem.range = testItem.location?.range;
 
@@ -192,4 +225,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,8 +7,12 @@ import {
     WindowsNamedPipeReader,
 } from "./TestEventStreamReader";
 import { ITestRunState } from "./TestRunState";
+import { TestClass } from "../TestDiscovery";
+import { sourceLocationToVSCodeLocation } from "../../utilities/utilities";
 
 // All events produced by a swift-testing run will be one of these three types.
+// Detailed information about swift-testing's JSON schema is available here:
+// https://github.com/apple/swift-testing/blob/main/Documentation/ABI/JSON.md
 export type SwiftTestEvent = MetadataRecord | TestRecord | EventRecord;
 
 interface VersionedRecord {
@@ -21,7 +26,7 @@ interface MetadataRecord extends VersionedRecord {
 
 interface TestRecord extends VersionedRecord {
     kind: "test";
-    payload: Test;
+    payload: TestSuite | TestFunction;
 }
 
 export type EventRecordPayload =
@@ -43,15 +48,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 +89,11 @@ interface BaseEvent {
     testID: string;
 }
 
+interface TestCaseEvent {
+    sourceLocation: SourceLocation;
+    _testCase: TestCase;
+}
+
 interface TestStarted extends BaseEvent {
     kind: "testStarted";
 }
@@ -84,19 +102,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 +133,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 +179,147 @@ 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 ? `${testId}/${this.idFromTestCase(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 idFromTestCase(testCase: TestCase): string {
+        return testCase.id === "argumentIDs: nil" ? testCase.displayName : testCase.id;
+    }
+
+    private parameterizedFunctionTestCaseToTestClass(
+        testId: string,
+        testCase: TestCase,
+        location: vscode.Location,
+        index: number
+    ): TestClass {
+        return {
+            id: this.testCaseId(testId, this.idFromTestCase(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.idFromTestCase(testCase), testCase);
+        });
+        this.testCaseMap.set(record.payload.id, map);
+    }
+
+    private getTestCaseIndex(runState: ITestRunState, testID: string): number {
+        const fullNameIndex = runState.getTestItemIndex(testID, undefined);
+        if (fullNameIndex === -1) {
+            return runState.getTestItemIndex(this.testName(testID), undefined);
+        }
+        return fullNameIndex;
+    }
+
     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(
+                        item.payload.id,
+                        testCase,
+                        sourceLocationToVSCodeLocation(
+                            item.payload.sourceLocation._filePath,
+                            item.payload.sourceLocation.line,
+                            item.payload.sourceLocation.column
+                        ),
+                        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.idFromTestCase(item.payload._testCase)
+                );
+                const testIndex = this.getTestCaseIndex(runState, testID);
+                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 testIndex = runState.getTestItemIndex(testName, undefined);
+                const testID = this.testCaseId(
+                    item.payload.testID,
+                    this.idFromTestCase(item.payload._testCase)
+                );
+                const testIndex = this.getTestCaseIndex(runState, testID);
                 const sourceLocation = item.payload.issue.sourceLocation;
+                const location = sourceLocationToVSCodeLocation(
+                    sourceLocation._filePath,
+                    sourceLocation.line,
+                    sourceLocation.column
+                );
                 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 = this.getTestCaseIndex(runState, item.payload.testID);
+                    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.idFromTestCase(item.payload._testCase)
+                );
+                const testIndex = this.getTestCaseIndex(runState, testID);
+
                 // 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)) {

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