Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion src/TestExplorer/TestRunArguments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
}

Expand Down
3 changes: 2 additions & 1 deletion src/TestExplorer/TestRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
9 changes: 7 additions & 2 deletions src/tasks/SwiftExecution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand All @@ -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;
}
Expand Down
89 changes: 89 additions & 0 deletions src/tasks/SwiftProcess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;
}
4 changes: 3 additions & 1 deletion src/tasks/SwiftTaskProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { beforeEach, afterEach } from "mocha";
import { TestExplorer } from "../../../src/TestExplorer/TestExplorer";
import {
assertContains,
assertContainsTrimmed,
assertTestControllerHierarchy,
assertTestResults,
eventPromise,
Expand Down Expand Up @@ -190,6 +191,7 @@ suite("Test Explorer Suite", function () {
["testPassing()", "testFailing()", "testDisabled()"],
"testWithKnownIssue()",
"testWithKnownIssueAndUnknownIssue()",
"testLotsOfOutput()",
"testAttachment()",
"DuplicateSuffixTests",
["testPassing()", "testPassingSuffix()"],
Expand Down Expand Up @@ -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))) {
Expand Down Expand Up @@ -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()"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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],
});
});
Expand Down
18 changes: 16 additions & 2 deletions test/integration-tests/testexplorer/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
}

/**
Expand Down