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

Fix possible ambiguous use of when(fulfilled:) in tests #1171

Merged
merged 2 commits into from
May 24, 2021
Merged

Fix possible ambiguous use of when(fulfilled:) in tests #1171

merged 2 commits into from
May 24, 2021

Conversation

RomanPodymov
Copy link
Collaborator

Hello.
Thank you for PromiseKit.
I found out that in testUnhandledErrorHandlerDoesNotFireForStragglers and testAllSealedRejectedFirstOneRejects it is hard to determine which version of when(fulfilled:) is being used. As for me, there should be the ambiguous use of when(fulfilled:) error. My suggestion is to store when(fulfilled: p1, p2, p3)'s result in a variable, it helps to know which version of when(fulfilled:) is being used.

@@ -218,7 +218,8 @@ class WhenTests: XCTestCase {
let p2 = after(.milliseconds(100)).done { throw Error.straggler }
let p3 = after(.milliseconds(200)).done { throw Error.straggler }

when(fulfilled: p1, p2, p3).catch { error -> Void in
let whenFulfilledP1P2P3: Promise<(Void, Void, Void)> = when(fulfilled: p1, p2, p3)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There can be also let whenFulfilledP1P2P3: Promise<Void> = when(fulfilled: p1, p2, p3).

@@ -241,7 +242,8 @@ class WhenTests: XCTestCase {
let p2 = Promise<Void>(error: Error.test2)
let p3 = Promise<Void>(error: Error.test3)

when(fulfilled: p1, p2, p3).catch { error in
let whenFulfilledP1P2P3: Promise<Void> = when(fulfilled: p1, p2, p3)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There can be also let whenFulfilledP1P2P3: Promise<(Void, Void, Void)> = when(fulfilled: p1, p2, p3).

@mxcl mxcl force-pushed the master branch 8 times, most recently from caddf7d to 0122f95 Compare May 24, 2021 16:06
@mxcl
Copy link
Owner

mxcl commented May 24, 2021

Forgive me, but I don’t see how this is ambiguous, ambiguous with which other version of when?

@RomanPodymov
Copy link
Collaborator Author

Hello @mxcl
For instance, in

let p1 = Promise<Void>(error: Error.test)
let p2 = after(.milliseconds(100)).done { throw Error.straggler }
let p3 = after(.milliseconds(200)).done { throw Error.straggler }

when(fulfilled: p1, p2, p3).catch { error -> Void in
    XCTAssertTrue(Error.test == error as? Error)
    ex1.fulfill()
}

As I can see in the debugger func when<U: Thenable, V: Thenable, W: Thenable>(fulfilled pu: U, _ pv: V, _ pw: W) -> Promise<(U.T, V.T, W.T)> is using here. But public func when<U: Thenable>(fulfilled promises: U...) -> Promise<Void> where U.T == Void is suitable here as well:

(when(fulfilled: p1, p2, p3) as Promise<Void>).catch { error -> Void in
    XCTAssertTrue(Error.test == error as? Error)
    ex1.fulfill()
}

Maybe there is a rule why func when<U: Thenable, V: Thenable, W: Thenable>(fulfilled pu: U, _ pv: V, _ pw: W) -> Promise<(U.T, V.T, W.T)> has a higher priority.

@mxcl
Copy link
Owner

mxcl commented May 24, 2021

Oh ok. I’m not sure it matters per se, but why not.

@mxcl mxcl merged commit 41e78ba into mxcl:master May 24, 2021
@RomanPodymov RomanPodymov deleted the fix/tests branch May 24, 2021 19:52
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