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: new error handling in auth plugin unit test suites #3254

Merged
merged 2 commits into from
Sep 29, 2023

Conversation

atierian
Copy link
Member

@atierian atierian commented Sep 29, 2023

Swift SDK Update 0.26.0

Makes necessary changes to get AWSCognitoAuthPlugin unit test suites to build and tests passing.

Debt Introduced

  • Client side retry errors from ClientRuntime / CRT.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@atierian atierian requested a review from a team as a code owner September 29, 2023 17:47
@atierian atierian changed the base branch from main to swift-sdk-0260 September 29, 2023 17:48
@atierian atierian changed the title fix fix: new error handling in auth plugin unit test suites Sep 29, 2023
@atierian atierian temporarily deployed to Fortify September 29, 2023 17:57 — with GitHub Actions Inactive
@@ -221,7 +228,7 @@ class SetUpTOTPTaskTests: BasePluginTest {
XCTFail("Should produce service error instead of \(error)")
return
}
guard case .mfaMethodNotFound = (underlyingError as? AWSCognitoAuthError) else {
guard case .softwareTokenMFANotEnabled = (underlyingError as? AWSCognitoAuthError) else {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is necessary because we were previously mapping to the wrong underlyingError.

@@ -175,14 +180,15 @@ class VerifyTOTPSetupTaskTests: BasePluginTest {
/// - When:
/// - I invoke verifyTOTPSetup
/// - Then:
/// - I should get a .service error with .mfaMethodNotFound as underlyingError
/// - I should get a .service error with .softwareTokenMFANotEnabled as underlyingError
Copy link
Member Author

Choose a reason for hiding this comment

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

We were previously mapping to the wrong underlyingError.

@@ -193,7 +199,7 @@ class VerifyTOTPSetupTaskTests: BasePluginTest {
XCTFail("Should produce service error instead of \(error)")
return
}
guard case .mfaMethodNotFound = (underlyingError as? AWSCognitoAuthError) else {
guard case .softwareTokenMFANotEnabled = (underlyingError as? AWSCognitoAuthError) else {
Copy link
Member Author

Choose a reason for hiding this comment

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

We were previously mapping to the wrong underlyingError.

@@ -417,7 +424,7 @@ class ConfirmSignInWithSetUpMFATaskTests: BasePluginTest {
XCTFail("Should produce service error instead of \(error)")
return
}
guard case .mfaMethodNotFound = (underlyingError as? AWSCognitoAuthError) else {
guard case .softwareTokenMFANotEnabled = (underlyingError as? AWSCognitoAuthError) else {
Copy link
Member Author

Choose a reason for hiding this comment

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

We were previously mapping to the wrong underlyingError.

@@ -493,7 +493,7 @@ class SignInSetUpTOTPTests: BasePluginTest {
XCTFail("Should not produce result - \(result)")
} catch {
guard case AuthError.service(_, _, let underlyingError) = error,
case .mfaMethodNotFound = (underlyingError as? AWSCognitoAuthError) else {
case .softwareTokenMFANotEnabled = (underlyingError as? AWSCognitoAuthError) else {
Copy link
Member Author

Choose a reason for hiding this comment

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

We were previously mapping to the wrong underlyingError.

Comment on lines +17 to +18
@MainActor
func testAuthCognitoPlugin() async {
Copy link
Member Author

Choose a reason for hiding this comment

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

async bubbled up to this test from a change in the SDK. It needs to be @MainActor because async and XCTContext.runActivity do not play nicely together otherwise.

@atierian atierian merged commit f91b3e6 into swift-sdk-0260 Sep 29, 2023
23 of 61 checks passed
@atierian atierian deleted the sdk/auth-tests branch September 29, 2023 18:39
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