From 180629b622209a3c49addab7709e4f21e2f062c9 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Fri, 13 Sep 2024 16:08:22 -0400 Subject: [PATCH 01/10] [SWT-NNNN] Range-based confirmations Swift Testing includes [an interface](https://swiftpackageindex.com/swiftlang/swift-testing/main/documentation/testing/confirmation(_:expectedcount:isolation:sourcelocation:_:)) for checking that some asynchronous event occurs a given number of times (typically exactly once or never at all.) This proposal enhances that interface to allow arbitrary ranges of event counts so that a test can be written against code that may not always fire said event the exact same number of times. Read the full proposal [here](). --- .../Proposals/NNNN-ranged-confirmations.md | 191 ++++++++++++++++++ Package.swift | 1 + Sources/Testing/Issues/Confirmation.swift | 25 +-- Sources/Testing/Issues/Issue.swift | 60 +++--- Sources/Testing/Testing.docc/Expectations.md | 3 +- .../Testing.docc/MigratingFromXCTest.md | 3 +- .../Testing.docc/testing-asynchronous-code.md | 31 ++- .../Support/DiagnosticMessage.swift | 26 +++ .../TestingMacros/TestDeclarationMacro.swift | 59 ++++++ .../TestDeclarationMacroTests.swift | 8 + Tests/TestingTests/ConfirmationTests.swift | 28 ++- Tests/TestingTests/IssueTests.swift | 1 - .../shared/AvailabilityDefinitions.cmake | 1 + 13 files changed, 358 insertions(+), 79 deletions(-) create mode 100644 Documentation/Proposals/NNNN-ranged-confirmations.md diff --git a/Documentation/Proposals/NNNN-ranged-confirmations.md b/Documentation/Proposals/NNNN-ranged-confirmations.md new file mode 100644 index 000000000..115e1237f --- /dev/null +++ b/Documentation/Proposals/NNNN-ranged-confirmations.md @@ -0,0 +1,191 @@ +# Range-based confirmations + +* Proposal: [SWT-NNNN](NNNN-ranged-confirmations.md) +* Authors: [Jonathan Grynspan](https://github.com/grynspan) +* Status: **Awaiting review** +* Implementation: [swiftlang/swift-testing#598](https://github.com/swiftlang/swift-testing/pull/598), [swiftlang/swift-testing#689](https://github.com/swiftlang/swift-testing/pull689) +* Review: TBD + +## Introduction + +Swift Testing includes [an interface](https://swiftpackageindex.com/swiftlang/swift-testing/main/documentation/testing/confirmation(_:expectedcount:isolation:sourcelocation:_:)) +for checking that some asynchronous event occurs a given number of times +(typically exactly once or never at all.) This proposal enhances that interface +to allow arbitrary ranges of event counts so that a test can be written against +code that may not always fire said event the exact same number of times. + +## Motivation + +Some tests rely on fixtures or external state that is not perfectly +deterministic. For example, consider a test that checks that clicking the mouse +button will generate a `.mouseClicked` event. Such a test might use the +`confirmation()` interface: + +```swift +await confirmation(expectedCount: 1) { mouseClicked in + var eventLoop = EventLoop() + eventLoop.eventHandler = { event in + if event == .mouseClicked { + mouseClicked() + } + } + await eventLoop.simulate(.mouseClicked) +} +``` + +But what happens if the user _actually_ clicks a mouse button while this test is +running? That might trigger a _second_ `.mouseClicked` event, and then the test +will fail spuriously. + +## Proposed solution + +If the test author could instead indicate to Swift Testing that their test will +generate _one or more_ events, they could avoid spurious failures: + +```swift +await confirmation(expectedCount: 1...) { mouseClicked in + ... +} +``` + +With this proposal, we add an overload of `confirmation()` that takes any range +expression instead of a single integer value (which is still accepted via the +existing overload.) + +## Detailed design + +A new overload of `confirmation()` is added: + +```swift +/// Confirm that some event occurs during the invocation of a function. +/// +/// - Parameters: +/// - comment: An optional comment to apply to any issues generated by this +/// function. +/// - expectedCount: A range of integers indicating the number of times the +/// expected event should occur when `body` is invoked. +/// - isolation: The actor to which `body` is isolated, if any. +/// - sourceLocation: The source location to which any recorded issues should +/// be attributed. +/// - body: The function to invoke. +/// +/// - Returns: Whatever is returned by `body`. +/// +/// - Throws: Whatever is thrown by `body`. +/// +/// Use confirmations to check that an event occurs while a test is running in +/// complex scenarios where `#expect()` and `#require()` are insufficient. For +/// example, a confirmation may be useful when an expected event occurs: +/// +/// - In a context that cannot be awaited by the calling function such as an +/// event handler or delegate callback; +/// - More than once, or never; or +/// - As a callback that is invoked as part of a larger operation. +/// +/// To use a confirmation, pass a closure containing the work to be performed. +/// The testing library will then pass an instance of ``Confirmation`` to the +/// closure. Every time the event in question occurs, the closure should call +/// the confirmation: +/// +/// ```swift +/// let minBuns = 5 +/// let maxBuns = 10 +/// await confirmation( +/// "Baked between \(minBuns) and \(maxBuns) buns", +/// expectedCount: minBuns ... maxBuns +/// ) { bunBaked in +/// foodTruck.eventHandler = { event in +/// if event == .baked(.cinnamonBun) { +/// bunBaked() +/// } +/// } +/// await foodTruck.bakeTray(of: .cinnamonBun) +/// } +/// ``` +/// +/// When the closure returns, the testing library checks if the confirmation's +/// preconditions have been met, and records an issue if they have not. +/// +/// If an exact count is expected, use +/// ``confirmation(_:expectedCount:isolation:sourceLocation:_:)`` instead. +public func confirmation( + _ comment: Comment? = nil, + expectedCount: some RangeExpression & Sendable, + isolation: isolated (any Actor)? = #isolation, + sourceLocation: SourceLocation = #_sourceLocation, + _ body: (Confirmation) async throws -> sending R +) async rethrows -> R +``` + +### Ranges without lower bounds + +Certain types of range, specifically [`PartialRangeUpTo`](https://developer.apple.com/documentation/swift/partialrangeupto) +and [`PartialRangeThrough`](https://developer.apple.com/documentation/swift/partialrangethrough), +are valid when used with this new interface, but may have surprising behavior +because they implicitly include `0`. If a test author writes `...10`, do they +mean "zero to ten" or "one to ten"? The programmatic meaning is the former, but +some test authors might mean the latter. If an event does not occur, a test +using `confirmation()` and this `expectedCount` value would pass when the test +author meant for it to fail. + +Swift Testing will attempt to detect these ambiguous uses of `...n` and `..( + _ comment: Comment? = nil, + expectedCount: UnboundedRange, + isolation: isolated (any Actor)? = #isolation, + sourceLocation: SourceLocation = #_sourceLocation, + _ body: (Confirmation) async throws -> R +) async rethrows -> R { + fatalError("Unsupported") +} +``` + +## Source compatibility + +This change is additive. Existing tests are unaffected. + +Code that refers to `confirmation(_:expectedCount:isolation:sourceLocation:_:)` +by symbol name may need to add a contextual type to disambiguate the two +overloads at compile time. + +## Integration with supporting tools + +The type of the associated value `expected` for the `Issue.Kind` case +`confirmationMiscounted(actual:expected:)` will change from `Int` to +`any RangeExpression & Sendable`[^1]. Tools that implement event handlers and +distinguish between `Issue.Kind` cases are advised not to assume the type of +this value is `Int`. + +## Alternatives considered + +- Doing nothing. We have identified real-world use cases for this interface + including in Swift Testing’s own test target. +- Allowing the use of any value as the `expectedCount` argument so long as it + conforms to a protocol `ExpectedCount` (we'd have range types and `Int` + conform by default.) It was unclear what this sort of flexibility would let + us do, and posed challenges for encoding and decoding events and issues when + using the JSON event stream interface. + +## Acknowledgments + +If significant changes or improvements suggested by members of the community +were incorporated into the proposal as it developed, take a moment here to thank +them for their contributions. This is a collaborative process, and everyone's +input should receive recognition! + +Generally, you should not acknowledge anyone who is listed as a co-author. + +[^1]: In the future, this type will change to + `any RangeExpression & Sendable`. Compiler support is required + ([96960993](rdar://96960993)). diff --git a/Package.swift b/Package.swift index f7fb5dc5f..8fdbd5585 100644 --- a/Package.swift +++ b/Package.swift @@ -150,6 +150,7 @@ extension Array where Element == PackageDescription.SwiftSetting { .enableExperimentalFeature("AvailabilityMacro=_clockAPI:macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0"), .enableExperimentalFeature("AvailabilityMacro=_regexAPI:macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0"), .enableExperimentalFeature("AvailabilityMacro=_swiftVersionAPI:macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0"), + .enableExperimentalFeature("AvailabilityMacro=_parameterizedProtocolsAPI:macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0"), .enableExperimentalFeature("AvailabilityMacro=_typedThrowsAPI:macOS 15.0, iOS 18.0, watchOS 11.0, tvOS 18.0, visionOS 2.0"), .enableExperimentalFeature("AvailabilityMacro=_distantFuture:macOS 99.0, iOS 99.0, watchOS 99.0, tvOS 99.0, visionOS 99.0"), diff --git a/Sources/Testing/Issues/Confirmation.swift b/Sources/Testing/Issues/Confirmation.swift index 48124c56e..b7dfd3646 100644 --- a/Sources/Testing/Issues/Confirmation.swift +++ b/Sources/Testing/Issues/Confirmation.swift @@ -161,7 +161,6 @@ public func confirmation( /// /// If an exact count is expected, use /// ``confirmation(_:expectedCount:isolation:sourceLocation:_:)`` instead. -@_spi(Experimental) public func confirmation( _ comment: Comment? = nil, expectedCount: some RangeExpression & Sendable, @@ -174,7 +173,7 @@ public func confirmation( let actualCount = confirmation.count.rawValue if !expectedCount.contains(actualCount) { let issue = Issue( - kind: expectedCount.issueKind(forActualCount: actualCount), + kind: .confirmationMiscounted(actual: actualCount, expected: expectedCount), comments: Array(comment), sourceContext: .init(backtrace: .current(), sourceLocation: sourceLocation) ) @@ -184,7 +183,7 @@ public func confirmation( return try await body(confirmation) } -/// An overload of ``confirmation(_:expectedCount:sourceLocation:_:)-9bfdc`` +/// An overload of ``confirmation(_:expectedCount:isolation:sourceLocation:_:)-6bkl6`` /// that handles the unbounded range operator (`...`). /// /// This overload is necessary because `UnboundedRange` does not conform to @@ -194,27 +193,9 @@ public func confirmation( public func confirmation( _ comment: Comment? = nil, expectedCount: UnboundedRange, + isolation: isolated (any Actor)? = #isolation, sourceLocation: SourceLocation = #_sourceLocation, _ body: (Confirmation) async throws -> R ) async rethrows -> R { fatalError("Unsupported") } - -extension RangeExpression where Bound == Int, Self: Sendable { - /// Get an instance of ``Issue/Kind-swift.enum`` corresponding to this value. - /// - /// - Parameters: - /// - actualCount: The actual count for the failed confirmation. - /// - /// - Returns: An instance of ``Issue/Kind-swift.enum`` that describes `self`. - fileprivate func issueKind(forActualCount actualCount: Int) -> Issue.Kind { - switch self { - case let expectedCount as ClosedRange where expectedCount.lowerBound == expectedCount.upperBound: - return .confirmationMiscounted(actual: actualCount, expected: expectedCount.lowerBound) - case let expectedCount as Range where expectedCount.lowerBound == expectedCount.upperBound - 1: - return .confirmationMiscounted(actual: actualCount, expected: expectedCount.lowerBound) - default: - return .confirmationOutOfRange(actual: actualCount, expected: self) - } - } -} diff --git a/Sources/Testing/Issues/Issue.swift b/Sources/Testing/Issues/Issue.swift index fe3130567..18d40e53e 100644 --- a/Sources/Testing/Issues/Issue.swift +++ b/Sources/Testing/Issues/Issue.swift @@ -32,27 +32,11 @@ public struct Issue: Sendable { /// - expected: The expected number of times /// ``Confirmation/confirm(count:)`` should have been called. /// - /// This issue can occur when calling - /// ``confirmation(_:expectedCount:isolation:sourceLocation:_:)`` when the - /// confirmation passed to these functions' `body` closures is confirmed too - /// few or too many times. - indirect case confirmationMiscounted(actual: Int, expected: Int) - - /// An issue due to a confirmation being confirmed the wrong number of - /// times. - /// - /// - Parameters: - /// - actual: The number of times ``Confirmation/confirm(count:)`` was - /// actually called. - /// - expected: The expected number of times - /// ``Confirmation/confirm(count:)`` should have been called. - /// - /// This issue can occur when calling - /// ``confirmation(_:expectedCount:isolation:sourceLocation:_:)-9rt6m`` when - /// the confirmation passed to these functions' `body` closures is confirmed - /// too few or too many times. - @_spi(Experimental) - indirect case confirmationOutOfRange(actual: Int, expected: any RangeExpression & Sendable) + /// This issue can occur when calling ``confirmation(_:expectedCount:isolation:sourceLocation:_:)-5mqz2`` + /// or ``confirmation(_:expectedCount:isolation:sourceLocation:_:)-6bkl6`` + /// when the confirmation passed to these functions' `body` closures is + /// confirmed too few or too many times. + indirect case confirmationMiscounted(actual: Int, expected: any RangeExpression & Sendable) /// An issue due to an `Error` being thrown by a test function and caught by /// the testing library. @@ -193,9 +177,9 @@ extension Issue.Kind: CustomStringConvertible { // Although the failure is unconditional at the point it is recorded, the // code that recorded the issue may not be unconditionally executing, so // we shouldn't describe it as unconditional (we just don't know!) - "Issue recorded" + return "Issue recorded" case let .expectationFailed(expectation): - if let mismatchedErrorDescription = expectation.mismatchedErrorDescription { + return if let mismatchedErrorDescription = expectation.mismatchedErrorDescription { "Expectation failed: \(mismatchedErrorDescription)" } else if let mismatchedExitConditionDescription = expectation.mismatchedExitConditionDescription { "Expectation failed: \(mismatchedExitConditionDescription)" @@ -203,19 +187,23 @@ extension Issue.Kind: CustomStringConvertible { "Expectation failed: \(expectation.evaluatedExpression.expandedDescription())" } case let .confirmationMiscounted(actual: actual, expected: expected): - "Confirmation was confirmed \(actual.counting("time")), but expected to be confirmed \(expected.counting("time"))" - case let .confirmationOutOfRange(actual: actual, expected: expected): - "Confirmation was confirmed \(actual.counting("time")), but expected to be confirmed \(String(describingForTest: expected)) time(s)" + if #available(_parameterizedProtocolsAPI, *), let expected = expected as? any RangeExpression { + let expected = expected.relative(to: []) + if expected.upperBound > expected.lowerBound && expected.lowerBound == expected.upperBound - 1 { + return "Confirmation was confirmed \(actual.counting("time")), but expected to be confirmed \(expected.lowerBound.counting("time"))" + } + } + return "Confirmation was confirmed \(actual.counting("time")), but expected to be confirmed \(String(describingForTest: expected)) time(s)" case let .errorCaught(error): - "Caught error: \(error)" + return "Caught error: \(error)" case let .timeLimitExceeded(timeLimitComponents: timeLimitComponents): - "Time limit was exceeded: \(TimeValue(timeLimitComponents))" + return "Time limit was exceeded: \(TimeValue(timeLimitComponents))" case .knownIssueNotRecorded: - "Known issue was not recorded" + return "Known issue was not recorded" case .apiMisused: - "An API was misused" + return "An API was misused" case .system: - "A system failure occurred" + return "A system failure occurred" } } } @@ -246,7 +234,7 @@ extension Issue { /// /// - Parameter issue: The original issue that gets snapshotted. public init(snapshotting issue: borrowing Issue) { - if case .confirmationOutOfRange = issue.kind { + if case .confirmationMiscounted = issue.kind { // Work around poor stringification of this issue kind in Xcode 16. self.kind = .unconditional self.comments = CollectionOfOne("\(issue.kind)") + issue.comments @@ -306,7 +294,7 @@ extension Issue.Kind { /// ``Confirmation/confirm(count:)`` should have been called. /// /// This issue can occur when calling - /// ``confirmation(_:expectedCount:sourceLocation:_:)`` when the + /// ``confirmation(_:expectedCount:isolation:sourceLocation:_:)`` when the /// confirmation passed to these functions' `body` closures is confirmed too /// few or too many times. indirect case confirmationMiscounted(actual: Int, expected: Int) @@ -349,10 +337,8 @@ extension Issue.Kind { .unconditional case let .expectationFailed(expectation): .expectationFailed(Expectation.Snapshot(snapshotting: expectation)) - case let .confirmationMiscounted(actual: actual, expected: expected): - .confirmationMiscounted(actual: actual, expected: expected) - case let .confirmationOutOfRange(actual: actual, expected: _): - .confirmationMiscounted(actual: actual, expected: 0) + case .confirmationMiscounted: + .unconditional case let .errorCaught(error): .errorCaught(ErrorSnapshot(snapshotting: error)) case let .timeLimitExceeded(timeLimitComponents: timeLimitComponents): diff --git a/Sources/Testing/Testing.docc/Expectations.md b/Sources/Testing/Testing.docc/Expectations.md index 083e09c64..331150730 100644 --- a/Sources/Testing/Testing.docc/Expectations.md +++ b/Sources/Testing/Testing.docc/Expectations.md @@ -75,7 +75,8 @@ the test when the code doesn't satisfy a requirement, use ### Confirming that asynchronous events occur - -- ``confirmation(_:expectedCount:isolation:sourceLocation:_:)`` +- ``confirmation(_:expectedCount:isolation:sourceLocation:_:)-5mqz2`` +- ``confirmation(_:expectedCount:isolation:sourceLocation:_:)-6bkl6`` - ``Confirmation`` ### Retrieving information about checked expectations diff --git a/Sources/Testing/Testing.docc/MigratingFromXCTest.md b/Sources/Testing/Testing.docc/MigratingFromXCTest.md index 529c56595..6b63ddf10 100644 --- a/Sources/Testing/Testing.docc/MigratingFromXCTest.md +++ b/Sources/Testing/Testing.docc/MigratingFromXCTest.md @@ -428,7 +428,8 @@ Some tests, especially those that test asynchronously-delivered events, cannot be readily converted to use Swift concurrency. The testing library offers functionality called _confirmations_ which can be used to implement these tests. Instances of ``Confirmation`` are created and used within the scope of the -function ``confirmation(_:expectedCount:isolation:sourceLocation:_:)``. +functions ``confirmation(_:expectedCount:isolation:sourceLocation:_:)-5mqz2`` +and ``confirmation(_:expectedCount:isolation:sourceLocation:_:)-6bkl6``. Confirmations function similarly to the expectations API of XCTest, however, they don't block or suspend the caller while waiting for a condition to be fulfilled. diff --git a/Sources/Testing/Testing.docc/testing-asynchronous-code.md b/Sources/Testing/Testing.docc/testing-asynchronous-code.md index 548cf07b0..964eb515a 100644 --- a/Sources/Testing/Testing.docc/testing-asynchronous-code.md +++ b/Sources/Testing/Testing.docc/testing-asynchronous-code.md @@ -31,7 +31,7 @@ expected event happens. ### Confirm that an event happens -Call ``confirmation(_:expectedCount:isolation:sourceLocation:_:)`` in your +Call ``confirmation(_:expectedCount:isolation:sourceLocation:_:)-5mqz2`` in your asynchronous test function to create a `Confirmation` for the expected event. In the trailing closure parameter, call the code under test. Swift Testing passes a `Confirmation` as the parameter to the closure, which you call as a function in @@ -54,6 +54,35 @@ If you expect the event to happen more than once, set the test passes if the number of occurrences during the test matches the expected count, and fails otherwise. +You can also pass a range to ``confirmation(_:expectedCount:isolation:sourceLocation:_:)-6bkl6`` +if the exact number of times the event occurs may change over time or is random: + +```swift +@Test("Customers bought sandwiches") +func boughtSandwiches() async { + await confirmation(expectedCount: 0 ..< 1000) { boughtSandwich in + var foodTruck = FoodTruck() + foodTruck.orderHandler = { order in + if order.contains(.sandwich) { + boughtSandwich() + } + } + await FoodTruck.operate() + } +} +``` + +In this example, there may be zero customers or up to (but not including) 1,000 +customers who order sandwiches. Any [range expression](https://developer.apple.com/documentation/swift/rangeexpression) +can be used: + +| Range Expression | Usage | +|-|-| +| `1...` | If an event must occur _at least_ once | +| `5...` | If an event must occur _at least_ five times | +| `1 ... 5` | If an event must occur at least once, but not more than five times | +| `0 ..< 100` | If an event may or may not occur, but _must not_ occur more than 99 times | + ### Confirm that an event doesn't happen To validate that a particular event doesn't occur during a test, diff --git a/Sources/TestingMacros/Support/DiagnosticMessage.swift b/Sources/TestingMacros/Support/DiagnosticMessage.swift index 7d8a83c20..15681857a 100644 --- a/Sources/TestingMacros/Support/DiagnosticMessage.swift +++ b/Sources/TestingMacros/Support/DiagnosticMessage.swift @@ -710,6 +710,32 @@ struct DiagnosticMessage: SwiftDiagnostics.DiagnosticMessage { ) } + /// Create a diagnostic messages stating that a range expression constructed + /// with a prefix operator (`...n` or `.. Self { + var exprCopy = expr + exprCopy.operator.trailingTrivia = .space + let lowerBoundPlaceholderExpr = EditorPlaceholderExprSyntax(type: "Int") + let replacementExpr: ExprSyntax = "\(lowerBoundPlaceholderExpr) \(exprCopy)" + + return Self( + syntax: Syntax(expr), + message: "Range expression '\(expr.trimmed)' is ambiguous without an explicit lower bound", + severity: .warning, + fixIts: [ + FixIt( + message: MacroExpansionFixItMessage("Add lower bound"), + changes: [.replace(oldNode: Syntax(expr), newNode: Syntax(replacementExpr)),] + ), + ] + ) + } + /// Create a diagnostic message stating that a condition macro nested inside /// an exit test will not record any diagnostics. /// diff --git a/Sources/TestingMacros/TestDeclarationMacro.swift b/Sources/TestingMacros/TestDeclarationMacro.swift index c98373469..012835a0c 100644 --- a/Sources/TestingMacros/TestDeclarationMacro.swift +++ b/Sources/TestingMacros/TestDeclarationMacro.swift @@ -35,6 +35,54 @@ public struct TestDeclarationMacro: PeerMacro, Sendable { .disabled } + /// A syntax visitor that looks for uses of `...n` and `.. PrefixOperatorExprSyntax? { + if let argumentExpr = removeParentheses(from: argumentExpr) { + return _ambiguousArgumentExpr(argumentExpr) + } + + guard let argumentExpr = argumentExpr.as(PrefixOperatorExprSyntax.self) else { + return nil + } + + let operatorTokenKind = argumentExpr.operator.tokenKind + if operatorTokenKind == .prefixOperator("...") || operatorTokenKind == .prefixOperator("..<") { + return argumentExpr + } + return nil + } + + override func visit(_ node: FunctionCallExprSyntax) -> SyntaxVisitorContinueKind { + let calledExpr = node.calledExpression.tokens(viewMode: .fixedUp).map(\.textWithoutBackticks).joined() + guard calledExpr == "confirmation" || calledExpr == "Testing.confirmation" else { + return .visitChildren + } + + let expectedCountArgument = node.arguments.first { $0.label?.tokenKind == .identifier("expectedCount") } + guard let argumentExpr = expectedCountArgument?.expression else { + return .visitChildren + } + + if let argumentExpr = _ambiguousArgumentExpr(argumentExpr) { + ambiguousArgumentExprs.append(argumentExpr) + } + return .visitChildren + } + } + /// Diagnose issues with a `@Test` declaration. /// /// - Parameters: @@ -118,6 +166,17 @@ public struct TestDeclarationMacro: PeerMacro, Sendable { } } + // Look for calls to confirmation() with a ..:-Xfrontend -define-availability -Xfrontend \"_clockAPI:macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0\">" "SHELL:$<$:-Xfrontend -define-availability -Xfrontend \"_regexAPI:macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0\">" "SHELL:$<$:-Xfrontend -define-availability -Xfrontend \"_swiftVersionAPI:macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0\">" + "SHELL:$<$:-Xfrontend -define-availability -Xfrontend \"_parameterizedProtocolsAPI:macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0\">" "SHELL:$<$:-Xfrontend -define-availability -Xfrontend \"_typedThrowsAPI:macOS 15.0, iOS 18.0, watchOS 11.0, tvOS 18.0, visionOS 2.0\">" "SHELL:$<$:-Xfrontend -define-availability -Xfrontend \"_distantFuture:macOS 99.0, iOS 99.0, watchOS 99.0, tvOS 99.0, visionOS 99.0\">") From fa2cf2fd3a268a37c427dbdad53bf5e7841f0f67 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Fri, 13 Sep 2024 16:20:01 -0400 Subject: [PATCH 02/10] Add pitch link --- Documentation/Proposals/NNNN-ranged-confirmations.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/Proposals/NNNN-ranged-confirmations.md b/Documentation/Proposals/NNNN-ranged-confirmations.md index 115e1237f..7601f6568 100644 --- a/Documentation/Proposals/NNNN-ranged-confirmations.md +++ b/Documentation/Proposals/NNNN-ranged-confirmations.md @@ -4,7 +4,7 @@ * Authors: [Jonathan Grynspan](https://github.com/grynspan) * Status: **Awaiting review** * Implementation: [swiftlang/swift-testing#598](https://github.com/swiftlang/swift-testing/pull/598), [swiftlang/swift-testing#689](https://github.com/swiftlang/swift-testing/pull689) -* Review: TBD +* Review: ([pitch](https://forums.swift.org/t/pitch-range-based-confirmations/74589)) ## Introduction From f5f6edf01ced677a88a35aa25d4f93f34186a5b5 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 21 Oct 2024 15:02:41 -0400 Subject: [PATCH 03/10] Require Sequence conformance too --- .../Proposals/NNNN-ranged-confirmations.md | 38 ++++++------ Sources/Testing/Issues/Confirmation.swift | 34 ++++++++++- .../Support/DiagnosticMessage.swift | 26 -------- .../TestingMacros/TestDeclarationMacro.swift | 59 ------------------- .../TestDeclarationMacroTests.swift | 8 --- Tests/TestingTests/ConfirmationTests.swift | 6 +- 6 files changed, 52 insertions(+), 119 deletions(-) diff --git a/Documentation/Proposals/NNNN-ranged-confirmations.md b/Documentation/Proposals/NNNN-ranged-confirmations.md index 7601f6568..bb5198496 100644 --- a/Documentation/Proposals/NNNN-ranged-confirmations.md +++ b/Documentation/Proposals/NNNN-ranged-confirmations.md @@ -110,7 +110,7 @@ A new overload of `confirmation()` is added: /// ``confirmation(_:expectedCount:isolation:sourceLocation:_:)`` instead. public func confirmation( _ comment: Comment? = nil, - expectedCount: some RangeExpression & Sendable, + expectedCount: some RangeExpression & Sequence Sendable, isolation: isolated (any Actor)? = #isolation, sourceLocation: SourceLocation = #_sourceLocation, _ body: (Confirmation) async throws -> sending R @@ -121,22 +121,22 @@ public func confirmation( Certain types of range, specifically [`PartialRangeUpTo`](https://developer.apple.com/documentation/swift/partialrangeupto) and [`PartialRangeThrough`](https://developer.apple.com/documentation/swift/partialrangethrough), -are valid when used with this new interface, but may have surprising behavior -because they implicitly include `0`. If a test author writes `...10`, do they -mean "zero to ten" or "one to ten"? The programmatic meaning is the former, but -some test authors might mean the latter. If an event does not occur, a test -using `confirmation()` and this `expectedCount` value would pass when the test -author meant for it to fail. - -Swift Testing will attempt to detect these ambiguous uses of `...n` and `..( isolation: isolated (any Actor)? = #isolation, sourceLocation: SourceLocation = #_sourceLocation, _ body: (Confirmation) async throws -> R -) async rethrows -> R { - fatalError("Unsupported") -} +) async rethrows -> R ``` ## Source compatibility diff --git a/Sources/Testing/Issues/Confirmation.swift b/Sources/Testing/Issues/Confirmation.swift index b7dfd3646..743f3ad45 100644 --- a/Sources/Testing/Issues/Confirmation.swift +++ b/Sources/Testing/Issues/Confirmation.swift @@ -163,7 +163,7 @@ public func confirmation( /// ``confirmation(_:expectedCount:isolation:sourceLocation:_:)`` instead. public func confirmation( _ comment: Comment? = nil, - expectedCount: some RangeExpression & Sendable, + expectedCount: some RangeExpression & Sequence & Sendable, isolation: isolated (any Actor)? = #isolation, sourceLocation: SourceLocation = #_sourceLocation, _ body: (Confirmation) async throws -> sending R @@ -199,3 +199,35 @@ public func confirmation( ) async rethrows -> R { fatalError("Unsupported") } + +/// An overload of ``confirmation(_:expectedCount:isolation:sourceLocation:_:)-6bkl6`` +/// that handles the unbounded range operator (`...`). +/// +/// This overload is necessary because the lower bound of `PartialRangeThrough` +/// is ambiguous: does it start at `0` or `1`? Test authors should specify a +@available(*, unavailable, message: "Range expression '...n' is ambiguous without an explicit lower bound") +public func confirmation( + _ comment: Comment? = nil, + expectedCount: PartialRangeThrough, + isolation: isolated (any Actor)? = #isolation, + sourceLocation: SourceLocation = #_sourceLocation, + _ body: (Confirmation) async throws -> R +) async rethrows -> R { + fatalError("Unsupported") +} + +/// An overload of ``confirmation(_:expectedCount:isolation:sourceLocation:_:)-6bkl6`` +/// that handles the unbounded range operator (`...`). +/// +/// This overload is necessary because the lower bound of `PartialRangeUpTo` is +/// ambiguous: does it start at `0` or `1`? Test authors should specify a +@available(*, unavailable, message: "Range expression '..( + _ comment: Comment? = nil, + expectedCount: PartialRangeUpTo, + isolation: isolated (any Actor)? = #isolation, + sourceLocation: SourceLocation = #_sourceLocation, + _ body: (Confirmation) async throws -> R +) async rethrows -> R { + fatalError("Unsupported") +} diff --git a/Sources/TestingMacros/Support/DiagnosticMessage.swift b/Sources/TestingMacros/Support/DiagnosticMessage.swift index 15681857a..7d8a83c20 100644 --- a/Sources/TestingMacros/Support/DiagnosticMessage.swift +++ b/Sources/TestingMacros/Support/DiagnosticMessage.swift @@ -710,32 +710,6 @@ struct DiagnosticMessage: SwiftDiagnostics.DiagnosticMessage { ) } - /// Create a diagnostic messages stating that a range expression constructed - /// with a prefix operator (`...n` or `.. Self { - var exprCopy = expr - exprCopy.operator.trailingTrivia = .space - let lowerBoundPlaceholderExpr = EditorPlaceholderExprSyntax(type: "Int") - let replacementExpr: ExprSyntax = "\(lowerBoundPlaceholderExpr) \(exprCopy)" - - return Self( - syntax: Syntax(expr), - message: "Range expression '\(expr.trimmed)' is ambiguous without an explicit lower bound", - severity: .warning, - fixIts: [ - FixIt( - message: MacroExpansionFixItMessage("Add lower bound"), - changes: [.replace(oldNode: Syntax(expr), newNode: Syntax(replacementExpr)),] - ), - ] - ) - } - /// Create a diagnostic message stating that a condition macro nested inside /// an exit test will not record any diagnostics. /// diff --git a/Sources/TestingMacros/TestDeclarationMacro.swift b/Sources/TestingMacros/TestDeclarationMacro.swift index 012835a0c..c98373469 100644 --- a/Sources/TestingMacros/TestDeclarationMacro.swift +++ b/Sources/TestingMacros/TestDeclarationMacro.swift @@ -35,54 +35,6 @@ public struct TestDeclarationMacro: PeerMacro, Sendable { .disabled } - /// A syntax visitor that looks for uses of `...n` and `.. PrefixOperatorExprSyntax? { - if let argumentExpr = removeParentheses(from: argumentExpr) { - return _ambiguousArgumentExpr(argumentExpr) - } - - guard let argumentExpr = argumentExpr.as(PrefixOperatorExprSyntax.self) else { - return nil - } - - let operatorTokenKind = argumentExpr.operator.tokenKind - if operatorTokenKind == .prefixOperator("...") || operatorTokenKind == .prefixOperator("..<") { - return argumentExpr - } - return nil - } - - override func visit(_ node: FunctionCallExprSyntax) -> SyntaxVisitorContinueKind { - let calledExpr = node.calledExpression.tokens(viewMode: .fixedUp).map(\.textWithoutBackticks).joined() - guard calledExpr == "confirmation" || calledExpr == "Testing.confirmation" else { - return .visitChildren - } - - let expectedCountArgument = node.arguments.first { $0.label?.tokenKind == .identifier("expectedCount") } - guard let argumentExpr = expectedCountArgument?.expression else { - return .visitChildren - } - - if let argumentExpr = _ambiguousArgumentExpr(argumentExpr) { - ambiguousArgumentExprs.append(argumentExpr) - } - return .visitChildren - } - } - /// Diagnose issues with a `@Test` declaration. /// /// - Parameters: @@ -166,17 +118,6 @@ public struct TestDeclarationMacro: PeerMacro, Sendable { } } - // Look for calls to confirmation() with a .. & Sendable`. ([96960993](rdar://96960993)) -protocol ExpectedCount: RangeExpression, Sendable where Bound == Int {} +protocol ExpectedCount: RangeExpression, Sequence, Sendable where Bound == Int, Element == Int {} extension ClosedRange: ExpectedCount {} extension PartialRangeFrom: ExpectedCount {} -extension PartialRangeThrough: ExpectedCount {} -extension PartialRangeUpTo: ExpectedCount {} extension Range: ExpectedCount {} From d3ffe034e2f961852d91bdf6010af15d55f0d7bc Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Wed, 23 Oct 2024 14:59:30 -0400 Subject: [PATCH 04/10] Address feedback --- Documentation/Proposals/NNNN-ranged-confirmations.md | 7 +------ Sources/Testing/Issues/Confirmation.swift | 4 ++-- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/Documentation/Proposals/NNNN-ranged-confirmations.md b/Documentation/Proposals/NNNN-ranged-confirmations.md index bb5198496..aa3aac400 100644 --- a/Documentation/Proposals/NNNN-ranged-confirmations.md +++ b/Documentation/Proposals/NNNN-ranged-confirmations.md @@ -177,12 +177,7 @@ this value is `Int`. ## Acknowledgments -If significant changes or improvements suggested by members of the community -were incorporated into the proposal as it developed, take a moment here to thank -them for their contributions. This is a collaborative process, and everyone's -input should receive recognition! - -Generally, you should not acknowledge anyone who is listed as a co-author. +Thanks to the testing team for their help preparing this pitch! [^1]: In the future, this type will change to `any RangeExpression & Sendable`. Compiler support is required diff --git a/Sources/Testing/Issues/Confirmation.swift b/Sources/Testing/Issues/Confirmation.swift index 743f3ad45..95b7284b0 100644 --- a/Sources/Testing/Issues/Confirmation.swift +++ b/Sources/Testing/Issues/Confirmation.swift @@ -201,7 +201,7 @@ public func confirmation( } /// An overload of ``confirmation(_:expectedCount:isolation:sourceLocation:_:)-6bkl6`` -/// that handles the unbounded range operator (`...`). +/// that handles the partial-range-through operator (`...n`). /// /// This overload is necessary because the lower bound of `PartialRangeThrough` /// is ambiguous: does it start at `0` or `1`? Test authors should specify a @@ -217,7 +217,7 @@ public func confirmation( } /// An overload of ``confirmation(_:expectedCount:isolation:sourceLocation:_:)-6bkl6`` -/// that handles the unbounded range operator (`...`). +/// that handles the partial-range-up-to operator (`.. Date: Wed, 23 Oct 2024 15:14:20 -0400 Subject: [PATCH 05/10] Avoid needing _parameterizedProtocolsAPI --- Package.swift | 1 - Sources/Testing/Issues/Issue.swift | 14 +++++++++++++- cmake/modules/shared/AvailabilityDefinitions.cmake | 1 - 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Package.swift b/Package.swift index 8fdbd5585..f7fb5dc5f 100644 --- a/Package.swift +++ b/Package.swift @@ -150,7 +150,6 @@ extension Array where Element == PackageDescription.SwiftSetting { .enableExperimentalFeature("AvailabilityMacro=_clockAPI:macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0"), .enableExperimentalFeature("AvailabilityMacro=_regexAPI:macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0"), .enableExperimentalFeature("AvailabilityMacro=_swiftVersionAPI:macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0"), - .enableExperimentalFeature("AvailabilityMacro=_parameterizedProtocolsAPI:macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0"), .enableExperimentalFeature("AvailabilityMacro=_typedThrowsAPI:macOS 15.0, iOS 18.0, watchOS 11.0, tvOS 18.0, visionOS 2.0"), .enableExperimentalFeature("AvailabilityMacro=_distantFuture:macOS 99.0, iOS 99.0, watchOS 99.0, tvOS 99.0, visionOS 99.0"), diff --git a/Sources/Testing/Issues/Issue.swift b/Sources/Testing/Issues/Issue.swift index 18d40e53e..886243cbe 100644 --- a/Sources/Testing/Issues/Issue.swift +++ b/Sources/Testing/Issues/Issue.swift @@ -170,6 +170,18 @@ extension Issue: CustomStringConvertible, CustomDebugStringConvertible { } } +/// An empty protocol defining a type that conforms to `RangeExpression`. +/// +/// In the future, when our minimum deployment target supports casting a value +/// to a constrained existential type ([SE-0353](https://github.com/swiftlang/swift-evolution/blob/main/proposals/0353-constrained-existential-types.md#effect-on-abi-stability)), +/// we can remove this protocol and cast to `RangeExpression` instead. +private protocol _RangeExpressionOverIntValues: RangeExpression where Bound == Int {} +extension ClosedRange: _RangeExpressionOverIntValues {} +extension PartialRangeFrom: _RangeExpressionOverIntValues {} +extension PartialRangeThrough: _RangeExpressionOverIntValues {} +extension PartialRangeUpTo: _RangeExpressionOverIntValues {} +extension Range: _RangeExpressionOverIntValues {} + extension Issue.Kind: CustomStringConvertible { public var description: String { switch self { @@ -187,7 +199,7 @@ extension Issue.Kind: CustomStringConvertible { "Expectation failed: \(expectation.evaluatedExpression.expandedDescription())" } case let .confirmationMiscounted(actual: actual, expected: expected): - if #available(_parameterizedProtocolsAPI, *), let expected = expected as? any RangeExpression { + if let expected = expected as? any _RangeExpressionOverIntValues { let expected = expected.relative(to: []) if expected.upperBound > expected.lowerBound && expected.lowerBound == expected.upperBound - 1 { return "Confirmation was confirmed \(actual.counting("time")), but expected to be confirmed \(expected.lowerBound.counting("time"))" diff --git a/cmake/modules/shared/AvailabilityDefinitions.cmake b/cmake/modules/shared/AvailabilityDefinitions.cmake index f0927eefe..0685fcecd 100644 --- a/cmake/modules/shared/AvailabilityDefinitions.cmake +++ b/cmake/modules/shared/AvailabilityDefinitions.cmake @@ -13,6 +13,5 @@ add_compile_options( "SHELL:$<$:-Xfrontend -define-availability -Xfrontend \"_clockAPI:macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0\">" "SHELL:$<$:-Xfrontend -define-availability -Xfrontend \"_regexAPI:macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0\">" "SHELL:$<$:-Xfrontend -define-availability -Xfrontend \"_swiftVersionAPI:macOS 13.0, iOS 16.0, tvOS 16.0, watchOS 9.0\">" - "SHELL:$<$:-Xfrontend -define-availability -Xfrontend \"_parameterizedProtocolsAPI:macOS 13.0, iOS 16.0, watchOS 9.0, tvOS 16.0\">" "SHELL:$<$:-Xfrontend -define-availability -Xfrontend \"_typedThrowsAPI:macOS 15.0, iOS 18.0, watchOS 11.0, tvOS 18.0, visionOS 2.0\">" "SHELL:$<$:-Xfrontend -define-availability -Xfrontend \"_distantFuture:macOS 99.0, iOS 99.0, watchOS 99.0, tvOS 99.0, visionOS 99.0\">") From fd1f83d32e6f52fa868d5bbda2e61057e2ce547b Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Wed, 23 Oct 2024 15:15:42 -0400 Subject: [PATCH 06/10] Update Sources/Testing/Testing.docc/testing-asynchronous-code.md Co-authored-by: Stuart Montgomery --- Sources/Testing/Testing.docc/testing-asynchronous-code.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Testing/Testing.docc/testing-asynchronous-code.md b/Sources/Testing/Testing.docc/testing-asynchronous-code.md index 964eb515a..54e4b6a2f 100644 --- a/Sources/Testing/Testing.docc/testing-asynchronous-code.md +++ b/Sources/Testing/Testing.docc/testing-asynchronous-code.md @@ -74,7 +74,7 @@ func boughtSandwiches() async { In this example, there may be zero customers or up to (but not including) 1,000 customers who order sandwiches. Any [range expression](https://developer.apple.com/documentation/swift/rangeexpression) -can be used: +which includes an explicit lower bound can be used: | Range Expression | Usage | |-|-| From dad1298350da87d1d2d6d6ff620a544f778e0c2c Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Wed, 23 Oct 2024 15:29:59 -0400 Subject: [PATCH 07/10] Add bug number --- Documentation/Proposals/NNNN-ranged-confirmations.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/Proposals/NNNN-ranged-confirmations.md b/Documentation/Proposals/NNNN-ranged-confirmations.md index aa3aac400..e27a4fca0 100644 --- a/Documentation/Proposals/NNNN-ranged-confirmations.md +++ b/Documentation/Proposals/NNNN-ranged-confirmations.md @@ -3,6 +3,7 @@ * Proposal: [SWT-NNNN](NNNN-ranged-confirmations.md) * Authors: [Jonathan Grynspan](https://github.com/grynspan) * Status: **Awaiting review** +* Bug: rdar://138499457 * Implementation: [swiftlang/swift-testing#598](https://github.com/swiftlang/swift-testing/pull/598), [swiftlang/swift-testing#689](https://github.com/swiftlang/swift-testing/pull689) * Review: ([pitch](https://forums.swift.org/t/pitch-range-based-confirmations/74589)) From 13e7098e7423ebc701faa1d2a969bb9ebac12c7e Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Thu, 24 Oct 2024 17:02:47 -0400 Subject: [PATCH 08/10] Fix a typo in a test --- Tests/TestingTests/ConfirmationTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/TestingTests/ConfirmationTests.swift b/Tests/TestingTests/ConfirmationTests.swift index c68b208a6..7fe824d71 100644 --- a/Tests/TestingTests/ConfirmationTests.swift +++ b/Tests/TestingTests/ConfirmationTests.swift @@ -29,7 +29,7 @@ struct ConfirmationTests { @Test("Unsuccessful confirmations") func unsuccessfulConfirmations() async { - await confirmation("Miscount recorded", expectedCount: 9) { miscountRecorded in + await confirmation("Miscount recorded", expectedCount: 7) { miscountRecorded in var configuration = Configuration() configuration.eventHandler = { event, _ in if case let .issueRecorded(issue) = event.kind { From 5195fae7bc6bb9e355cc88106194d4567aafe3e1 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Fri, 25 Oct 2024 14:59:28 -0400 Subject: [PATCH 09/10] Update proposal number --- ...N-ranged-confirmations.md => 0005-ranged-confirmations.md} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename Documentation/Proposals/{NNNN-ranged-confirmations.md => 0005-ranged-confirmations.md} (98%) diff --git a/Documentation/Proposals/NNNN-ranged-confirmations.md b/Documentation/Proposals/0005-ranged-confirmations.md similarity index 98% rename from Documentation/Proposals/NNNN-ranged-confirmations.md rename to Documentation/Proposals/0005-ranged-confirmations.md index e27a4fca0..ebad53800 100644 --- a/Documentation/Proposals/NNNN-ranged-confirmations.md +++ b/Documentation/Proposals/0005-ranged-confirmations.md @@ -1,8 +1,8 @@ # Range-based confirmations -* Proposal: [SWT-NNNN](NNNN-ranged-confirmations.md) +* Proposal: [SWT-0005](0005-ranged-confirmations.md) * Authors: [Jonathan Grynspan](https://github.com/grynspan) -* Status: **Awaiting review** +* Status: **Accepted** * Bug: rdar://138499457 * Implementation: [swiftlang/swift-testing#598](https://github.com/swiftlang/swift-testing/pull/598), [swiftlang/swift-testing#689](https://github.com/swiftlang/swift-testing/pull689) * Review: ([pitch](https://forums.swift.org/t/pitch-range-based-confirmations/74589)) From 1eb2987b39b7e6f38563aee16a8eac5a4345e377 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Fri, 25 Oct 2024 15:31:07 -0400 Subject: [PATCH 10/10] Add acceptance link --- Documentation/Proposals/0005-ranged-confirmations.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/Proposals/0005-ranged-confirmations.md b/Documentation/Proposals/0005-ranged-confirmations.md index ebad53800..7ba133c9f 100644 --- a/Documentation/Proposals/0005-ranged-confirmations.md +++ b/Documentation/Proposals/0005-ranged-confirmations.md @@ -5,7 +5,8 @@ * Status: **Accepted** * Bug: rdar://138499457 * Implementation: [swiftlang/swift-testing#598](https://github.com/swiftlang/swift-testing/pull/598), [swiftlang/swift-testing#689](https://github.com/swiftlang/swift-testing/pull689) -* Review: ([pitch](https://forums.swift.org/t/pitch-range-based-confirmations/74589)) +* Review: ([pitch](https://forums.swift.org/t/pitch-range-based-confirmations/74589)), + ([acceptance](https://forums.swift.org/t/pitch-range-based-confirmations/74589/7)) ## Introduction