Skip to content

Conversation

@DivineDominion
Copy link
Contributor

The compiler warning I got was:

StoreTests.swift:125:15: warning: '#expect(_:_:)' will always fail here; use 'Bool(false)' to silence this warning (from macro 'expect')

Curiously, this was expressing "this part should not be reached" in a do-catch block. We have expect(throws:performing:) for that in Swift Testing to expresses the idea that the expected path is the error path. This PR suggests using that macro instead.

@Test func dispatchThunkThrowsError() async throws {
self.thunk.returnError = Error()

let error = Error()
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 went with a local constant that's known to be non-optional to make the expectation below more directly tell that we don't expect the error to be non-nil whenever the thunk's returnError is non-nil (for which the reader needs to find the conditions), but that it's always going to be a local constant set-up in this test case.

Copy link
Member

@vanvoorden vanvoorden left a comment

Choose a reason for hiding this comment

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

@DivineDominion Ahh… so this breaks tests from Xcode 16.0. Here is the issue:

swiftlang/swift#74882

If ImmutableData did drop support for Xcode 16.0… my preference would be to wait for a 1.0.0 release. That would probably be in Q3 when Swift 6.2 ships to production.

I totally appreciate the effort… but my preference for right now is not to land this diff. Sorry about that. Maybe the best idea for now is to just transition this diff to a draft.

@DivineDominion
Copy link
Contributor Author

Argh, that's sad. Well then, I'll close without merging. The Animals tests would've benefitted from something like this in the future, but there's no reason to leave this open and get stale.

@vanvoorden
Copy link
Member

@DivineDominion BTW you also have the option to enable actions for your own fork. If you fork the repo and push a commit to your fork you can choose to run the actions on that forked commit. This might help catch any build issues early.

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.

2 participants