From da26370853fbb06cab8bc10602aed3118428e9bd Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Sat, 22 Feb 2025 19:48:16 -0500 Subject: [PATCH] Fix missing Test Result output on Linux when using `print` 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 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 --- .../Tests/PackageTests/PackageTests.swift | 8 ++ src/TestExplorer/TestRunArguments.ts | 3 +- src/TestExplorer/TestRunner.ts | 3 +- src/tasks/SwiftExecution.ts | 9 +- src/tasks/SwiftProcess.ts | 89 +++++++++++++++++++ src/tasks/SwiftTaskProvider.ts | 4 +- .../TestExplorerIntegration.test.ts | 31 ++++++- .../testexplorer/TestRunArguments.test.ts | 2 +- .../testexplorer/utilities.ts | 18 +++- 9 files changed, 157 insertions(+), 10 deletions(-) diff --git a/assets/test/defaultPackage/Tests/PackageTests/PackageTests.swift b/assets/test/defaultPackage/Tests/PackageTests/PackageTests.swift index 835e7f127..dc856f6ce 100644 --- a/assets/test/defaultPackage/Tests/PackageTests/PackageTests.swift +++ b/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) diff --git a/src/TestExplorer/TestRunArguments.ts b/src/TestExplorer/TestRunArguments.ts index 027e3c2ec..d6d4a5fa6 100644 --- a/src/TestExplorer/TestRunArguments.ts +++ b/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; }); } diff --git a/src/TestExplorer/TestRunner.ts b/src/TestExplorer/TestRunner.ts index d9fae8a14..abf54b60d 100644 --- a/src/TestExplorer/TestRunner.ts +++ b/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 => { diff --git a/src/tasks/SwiftExecution.ts b/src/tasks/SwiftExecution.ts index f9c6c5502..3cb73495f 100644 --- a/src/tasks/SwiftExecution.ts +++ b/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; } diff --git a/src/tasks/SwiftProcess.ts b/src/tasks/SwiftProcess.ts index 1c0ce05e8..0a41726c9 100644 --- a/src/tasks/SwiftProcess.ts +++ b/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 = 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 = new vscode.EventEmitter(); + private readonly writeEmitter: vscode.EventEmitter = new vscode.EventEmitter(); + private readonly errorEmitter: vscode.EventEmitter = new vscode.EventEmitter(); + private readonly closeEmitter: vscode.EventEmitter = 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 = this.spawnEmitter.event; + + onDidWrite: vscode.Event = this.writeEmitter.event; + + onDidThrowError: vscode.Event = this.errorEmitter.event; + + onDidClose: vscode.Event = this.closeEmitter.event; +} diff --git a/src/tasks/SwiftTaskProvider.ts b/src/tasks/SwiftTaskProvider.ts index 77bd93689..40ac5179f 100644 --- a/src/tasks/SwiftTaskProvider.ts +++ b/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. diff --git a/test/integration-tests/testexplorer/TestExplorerIntegration.test.ts b/test/integration-tests/testexplorer/TestExplorerIntegration.test.ts index aa5c34d09..b613bdc82 100644 --- a/test/integration-tests/testexplorer/TestExplorerIntegration.test.ts +++ b/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()"], diff --git a/test/integration-tests/testexplorer/TestRunArguments.test.ts b/test/integration-tests/testexplorer/TestRunArguments.test.ts index 6fc75aa8f..f435d2774 100644 --- a/test/integration-tests/testexplorer/TestRunArguments.test.ts +++ b/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], }); }); diff --git a/test/integration-tests/testexplorer/utilities.ts b/test/integration-tests/testexplorer/utilities.ts index 00460a88e..b049f74c7 100644 --- a/test/integration-tests/testexplorer/utilities.ts +++ b/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(array: T[], value: T) { - assert.ok(array.includes(value), `${value} is not in ${array}`); +export function assertContains(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}`); } /**