Skip to content

Non-darwin test discovery with Swift Build#8722

Merged
owenv merged 1 commit into
swiftlang:mainfrom
owenv:owenv/test-discovery
Jun 24, 2025
Merged

Non-darwin test discovery with Swift Build#8722
owenv merged 1 commit into
swiftlang:mainfrom
owenv:owenv/test-discovery

Conversation

@owenv

@owenv owenv commented May 25, 2025

Copy link
Copy Markdown
Contributor

Still needs a fair amount of work and cleanup, I'll look at breaking off some parts that can land independently. Depends on the larger patch in swiftlang/swift-build#499

Comment thread Sources/SwiftBuildSupport/PIFBuilder.swift Outdated
Comment thread Sources/SwiftBuildSupport/PackagePIFBuilder.swift
Comment thread Sources/SwiftBuildSupport/PackagePIFProjectBuilder+Products.swift
Comment thread Sources/SwiftBuildSupport/PackagePIFProjectBuilder+Products.swift
Comment thread Sources/SwiftBuildSupport/PackagePIFProjectBuilder+Products.swift Outdated
@owenv

owenv commented May 25, 2025

Copy link
Copy Markdown
Contributor Author

Locally, I can pass a decent chunk of TestDiscoveryTests on linux. macOS will need a bit of work to skip building the test runner executables, and I expect Windows will need a few tweaks

@swift-ci test

Comment thread Sources/SwiftBuildSupport/PackagePIFBuilder.swift Outdated
@owenv owenv force-pushed the owenv/test-discovery branch from a18182a to 10bc51d Compare May 25, 2025 20:30
@owenv

owenv commented May 25, 2025

Copy link
Copy Markdown
Contributor Author

@owenv owenv marked this pull request as ready for review May 25, 2025 20:38
@owenv owenv changed the title WIP: Non-darwin test discovery with Swift Build Non-darwin test discovery with Swift Build May 25, 2025
@owenv owenv force-pushed the owenv/test-discovery branch from 10bc51d to 401a93f Compare June 4, 2025 18:20
@owenv

owenv commented Jun 4, 2025

Copy link
Copy Markdown
Contributor Author

@swift-ci test

@owenv owenv force-pushed the owenv/test-discovery branch from 401a93f to 3757526 Compare June 9, 2025 16:10
@owenv

owenv commented Jun 9, 2025

Copy link
Copy Markdown
Contributor Author

@swift-ci test

@bkhouri bkhouri left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for converting the TestDiscoverTests from XCTest to Swift Testing. I have a couple questions/comments, but nothing blocking.

Comment thread Tests/FunctionalTests/TestDiscoveryTests.swift Outdated
Comment thread Tests/FunctionalTests/TestDiscoveryTests.swift
Comment thread Tests/FunctionalTests/TestDiscoveryTests.swift
@owenv owenv force-pushed the owenv/test-discovery branch from 633a864 to 88ccf4a Compare June 10, 2025 18:13
@owenv

owenv commented Jun 10, 2025

Copy link
Copy Markdown
Contributor Author

@swift-ci test

@owenv owenv force-pushed the owenv/test-discovery branch from 88ccf4a to 23a8d43 Compare June 11, 2025 16:45
@owenv

owenv commented Jun 11, 2025

Copy link
Copy Markdown
Contributor Author

@swift-ci test

@owenv owenv force-pushed the owenv/test-discovery branch from 23a8d43 to 61c3ec0 Compare June 11, 2025 18:33
@owenv

owenv commented Jun 20, 2025

Copy link
Copy Markdown
Contributor Author

@swift-ci test

@owenv owenv force-pushed the owenv/test-discovery branch from 9cfbd82 to e63dc87 Compare June 20, 2025 19:27
@owenv

owenv commented Jun 20, 2025

Copy link
Copy Markdown
Contributor Author

@swift-ci test

@owenv owenv force-pushed the owenv/test-discovery branch from e63dc87 to 70dcd09 Compare June 20, 2025 20:51
Comment thread Tests/CommandsTests/PackageCommandTests.swift Outdated
Comment thread Tests/FunctionalTests/TestDiscoveryTests.swift Outdated

@bkhouri bkhouri left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I only reviewed the tests changes. Although the overcharge is good, we should ensure to run the TestDicoveryTests against SwiftBuild on Windows and use the withKnownIssue Swift Testing API to essentially mark the test as "expected fail". This will give us signal when the underlying Windows issue is fix, thus prompting the removal of the withKnownIssue as opposed to having to remember to augment the test.

Comment thread Sources/SPMBuildCore/BuildParameters/BuildParameters.swift
Comment thread Sources/SwiftBuildSupport/PackagePIFBuilder.swift Outdated
@owenv owenv force-pushed the owenv/test-discovery branch 3 times, most recently from 1b4de2a to e73ba32 Compare June 22, 2025 18:13
@owenv

owenv commented Jun 23, 2025

Copy link
Copy Markdown
Contributor Author

@swift-ci test

@owenv

owenv commented Jun 23, 2025

Copy link
Copy Markdown
Contributor Author

@swift-ci test Windows

@owenv

owenv commented Jun 23, 2025

Copy link
Copy Markdown
Contributor Author

Windows: 'TestCommandSwiftBuildTests.testListWithSkipBuildAndNoBuildArtifacts' failed (14.613 seconds)

@owenv owenv force-pushed the owenv/test-discovery branch from e73ba32 to a180198 Compare June 23, 2025 21:14
@owenv

owenv commented Jun 23, 2025

Copy link
Copy Markdown
Contributor Author

@swift-ci test

@owenv owenv force-pushed the owenv/test-discovery branch from a180198 to 96ab7a9 Compare June 23, 2025 21:56
@owenv

owenv commented Jun 23, 2025

Copy link
Copy Markdown
Contributor Author

@swift-ci test

@owenv owenv requested a review from bkhouri June 23, 2025 22:06
struct TestDiscoveryTests {
static var buildSystems: [BuildSystemProvider.Kind] = [BuildSystemProvider.Kind.native, .swiftbuild]

@Test(arguments: buildSystems)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion (non-blocking, if-pr-update): using arguments: SupportedBuildSystemOnAllPlatforms does essentially the same thing :)

}

func testNonStandardName() async throws {
@Test(.bug("https://github.com/swiftlang/swift-build/issues/13"), arguments: [BuildSystemProvider.Kind.native])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

question: out of curiosity, why aren't we executing this test against the SwiftBuild build system?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There are some design questions I need to sort out related to response file quoting on non-Darwin platforms and it's not 100% clear to me marking this as a known issue is the right thing to do yet

@bkhouri

bkhouri commented Jun 23, 2025

Copy link
Copy Markdown
Contributor

@swift-ci test windows

@owenv

owenv commented Jun 24, 2025

Copy link
Copy Markdown
Contributor Author

@swift-ci smoke test macOS

@owenv

owenv commented Jun 24, 2025

Copy link
Copy Markdown
Contributor Author

@swift-ci test macOS

@owenv

owenv commented Jun 24, 2025

Copy link
Copy Markdown
Contributor Author

macOS checkout issue
@swift-ci test macOS

@owenv owenv merged commit 3db835d into swiftlang:main Jun 24, 2025
6 checks passed
marcprux pushed a commit to swift-everywhere/swift-package-manager that referenced this pull request Jul 7, 2025
Still needs a fair amount of work and cleanup, I'll look at breaking off
some parts that can land independently. Depends on the larger patch in
swiftlang/swift-build#499
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.

4 participants