Fix missing Test Result output on Linux when using print (#1401)

node-pty on Linux suffers from a long standing issue where the last
chunk of output before a program exits is sometimes dropped, especially
if that program produces a lot of output immediately before exiting.
See microsoft/node-pty#72

For swift processes that don't require input and may produce a lot of
output that could potentially be truncated we can add a new
`ReadOnlySwiftProcess` that spawns a child process using node's built
in `child_process`. These spawned child processes emit their full
output before exiting without the need to resort to flakey workarounds
on the node-pty process.

When creating new `SwiftExecution`s in future `readOnlyTerminal` should
be set to `true` unless the process may require user input.

Issue: #1393

Author: plemarquand

assets/test/defaultPackage/Tests/PackageTests/PackageTests.swift

@@ -102,6 +102,14 @@ struct MixedSwiftTestingSuite {
   }
   #expect(2 == 3)
 }
+
+@Test func testLotsOfOutput() {
+  var string = ""
+  for i in 1...100_000 {
+    string += "\(i)\n"
+  }
+  print(string)
+}
 #endif
 
 #if swift(>=6.1)

src/TestExplorer/TestRunArguments.ts

@@ -93,11 +93,12 @@ export class TestRunArguments {
                 const terminator = hasChildren ? "/" : "$";
                 // Debugging XCTests requires exact matches, so we don't need a trailing terminator.
                 return isDebug ? arg.id : `${arg.id}${terminator}`;
-            } else {
+            } else if (hasChildren) {
                 // Append a trailing slash to match a suite name exactly.
                 // This prevents TestTarget.MySuite matching TestTarget.MySuite2.
                 return `${arg.id}/`;
             }
+            return arg.id;
         });
     }
 

src/TestExplorer/TestRunner.ts

@@ -715,7 +715,8 @@ export class TestRunner {
                     presentationOptions: { reveal: vscode.TaskRevealKind.Never },
                 },
                 this.folderContext.workspaceContext.toolchain,
-                { ...process.env, ...testBuildConfig.env }
+                { ...process.env, ...testBuildConfig.env },
+                { readOnlyTerminal: process.platform !== "win32" }
             );
 
             task.execution.onDidWrite(str => {

src/tasks/SwiftExecution.ts

@@ -13,11 +13,12 @@
 //===----------------------------------------------------------------------===//
 
 import * as vscode from "vscode";
-import { SwiftProcess, SwiftPtyProcess } from "./SwiftProcess";
+import { ReadOnlySwiftProcess, SwiftProcess, SwiftPtyProcess } from "./SwiftProcess";
 import { SwiftPseudoterminal } from "./SwiftPseudoterminal";
 
 export interface SwiftExecutionOptions extends vscode.ProcessExecutionOptions {
     presentation?: vscode.TaskPresentationOptions;
+    readOnlyTerminal?: boolean;
 }
 
 /**
@@ -30,11 +31,15 @@ export class SwiftExecution extends vscode.CustomExecution {
         public readonly command: string,
         public readonly args: string[],
         public readonly options: SwiftExecutionOptions,
-        private readonly swiftProcess: SwiftProcess = new SwiftPtyProcess(command, args, options)
+        private readonly swiftProcess: SwiftProcess = options.readOnlyTerminal
+            ? new ReadOnlySwiftProcess(command, args, options)
+            : new SwiftPtyProcess(command, args, options)
     ) {
         super(async () => {
             return new SwiftPseudoterminal(swiftProcess, options.presentation || {});
         });
+
+        this.swiftProcess = swiftProcess;
         this.onDidWrite = swiftProcess.onDidWrite;
         this.onDidClose = swiftProcess.onDidClose;
     }

src/tasks/SwiftProcess.ts

@@ -13,6 +13,7 @@
 //===----------------------------------------------------------------------===//
 
 import type * as nodePty from "node-pty";
+import * as child_process from "child_process";
 import * as vscode from "vscode";
 
 import { requireNativeModule } from "../utilities/native";
@@ -153,3 +154,91 @@ export class SwiftPtyProcess implements SwiftProcess {
 
     onDidClose: vscode.Event<number | void> = this.closeEmitter.event;
 }
+
+/**
+ * A {@link SwiftProcess} that spawns a child process and does not bind to stdio.
+ *
+ * Use this for Swift tasks that do not need to accept input, as its lighter weight and
+ * less error prone than using a spawned node-pty process.
+ *
+ * Specifically node-pty on Linux suffers from a long standing issue where the last chunk
+ * of output before a program exits is sometimes dropped, especially if that program produces
+ * a lot of output immediately before exiting. See https://github.com/microsoft/node-pty/issues/72
+ */
+export class ReadOnlySwiftProcess implements SwiftProcess {
+    private readonly spawnEmitter: vscode.EventEmitter<void> = new vscode.EventEmitter<void>();
+    private readonly writeEmitter: vscode.EventEmitter<string> = new vscode.EventEmitter<string>();
+    private readonly errorEmitter: vscode.EventEmitter<Error> = new vscode.EventEmitter<Error>();
+    private readonly closeEmitter: vscode.EventEmitter<number | void> = new vscode.EventEmitter<
+        number | void
+    >();
+
+    private spawnedProcess: child_process.ChildProcessWithoutNullStreams | undefined;
+
+    constructor(
+        public readonly command: string,
+        public readonly args: string[],
+        private readonly options: vscode.ProcessExecutionOptions = {}
+    ) {}
+
+    spawn(): void {
+        try {
+            this.spawnedProcess = child_process.spawn(this.command, this.args, {
+                cwd: this.options.cwd,
+                env: { ...process.env, ...this.options.env },
+            });
+
+            this.spawnedProcess.stdout.on("data", data => {
+                this.writeEmitter.fire(data.toString());
+            });
+
+            this.spawnedProcess.stderr.on("data", data => {
+                this.writeEmitter.fire(data.toString());
+            });
+
+            this.spawnedProcess.on("error", error => {
+                this.errorEmitter.fire(new Error(`${error}`));
+                this.closeEmitter.fire();
+            });
+
+            this.spawnedProcess.once("exit", code => {
+                this.closeEmitter.fire(code ?? undefined);
+                this.dispose();
+            });
+        } catch (error) {
+            this.errorEmitter.fire(new Error(`${error}`));
+            this.closeEmitter.fire();
+            this.dispose();
+        }
+    }
+
+    handleInput(_s: string): void {
+        // Do nothing
+    }
+
+    terminate(signal?: NodeJS.Signals): void {
+        if (!this.spawnedProcess) {
+            return;
+        }
+        this.spawnedProcess.kill(signal);
+        this.dispose();
+    }
+
+    setDimensions(_dimensions: vscode.TerminalDimensions): void {
+        // Do nothing
+    }
+
+    dispose(): void {
+        this.spawnedProcess?.stdout.removeAllListeners();
+        this.spawnedProcess?.stderr.removeAllListeners();
+        this.spawnedProcess?.removeAllListeners();
+    }
+
+    onDidSpawn: vscode.Event<void> = this.spawnEmitter.event;
+
+    onDidWrite: vscode.Event<string> = this.writeEmitter.event;
+
+    onDidThrowError: vscode.Event<Error> = this.errorEmitter.event;
+
+    onDidClose: vscode.Event<number | void> = this.closeEmitter.event;
+}

src/tasks/SwiftTaskProvider.ts

@@ -268,7 +268,8 @@ export function createSwiftTask(
     name: string,
     config: TaskConfig,
     toolchain: SwiftToolchain,
-    cmdEnv: { [key: string]: string } = {}
+    cmdEnv: { [key: string]: string } = {},
+    options: { readOnlyTerminal: boolean } = { readOnlyTerminal: false }
 ): SwiftTask {
     const swift = toolchain.getToolchainExecutable("swift");
     args = toolchain.buildFlags.withAdditionalFlags(args);
@@ -313,6 +314,7 @@ export function createSwiftTask(
             cwd: fullCwd,
             env: env,
             presentation,
+            readOnlyTerminal: options.readOnlyTerminal,
         })
     );
     // This doesn't include any quotes added by VS Code.

test/integration-tests/testexplorer/TestExplorerIntegration.test.ts

@@ -20,6 +20,7 @@ import { beforeEach, afterEach } from "mocha";
 import { TestExplorer } from "../../../src/TestExplorer/TestExplorer";
 import {
     assertContains,
+    assertContainsTrimmed,
     assertTestControllerHierarchy,
     assertTestResults,
     eventPromise,
@@ -190,6 +191,7 @@ suite("Test Explorer Suite", function () {
                         ["testPassing()", "testFailing()", "testDisabled()"],
                         "testWithKnownIssue()",
                         "testWithKnownIssueAndUnknownIssue()",
+                        "testLotsOfOutput()",
                         "testAttachment()",
                         "DuplicateSuffixTests",
                         ["testPassing()", "testPassingSuffix()"],
@@ -230,6 +232,29 @@ suite("Test Explorer Suite", function () {
                 }
             });
 
+            test("captures lots of output", async () => {
+                const testRun = await runTest(
+                    testExplorer,
+                    TestKind.standard,
+                    "PackageTests.testLotsOfOutput()"
+                );
+
+                assertTestResults(testRun, {
+                    passed: ["PackageTests.testLotsOfOutput()"],
+                });
+
+                // Right now the swift-testing "test run complete" text is being emitted
+                // in the middle of the print, so the last line is actually end end of our
+                // huge string. If they fix this in future this `find` ensures the test wont break.
+                const needle = "100000";
+                const lastTenLines = testRun.runState.output.slice(-10).join("\n");
+                assertContainsTrimmed(
+                    testRun.runState.output,
+                    needle,
+                    `Expected all test output to be captured, but it was truncated. Last 10 lines of output were: ${lastTenLines}`
+                );
+            });
+
             test("attachments", async function () {
                 // Attachments were introduced in 6.1
                 if (workspaceContext.swiftVersion.isLessThan(new Version(6, 1, 0))) {
@@ -535,9 +560,11 @@ suite("Test Explorer Suite", function () {
                             "PackageTests.topLevelTestPassing()"
                         );
 
-                        assertContains(
+                        // Use assertContainsTrimmed to ignore the line ending differences
+                        // across platforms (windows vs linux/darwin)
+                        assertContainsTrimmed(
                             testRun.runState.output,
-                            "A print statement in a test.\r\r\n"
+                            "A print statement in a test."
                         );
                         assertTestResults(testRun, {
                             passed: ["PackageTests.topLevelTestPassing()"],

test/integration-tests/testexplorer/TestRunArguments.test.ts

@@ -240,7 +240,7 @@ suite("TestRunArguments Suite", () => {
         const testArgs = new TestRunArguments(runRequestByIds([anotherSwiftTestId]), false);
         assertRunArguments(testArgs, {
             xcTestArgs: [],
-            swiftTestArgs: [`${anotherSwiftTestId}/`],
+            swiftTestArgs: [anotherSwiftTestId],
             testItems: [swiftTestSuiteId, testTargetId, anotherSwiftTestId],
         });
     });

test/integration-tests/testexplorer/utilities.ts

@@ -110,9 +110,23 @@ export function assertTestControllerHierarchy(
  *
  * @param array The array to check.
  * @param value The value to check for.
+ * @param message An optional message to display if the assertion fails.
  */
-export function assertContains<T>(array: T[], value: T) {
-    assert.ok(array.includes(value), `${value} is not in ${array}`);
+export function assertContains<T>(array: T[], value: T, message?: string) {
+    assert.ok(array.includes(value), message ?? `${value} is not in ${array}`);
+}
+
+/**
+ * Asserts that an array of strings contains the value ignoring
+ * leading/trailing whitespace.
+ *
+ * @param array The array to check.
+ * @param value The value to check for.
+ * @param message An optional message to display if the assertion fails.
+ */
+export function assertContainsTrimmed(array: string[], value: string, message?: string) {
+    const found = array.find(row => row.trim() === value);
+    assert.ok(found, message ?? `${value} is not in ${array}`);
 }
 
 /**