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

Introduce timeout and after to support delayed verification. #434

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sk409
Copy link
Contributor

@sk409 sk409 commented Sep 3, 2022

I introduced these two global functions to support delayed verification discussed in this issue(#277).

// timeout waits until CallMatcher::matches returns true and exits on success.
public func timeout(_ timeoutDuration: TimeInterval, waitingDuration: TimeInterval = 0.01) -> ContinuationWithTimeout
// after waits until delayDuration elapsed and exits when CallMatcher::matches never return true.
public func after(_ delayDuration: TimeInterval, waitingDuration: TimeInterval = 0.01) -> ContinueationAfterDelay

ContinuationWithTimeout and ContinueationAfterDelay conform to Continuation protocol.

Continuation determines whether CallMatcher::matches should be called again, waits, etc.

like this:

// MockManager
public func verify<IN, OUT>(_ method: String, callMatcher: CallMatcher, parameterMatchers: [ParameterMatcher<IN>], continuation: Continuation, sourceLocation: SourceLocation) -> __DoNotUse<IN, OUT> {
        ...
        while continuation.check() {
            calls = []
            indexesToRemove = []
            for (i, stubCall) in stubCalls.enumerated() {
                if let stubCall = stubCall as? ConcreteStubCall<IN> , (parameterMatchers.reduce(stubCall.method == method) { $0 && $1.matches(stubCall.parameters) }) {
                    calls.append(stubCall)
                    indexesToRemove.append(i)
                }
            }
            matches = callMatcher.matches(calls)
            if !matches && !callMatcher.canRecoverFromFailure(calls) {
                break
            }
            if matches && continuation.exitOnSuccess {
                break
            }
            continuation.wait()
        }
        ...
        return __DoNotUse()
    }

Users can simply write test of fire-and-forget asynchronous function and asynchronous function with completion closure that does not require argument verification.

stub(mockTestClass) { stub in
    when(stub.f()).thenDoNothing()
}
// someMethod is expected to call f three times within three seconds.
mockTestClass.someMethod()
verify(mockTestClass, timeout(3).times(3)).f()

But I think that if users want to verify completion closure argument, They should use expectation.

stub(mockTestClass) { stub in
    when(stub.f()).thenDoNothing()
}
let expectation = XCTestExpectation(description: #function)
mockTestClass.someMethod() { i in
    XCTAssertEqual(i, 1)
    expectation.fulfill()
}
wait(for: [expectation], timeout: 3)
verify(mockTestClass, times(3)).f()

If users write below code, XCTAssertEqual may no be called.

stub(mockTestClass) { stub in
    when(stub.f()).thenDoNothing()
}
mockTestClass.someMethod() { i in
    XCTAssertEqual(i, 1)
}
verify(mockTestClass, timeout(3).times(3)).f()

This PR is a prototype.
If this PR is acceptable, I will write comments, tests and explanation to README.md.

If there are any improvements, please point them out.

self.sourceLocation = sourceLocation
}

{{ container.accessibility }} init(manager: Cuckoo.MockManager, callMatcher: Cuckoo.CallMatcher, sourceLocation: Cuckoo.SourceLocation) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why opt for duplicate init instead of a default parameter value?

}

func atMost(_ count: Int) -> VerificationSpec {
VerificationSpec(callMatcher: Cuckoo.atMost(count), continuation: self)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems to me like this one and every method above it can just call the with(_:) method, is that right?


import Foundation

public class ContinueationAfterDelay: NSObject, ContinuationWrapper {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo "Continueation".

//
// Created by Shoto Kobayashi on 03/09/2022.
//

Copy link
Collaborator

Choose a reason for hiding this comment

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

Reduntant line.

}

public func check() -> Bool {
if start == nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please bind start as if let start instead of using a force unwrap below.


import Foundation

public protocol ContinuationWrapper: Continuation {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this type necessary?

@@ -11,33 +11,44 @@ public struct CallMatcher {
public let name: String

private let matchesFunction: ([StubCall]) -> Bool
private let canRecoverFromFailureFunction: ([StubCall]) -> Bool
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this necessary?

if matches && continuation.exitOnSuccess {
break
}
continuation.wait()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't this always wait for the full duration of the timeout even if the matcher is satisfied?

@MatyasKriz
Copy link
Collaborator

I like the API this provides, but I need some more context on the implementation before we go ahead with it.

@MatyasKriz MatyasKriz force-pushed the master branch 2 times, most recently from d2d44b9 to a4618f6 Compare April 22, 2024 20:38
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