Skip to content

Commit 0fce1ee

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 We can work around this by delegating to bash and forwarding the exit code. This causes the underlying pipe to be fully flushed before we recieve the exit event. This workaround was suggested here: chjj/pty.js#110 (comment) Swift currently ships on Ubuntu Debian and Fedora which all come with `bash`. This patch assumes additional Linux distributions added in the future will ship with `bash`. Issue: #1393
1 parent 8b57d89 commit 0fce1ee

File tree

6 files changed

+72
-5
lines changed

6 files changed

+72
-5
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/tasks/SwiftProcess.ts

+35-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import type * as nodePty from "node-pty";
1616
import * as vscode from "vscode";
1717

1818
import { requireNativeModule } from "../utilities/native";
19+
import { regexEscapedString } from "../utilities/utilities";
1920
const { spawn } = requireNativeModule<typeof nodePty>("node-pty");
2021

2122
export interface SwiftProcess {
@@ -98,14 +99,17 @@ 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;
101-
this.spawnedProcess = spawn(this.command, this.args, {
102+
103+
this.spawnedProcess = this.spawnProcess({
104+
// this.spawnedProcess = spawn(this.command, this.args, {
102105
cwd: this.options.cwd,
103106
env: { ...process.env, ...this.options.env },
104107
useConpty,
105108
// https://github.com/swiftlang/vscode-swift/issues/1074
106109
// Causing weird truncation issues
107110
cols: isWindows ? 4096 : undefined,
108111
});
112+
109113
this.spawnEmitter.fire();
110114
this.spawnedProcess.onData(data => {
111115
this.writeEmitter.fire(data);
@@ -152,4 +156,34 @@ export class SwiftPtyProcess implements SwiftProcess {
152156
onDidThrowError: vscode.Event<Error> = this.errorEmitter.event;
153157

154158
onDidClose: vscode.Event<number | void> = this.closeEmitter.event;
159+
160+
private spawnProcess(
161+
options: nodePty.IPtyForkOptions | nodePty.IWindowsPtyForkOptions
162+
): nodePty.IPty {
163+
// node-pty on Linux suffers from a long standing issue where the last
164+
// chunk of output before a program exits is sometimes dropped, especially
165+
// if that program produces a lot of output immediately before exiting.
166+
// See https://github.com/microsoft/node-pty/issues/72
167+
168+
// We can work around this by delegating to bash and forwarding the exit
169+
// code. This causes the underlying pipe to be fully flushed before we recieve
170+
// the exit event. This workaround was suggested here:
171+
// https://github.com/chjj/pty.js/pull/110#issuecomment-406470140
172+
if (process.platform === "linux") {
173+
// In order to be passed as an argument to bash the command arguments
174+
// should not be escaped with the full set of characters normally escaped
175+
// using the command line directly. Unescape and reescape the string with
176+
// a more limited set of characters.
177+
const commandArgs = regexEscapedString(
178+
this.args.join(" ").replaceAll(/\\(.)/g, "$1"),
179+
new Set(["[", "]", ".", "$", "^", "?", "|", "/", ":", "\\"]),
180+
"\\\\\\"
181+
);
182+
183+
const args = ["-c", `${this.command} ${commandArgs}; exit $?`];
184+
return spawn("bash", args, options);
185+
} else {
186+
return spawn(this.command, this.args, options);
187+
}
188+
}
155189
}

src/utilities/utilities.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -350,11 +350,15 @@ const regexEscapedCharacters = new Set(["(", ")", "[", "]", ".", "$", "^", "?",
350350
* @param string A string to escape
351351
* @returns The escaped string
352352
*/
353-
export function regexEscapedString(string: string, omitting?: Set<string>): string {
353+
export function regexEscapedString(
354+
string: string,
355+
omitting?: Set<string>,
356+
escape: string = "\\"
357+
): string {
354358
let result = "";
355359
for (const c of string) {
356360
if (regexEscapedCharacters.has(c) && (!omitting || !omitting.has(c))) {
357-
result += `\\${c}`;
361+
result += `${escape}${c}`;
358362
} else {
359363
result += c;
360364
}

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

+20
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,25 @@ 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+
assert.equal(lastOutputLine?.trim(), needle);
251+
});
252+
233253
test("attachments", async function () {
234254
// Attachments were introduced in 6.1
235255
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)