Skip to content

Tests: Fix always succeeding XCTAssertAsyncNoThrow #8026

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

Conversation

kateinoigakukun
Copy link
Member

@kateinoigakukun kateinoigakukun commented Oct 7, 2024

The autoclosure expression { throw error } just creates a closure that won't be executed. It leads to the test always succeeding even if the expression throws an error.

Fortunately, I didn't find any issue to enable the assertion except for a single case (ab1e784)

The autoclosure expression `{ throw error }` just creates a closure that
won't be executed. It leads to the test always succeeding even if the
`expression` throws an error.
@kateinoigakukun
Copy link
Member Author

@swift-ci test

@kateinoigakukun kateinoigakukun added bug test suite improvements to SwiftPM test suite labels Oct 7, 2024
@kateinoigakukun
Copy link
Member Author

@swift-ci test

@kateinoigakukun
Copy link
Member Author

@grynspan This assertion activation revealed that SWIFT_TESTING_ENABLED is set to "0" only on macOS and not set on other platforms. (introduced by this change)

Is this intentional behavior or should we set the env variable regardless of the target platform?

@grynspan
Copy link
Contributor

grynspan commented Oct 7, 2024

It has no effect on other platforms. We set it on macOS so that XCTest doesn't try to host Swift Testing from within Xcode, like it would if you were using Xcode instead of SwiftPM.

It's a bit of an inelegant hack and we may remove it in a future Xcode update.

`SWIFT_PM_TEST_LIBRARY` was renamed to `SWIFT_TESTING_ENABLED` in
swiftlang#7789, but we
forgot to update the test because `XCTAssertAsyncNoThrow` always
succeeded and we didn't notice the failure.
@kateinoigakukun kateinoigakukun force-pushed the yt/assert-async-no-throw branch from ab1e784 to c50518d Compare October 7, 2024 11:25
@kateinoigakukun
Copy link
Member Author

Ok, then skip the test on non-macOS platforms

@kateinoigakukun
Copy link
Member Author

@swift-ci test

@kateinoigakukun
Copy link
Member Author

@swift-ci test Windows

@MaxDesiatov MaxDesiatov enabled auto-merge (squash) October 7, 2024 15:04
@MaxDesiatov MaxDesiatov merged commit 58f9aa4 into swiftlang:main Oct 7, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug test suite improvements to SwiftPM test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants