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 stub for Testing module #5077

Merged
merged 2 commits into from
Aug 21, 2024
Merged

Conversation

jmschonfeld
Copy link
Contributor

Now that swift-testing is present in the toolchain, SwiftPM builds of the test target in this package will automatically import Testing, causing our test target to link the Testing library in the toolchain. This is problematic, because the Testing module links Foundation (from the toolchain) which results in a variety of issues and crashes when running our tests caused by the presence of two Foundations (classes of the same name are indistinguishable and one implementation is nondeterministically picked leading to failures when bridging). To resolve this, we will use the same solution we found for XCTest - to vendor a small copy of the library within our project that will be picked up instead of the toolchain version. In our case, we only need a small stub because we don't actually use the Testing module yet.

In the future, now that SwiftPM supports cycles in package dependencies as long as there is no target-level cycle, we could update the package build to use the package build of swift-testing (which we could have rely on the package build of Foundation thus ensuring only one Foundation is present) but for now this simple solution gets us building and testing again.

@jmschonfeld
Copy link
Contributor Author

@swift-ci please test

Copy link
Contributor

@grynspan grynspan left a comment

Choose a reason for hiding this comment

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

We hates it! :shipit:

@jmschonfeld
Copy link
Contributor Author

Confirmed that the tests still pass with this change, and they also pass with the linked change so this should resolve the issue that the linked PR was originally hitting when re-ordering the swift-testing build

@jmschonfeld
Copy link
Contributor Author

@swift-ci please test

@jmschonfeld
Copy link
Contributor Author

@swift-ci please test Windows platform

import Musl
#elseif os(WASI)
import WASILibc
#elseif canImport(CRT)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit I think we should might as well add #if canImport(Bionic) here as well. But def none blocking

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point - I'm not sure if we've tried running SwiftPM tests on android for this repo yet so I'll follow up with that later once we try to do that

Copy link
Member

Choose a reason for hiding this comment

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

Submitted in #5149

@jmschonfeld jmschonfeld merged commit 4418280 into swiftlang:main Aug 21, 2024
2 of 3 checks passed
@jmschonfeld jmschonfeld deleted the stub-swift-testing branch August 21, 2024 17:21
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