Skip to content

Commit 78337ff

Browse files
committed
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 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
1 parent 8b57d89 commit 78337ff

File tree

8 files changed

+138
-6
lines changed

8 files changed

+138
-6
lines changed

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

+8
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,14 @@ struct MixedSwiftTestingSuite {
102102
}
103103
#expect(2 == 3)
104104
}
105+
106+
@Test func testLotsOfOutput() {
107+
var string = ""
108+
for i in 1...100_000 {
109+
string += "\(i)\n"
110+
}
111+
print(string)
112+
}
105113
#endif
106114

107115
#if swift(>=6.1)

src/TestExplorer/TestRunArguments.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -93,11 +93,12 @@ export class TestRunArguments {
9393
const terminator = hasChildren ? "/" : "$";
9494
// Debugging XCTests requires exact matches, so we don't need a trailing terminator.
9595
return isDebug ? arg.id : `${arg.id}${terminator}`;
96-
} else {
96+
} else if (hasChildren) {
9797
// Append a trailing slash to match a suite name exactly.
9898
// This prevents TestTarget.MySuite matching TestTarget.MySuite2.
9999
return `${arg.id}/`;
100100
}
101+
return arg.id;
101102
});
102103
}
103104

src/TestExplorer/TestRunner.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -715,7 +715,8 @@ export class TestRunner {
715715
presentationOptions: { reveal: vscode.TaskRevealKind.Never },
716716
},
717717
this.folderContext.workspaceContext.toolchain,
718-
{ ...process.env, ...testBuildConfig.env }
718+
{ ...process.env, ...testBuildConfig.env },
719+
true
719720
);
720721

721722
task.execution.onDidWrite(str => {

src/tasks/SwiftExecution.ts

+7-2
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@
1313
//===----------------------------------------------------------------------===//
1414

1515
import * as vscode from "vscode";
16-
import { SwiftProcess, SwiftPtyProcess } from "./SwiftProcess";
16+
import { ReadOnlySwiftProcess, SwiftProcess, SwiftPtyProcess } from "./SwiftProcess";
1717
import { SwiftPseudoterminal } from "./SwiftPseudoterminal";
1818

1919
export interface SwiftExecutionOptions extends vscode.ProcessExecutionOptions {
2020
presentation?: vscode.TaskPresentationOptions;
21+
readOnlyTerminal?: boolean;
2122
}
2223

2324
/**
@@ -30,11 +31,15 @@ export class SwiftExecution extends vscode.CustomExecution {
3031
public readonly command: string,
3132
public readonly args: string[],
3233
public readonly options: SwiftExecutionOptions,
33-
private readonly swiftProcess: SwiftProcess = new SwiftPtyProcess(command, args, options)
34+
private readonly swiftProcess: SwiftProcess = options.readOnlyTerminal
35+
? new ReadOnlySwiftProcess(command, args, options)
36+
: new SwiftPtyProcess(command, args, options)
3437
) {
3538
super(async () => {
3639
return new SwiftPseudoterminal(swiftProcess, options.presentation || {});
3740
});
41+
42+
this.swiftProcess = swiftProcess;
3843
this.onDidWrite = swiftProcess.onDidWrite;
3944
this.onDidClose = swiftProcess.onDidClose;
4045
}

src/tasks/SwiftProcess.ts

+90
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
//===----------------------------------------------------------------------===//
1414

1515
import type * as nodePty from "node-pty";
16+
import * as child_process from "child_process";
1617
import * as vscode from "vscode";
1718

1819
import { requireNativeModule } from "../utilities/native";
@@ -98,6 +99,7 @@ export class SwiftPtyProcess implements SwiftProcess {
9899
// The pty process hangs on Windows when debugging the extension if we use conpty
99100
// See https://github.com/microsoft/node-pty/issues/640
100101
const useConpty = isWindows && process.env["VSCODE_DEBUG"] === "1" ? false : true;
102+
101103
this.spawnedProcess = spawn(this.command, this.args, {
102104
cwd: this.options.cwd,
103105
env: { ...process.env, ...this.options.env },
@@ -153,3 +155,91 @@ export class SwiftPtyProcess implements SwiftProcess {
153155

154156
onDidClose: vscode.Event<number | void> = this.closeEmitter.event;
155157
}
158+
159+
/**
160+
* A {@link SwiftProcess} that spawns a child process and does not bind to stdio.
161+
*
162+
* Use this for Swift tasks that do not need to accept input, as its lighter weight and
163+
* less error prone than using a spawned node-pty process.
164+
*
165+
* Specifically node-pty on Linux suffers from a long standing issue where the last chunk
166+
* of output before a program exits is sometimes dropped, especially if that program produces
167+
* a lot of output immediately before exiting. See https://github.com/microsoft/node-pty/issues/72
168+
*/
169+
export class ReadOnlySwiftProcess implements SwiftProcess {
170+
private readonly spawnEmitter: vscode.EventEmitter<void> = new vscode.EventEmitter<void>();
171+
private readonly writeEmitter: vscode.EventEmitter<string> = new vscode.EventEmitter<string>();
172+
private readonly errorEmitter: vscode.EventEmitter<Error> = new vscode.EventEmitter<Error>();
173+
private readonly closeEmitter: vscode.EventEmitter<number | void> = new vscode.EventEmitter<
174+
number | void
175+
>();
176+
177+
private spawnedProcess: child_process.ChildProcessWithoutNullStreams | undefined;
178+
179+
constructor(
180+
public readonly command: string,
181+
public readonly args: string[],
182+
private readonly options: vscode.ProcessExecutionOptions = {}
183+
) {}
184+
185+
spawn(): void {
186+
try {
187+
this.spawnedProcess = child_process.spawn(this.command, this.args, {
188+
cwd: this.options.cwd,
189+
env: { ...process.env, ...this.options.env },
190+
});
191+
192+
this.spawnedProcess.stdout.on("data", data => {
193+
this.writeEmitter.fire(data.toString());
194+
});
195+
196+
this.spawnedProcess.stderr.on("data", data => {
197+
this.writeEmitter.fire(data.toString());
198+
});
199+
200+
this.spawnedProcess.on("error", error => {
201+
this.errorEmitter.fire(new Error(`${error}`));
202+
this.closeEmitter.fire();
203+
});
204+
205+
this.spawnedProcess.once("exit", code => {
206+
this.closeEmitter.fire(code ?? undefined);
207+
this.dispose();
208+
});
209+
} catch (error) {
210+
this.errorEmitter.fire(new Error(`${error}`));
211+
this.closeEmitter.fire();
212+
this.dispose();
213+
}
214+
}
215+
216+
handleInput(_s: string): void {
217+
// Do nothing
218+
}
219+
220+
terminate(signal?: NodeJS.Signals): void {
221+
if (!this.spawnedProcess) {
222+
return;
223+
}
224+
this.spawnedProcess.kill(signal);
225+
this.dispose();
226+
}
227+
228+
setDimensions(_dimensions: vscode.TerminalDimensions): void {
229+
// Do nothing
230+
}
231+
232+
dispose(): void {
233+
this.spawnedProcess?.stdout.removeAllListeners();
234+
this.spawnedProcess?.stderr.removeAllListeners();
235+
this.spawnedProcess?.removeAllListeners();
236+
}
237+
238+
onDidSpawn: vscode.Event<void> = this.spawnEmitter.event;
239+
240+
onDidWrite: vscode.Event<string> = this.writeEmitter.event;
241+
242+
onDidThrowError: vscode.Event<Error> = this.errorEmitter.event;
243+
244+
onDidClose: vscode.Event<number | void> = this.closeEmitter.event;
245+
}

src/tasks/SwiftTaskProvider.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,8 @@ export function createSwiftTask(
268268
name: string,
269269
config: TaskConfig,
270270
toolchain: SwiftToolchain,
271-
cmdEnv: { [key: string]: string } = {}
271+
cmdEnv: { [key: string]: string } = {},
272+
readOnlyTerminal: boolean = false
272273
): SwiftTask {
273274
const swift = toolchain.getToolchainExecutable("swift");
274275
args = toolchain.buildFlags.withAdditionalFlags(args);
@@ -313,6 +314,7 @@ export function createSwiftTask(
313314
cwd: fullCwd,
314315
env: env,
315316
presentation,
317+
readOnlyTerminal,
316318
})
317319
);
318320
// This doesn't include any quotes added by VS Code.

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

+25
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ suite("Test Explorer Suite", function () {
190190
["testPassing()", "testFailing()", "testDisabled()"],
191191
"testWithKnownIssue()",
192192
"testWithKnownIssueAndUnknownIssue()",
193+
"testLotsOfOutput()",
193194
"testAttachment()",
194195
"DuplicateSuffixTests",
195196
["testPassing()", "testPassingSuffix()"],
@@ -230,6 +231,30 @@ suite("Test Explorer Suite", function () {
230231
}
231232
});
232233

234+
test("captures lots of output", async () => {
235+
const testRun = await runTest(
236+
testExplorer,
237+
TestKind.standard,
238+
"PackageTests.testLotsOfOutput()"
239+
);
240+
241+
assertTestResults(testRun, {
242+
passed: ["PackageTests.testLotsOfOutput()"],
243+
});
244+
245+
// Right now the swift-testing "test run complete" text is being emitted
246+
// in the middle of the print, so the last line is actually end end of our
247+
// huge string. If they fix this in future this `find` ensures the test wont break.
248+
const needle = "100000";
249+
const lastOutputLine = testRun.runState.output.find(row => row.trim() === needle);
250+
const lastTenLines = testRun.runState.output.slice(-10).join("\n");
251+
assert.equal(
252+
lastOutputLine?.trim(),
253+
needle,
254+
`Expected all test output to be captured, but it was truncated. Last 10 lines of output were: ${lastTenLines}`
255+
);
256+
});
257+
233258
test("attachments", async function () {
234259
// Attachments were introduced in 6.1
235260
if (workspaceContext.swiftVersion.isLessThan(new Version(6, 1, 0))) {

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ suite("TestRunArguments Suite", () => {
240240
const testArgs = new TestRunArguments(runRequestByIds([anotherSwiftTestId]), false);
241241
assertRunArguments(testArgs, {
242242
xcTestArgs: [],
243-
swiftTestArgs: [`${anotherSwiftTestId}/`],
243+
swiftTestArgs: [anotherSwiftTestId],
244244
testItems: [swiftTestSuiteId, testTargetId, anotherSwiftTestId],
245245
});
246246
});

0 commit comments

Comments
 (0)