-
Notifications
You must be signed in to change notification settings - Fork 28
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
Migrate to Swift Testing #99
base: main
Are you sure you want to change the base?
Conversation
@@ -11,22 +11,21 @@ | |||
// | |||
//===----------------------------------------------------------------------===// | |||
|
|||
import XCTest | |||
import Testing | |||
import WebAuthn | |||
|
|||
func assertThrowsError<T, E: Error>( |
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.
We can probably drop this helper altogether now, can't we?
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.
I think this helper is beneficial. Why would we want to drop it?
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.
Doesn't swift Testing have a new #expect(throws: ...) { ... }
that would replace this helper?
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.
It does yes
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.
But doesn’t this work the same as XCTAssertThrowsError?
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.
Great, I’ll work on that
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.
But doesn’t this work the same as XCTAssertThrowsError?
I assume these were there because XCTAssert didn't handle async code very well, so this abstracted over that with a single signature
@@ -11,22 +11,21 @@ | |||
// | |||
//===----------------------------------------------------------------------===// | |||
|
|||
import XCTest | |||
import Testing | |||
import WebAuthn | |||
|
|||
func assertThrowsError<T, E: Error>( |
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.
Same here
@0xTim I assume we are ok dropping Swift 5 support for this, right? |
Yes this is a new library and we're yet to hit 1.0.0 so I don't see an issue with dropping older versions of Swift, especially for the server world |
@M7md-Ebrahim When you can, please rebase off of main and tests should pass. With |
…-webauthn into swift_testing # Conflicts: # Tests/WebAuthnTests/Utils/assert+async.swift # Tests/WebAuthnTests/Utils/assert+expect.swift # Tests/WebAuthnTests/WebAuthnManagerAuthenticationTests.swift # Tests/WebAuthnTests/WebAuthnManagerRegistrationTests.swift
Migrate to Swift Testing #92