Skip to content
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

Add support for swift-testing #775

Merged
merged 20 commits into from
May 13, 2024

Conversation

plemarquand
Copy link
Contributor

@plemarquand plemarquand commented May 1, 2024

Support running swift-testing tests the same as XCTests. If a test run has both types, swift-testing tests will be run first followed by XCTests.

First a list of XCTests and swift-testing tests to run is parsed from the test request. Test type is determined by a tag on each vscode.TestItem[], either "XCTest" or "swift-testing".

swift-testing tests are launched by running the binary named PackageTests.swift-testing inside the build debug folder. This binary is run with the --experimental-event-stream-output flag which forwards test events (test started, complete, issue recorded, etc) to a named pipe. The SwiftTestingOutputParser watches this pipe for events as the tests are being run and translates them in to ITestRunner calls to record test progress in VSCode.

There are different named pipe reader implementations between macOS/Linux and Windows.

@plemarquand plemarquand marked this pull request as draft May 1, 2024 13:32
@plemarquand plemarquand force-pushed the swift-testing-support branch from 9266bee to 5e4ad57 Compare May 3, 2024 11:29
Copy link
Contributor

@adam-fowler adam-fowler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some initial comments


return testBuildConfig;
} else {
const testBuildConfig = createSwiftTestConfiguration(folderContext, fifoPipePath, true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mkfifo isn't called here. Is it cleaned up anywhere? You could use TemporaryFolder.withTemporaryFile to create a temporary file name, which will get deleted once you return found the supplied closure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up just rming the pipe at the end of the test execution because TemporaryFolder.withTemporaryFile's API didn't fit quite right for windows pipes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah there are cases where you exit the function before the fifo is deleted (if cancellation has been requested). Also some of those functions called might throw errors. I'd rather we did something safer with a similar style to withTemporaryFile.

// do platform specific setup of fifo and run function and then remove fifo
withTemporaryFifo(fifoPath => {
    const testBuildConfig =
                await LaunchConfigurations.createLaunchConfigurationForSwiftTesting(
    etc
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed—that would help encapsulate the platform-specific FIFO code too.

@plemarquand plemarquand force-pushed the swift-testing-support branch from 86fc01b to d4dd446 Compare May 4, 2024 15:18
Copy link
Contributor

@adam-fowler adam-fowler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its pretty much there, a few more comments
I haven't looked at the tests in any detail yet

parsedOutputStream,
outputStream,
testBuildConfig
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both parallel and coverage sessions don't seem to run swift testing tests

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will come in a subsequent patch as there are some changes in SwiftPM required to support our coverage workflow. Specifically we need the --enable-code-coverage flag to be added to swift build (see swiftlang/swift-package-manager#7518).

I have the code written, I just need to test the SwiftPM update with a nightly toolchain, which I should be able to do shortly.

@plemarquand plemarquand marked this pull request as ready for review May 10, 2024 23:28
Support running swift-testing tests the same as XCTests. If a test run
has both types, swift-testing tests will be run first followed by
XCTests.

First a list of XCTests and swift-testing tests to run is parsed from
the test request. Test type is determined by a tag on each
`vscode.TestItem[]`, either `"XCTest"` or `"swift-testing"`.

swift-testing tests are launched by running the binary named
<PackageName>PackageTests.swift-testing inside the build debug folder.
This binary is run with the `--experimental-event-stream-output` flag
which forwards test events (test started, complete, issue recorded, etc)
to a named pipe. The `SwiftTestingOutputParser` watches this pipe for
events as the tests are being run and translates them in to
`ITestRunner` calls to record test progress in VSCode.

There are different named pipe reader implementations between
macOS/Linux and Windows.

TODO: Coverage on swift-testing tests is not supported until
swiftlang/swift-package-manager#7518 is available.
Also define a stable sort order on TestItems
@plemarquand plemarquand force-pushed the swift-testing-support branch from ccd4238 to de1397c Compare May 12, 2024 21:25
Copy link
Contributor

@adam-fowler adam-fowler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are good.
I see this is no longer a draft, has the related swift-testing PR been merged

@plemarquand
Copy link
Contributor Author

@adam-fowler The related swift-testing changes are merged. If you're trying this out the swift-testing used must be the main branch until their next tagged release (> 0.8).

@plemarquand plemarquand merged commit 18201e5 into swiftlang:main May 13, 2024
6 checks passed
@plemarquand plemarquand deleted the swift-testing-support branch May 13, 2024 12:13
@grynspan
Copy link

I can tag 0.9 any time, although I've been trying to maintain a beginning-of-the-month cadence.

@adam-fowler
Copy link
Contributor

@grynspan We have a few other things to finish before doing the next release of the extension, so you can probably hold off for a two weeks or so before doing a swift-testing release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants