Skip to content

Invoke foundation tests via SwiftPM #74594

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
merged 2 commits into from
Jul 29, 2024

Conversation

jmschonfeld
Copy link
Contributor

This adds new phases to the build script that will build and run swift-foundation unit tests and swift-corelibs-foundation unit tests via swiftpm. On platforms that use the build-script, this will take place later than before after the SwiftPM build has finished. On platforms that use build.ps1 the tests will happen as they did previously but will build with SwiftPM instead. We set the SWIFTCI_USE_LOCAL_DEPS environment variable when building these projects so that they find dependencies in the local checkout rather than fetching them from GitHub.

@jmschonfeld
Copy link
Contributor Author

@swift-ci please smoke test

@jmschonfeld jmschonfeld force-pushed the foundation-test-as-package branch 2 times, most recently from 4d1d1c5 to c19939b Compare July 22, 2024 18:35
@jmschonfeld
Copy link
Contributor Author

@swift-ci please smoke test

@jmschonfeld jmschonfeld force-pushed the foundation-test-as-package branch from c19939b to ab7f7a4 Compare July 22, 2024 19:04
@jmschonfeld
Copy link
Contributor Author

@swift-ci please smoke test

@jmschonfeld
Copy link
Contributor Author

swiftlang/swift-foundation#756

@swift-ci please smoke test

@jmschonfeld jmschonfeld force-pushed the foundation-test-as-package branch from ab7f7a4 to 252e44f Compare July 23, 2024 23:04
@jmschonfeld
Copy link
Contributor Author

swiftlang/swift-foundation#761

@swift-ci please smoke test

@jmschonfeld jmschonfeld force-pushed the foundation-test-as-package branch from 252e44f to 0a941ba Compare July 24, 2024 16:23
@jmschonfeld
Copy link
Contributor Author

swiftlang/swift-corelibs-foundation#5025

@swift-ci please smoke test

@jmschonfeld
Copy link
Contributor Author

swiftlang/swift-corelibs-foundation#5025

@swift-ci please smoke test macOS platform

@jmschonfeld jmschonfeld marked this pull request as ready for review July 24, 2024 20:43
@jmschonfeld
Copy link
Contributor Author

This is now ready to go / ready for review! Just a note: this does not yet enable these tests on Windows yet, I'm still working through a few issues with getting those building correctly (the tests can be invoked but do not pass yet and so I've left them disabled in CI for now, but would like to add this ability so that it can be tested and iterated on locally with ease). In the linux CI run, you can see the tests for swift-foundation and swift-corelibs-foundation running and passing.

@jmschonfeld
Copy link
Contributor Author

@swift-ci please smoke test

@finagolfin
Copy link
Member

Glad to see the Foundation tests running on linux again, but if Foundation breaks functionality but builds, it could break SwiftPM too and then these tests can't be run by that broken SwiftPM to diagnose the issue.

Since you are now switching to SwiftPM to build and run the Foundation tests on linux and we already have prebuilt toolchains on all CI to build the Swift portions of the Swift compiler, whose prebuilt SwiftPM has some support for using different Swift compiler versions, have you considered using the prebuilt SwiftPM with the freshly-built Swift compiler to build and run these tests right away? I know that may be unusual and would limit the Foundation package manifest to the features of the older prebuilt SwiftPM, but it would allow running these tests right away, ie before freshly building trunk SwiftPM.

Pinging @neonichu and @bnbarham, who know SwiftPM better.

@parkera
Copy link
Contributor

parkera commented Jul 25, 2024

In that case, there would be some indication at test time that swiftpm failed to run the tests - a crash, for example, right?

@finagolfin
Copy link
Member

Many different possibilities, but there's a reason this swift-corelibs-foundation test run on the CI was careful to run the tests before SwiftPM, even though it had to build XCTest first then come back to run the swift-corelibs-foundation tests.

I wonder if we can keep that by leveraging the prebuilt release SwiftPM on the CI to invoke the freshly-built trunk Swift compiler and keep running the swift-corelibs-foundation tests right after building them, that's all.

@jmschonfeld
Copy link
Contributor Author

have you considered using the prebuilt SwiftPM with the freshly-built Swift compiler to build and run these tests right away?

I don't think this is a viable option (currently) for swift-foundation since the CI images have the 5.8 toolchain AFAIK and swift-foundation relies on Swift macros which are only supported in the SwiftPM tools packaged with swift 5.9+.

I see where you're coming from here but I'm also not sure it'll be too large of an issue in practice. We expect that at some point soon once we re-configure our CI, changes to swift-foundation/swift-corelibs-foundation will be able to run SwiftPM tests in CI directly using a nightly toolchain without requiring a full toolchain build (in addition to the option to run full toolchain tests). So ideally if something here breaks in a full toolchain test, we'd have the option to run the unit tests against the nightly swift toolchain to diagnose further in CI.

This change as it stands at least gets us closer to where we want to be re. testing Foundation instead of avoiding testing at all like we do today, and if in the future we have a pre-built toolchain of a compatible version and mixing-and-matching was supported fully on the SwiftPM side then we could investigate that as a future incremental followup, but IMO this is a nice first step that we can take.

@finagolfin
Copy link
Member

Sounds good, good point about SwiftPM 5.8.1 on the CI not supporting macros yet.

@finagolfin
Copy link
Member

@compnerd, if you would review, we can get these tests running on the CI again.

@jmschonfeld jmschonfeld merged commit 443c29d into swiftlang:main Jul 29, 2024
3 checks passed
@jmschonfeld jmschonfeld deleted the foundation-test-as-package branch July 29, 2024 23:38
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.

5 participants