-
Notifications
You must be signed in to change notification settings - Fork 124
Change the semantics of Test.cancel()
.
#1332
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
Conversation
This PR changes the semantics of `Test.cancel()` such that it only cancels the current test case if the current test is parameterized. This PR also removes `Test.Case.cancel()`, but we may opt to add `Test.Case.cancelAll()` or some such in the future to re-add an interface that cancels an entire parameterized test.
configuration.eventHandler = { event, eventContext in | ||
switch event.kind { | ||
case .issueRecorded, .valueAttached, .testCancelled, .testCaseCancelled: | ||
case .issueRecorded, .valueAttached, .testCancelled: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exit tests no longer need to be concerned with .testCaseCancelled
events.
static func withCurrent<R>(_ test: Self, perform body: () async throws -> R) async rethrows -> R { | ||
var runtimeState = Runner.RuntimeState.current ?? .init() | ||
runtimeState.test = test | ||
runtimeState.testCase = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change looks out-of-place, but it ensures that when we set the current test, we clear the current test case. This has no effect on third-party tests but ensures when we run nested tests of our own that they don't get their states intermingled (which breaks Test.cancel()
's new semantics.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. This is probably something we'd probably want even if we'd never sought to add test cancellation
This PR changes the semantics of
Test.cancel()
such that it only cancels the current test case if the current test is parameterized. This PR also removesTest.Case.cancel()
, but we may opt to addTest.Case.cancelAll()
or some such in the future to re-add an interface that cancels an entire parameterized test.Checklist: