-
Notifications
You must be signed in to change notification settings - Fork 27
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
Allowing DeviceBinding NONE
option on Simulators
#308
Conversation
// DeviceBinding/Signing is not supported on the iOS Simulator | ||
handleException(status: .unsupported(errorMessage: "DeviceBinding/Signing is not supported on the iOS Simulator"), completion: completion) | ||
return | ||
// DeviceBinding/Signing other than `.NONE` type is not supported on the iOS Simulator |
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.
can we add some tests for this?
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.
Tests were failing and fixed based 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.
sorry didnt notice this test class update, when I add my comment, and at the same time u pushed.
// DeviceBinding/Signing is not supported on the iOS Simulator | ||
handleException(status: .unsupported(errorMessage: "DeviceBinding/Signing is not supported on the iOS Simulator"), completion: completion) | ||
return | ||
// DeviceBinding/Signing other than `.NONE` type is not supported on the iOS Simulator |
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.
add some tests for this
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.
Tests were failing and fixed based 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.
Looks good to me, but there are still tests failing.
Failing tests:
DeviceBindingCallbackTests.test_21_execute_fail_timeout()
DeviceSigningVerifierCallbackTests.test_20_execute_fail_timeout()
DeviceSigningVerifierCallbackTests.test_20_execute_single_success_with_invalid_custom_claims()
DeviceSigningVerifierCallbackTests.test_23_sign_CustomDeviceAuthenticatorCustomClaimsAlwaysInvalid()
There are two more tests failing for recaptcha but they are not related to your change.
If u wanna fix them here too, u can replace
XCTAssertEqual(textOutputCallback.message, "\"VALIDATION_ERROR:CAPTCHA validation failed\"")
with XCTAssertTrue(textOutputCallback.message.contains("VALIDATION_ERROR:CAPTCHA validation failed"))
and replace
XCTAssertEqual(textOutputCallback.message, "\"CLIENT_ERROR:CUSTOM_CLIENT_ERROR\"")
with XCTAssertTrue(textOutputCallback.message.contains("CLIENT_ERROR:CUSTOM_CLIENT_ERROR"))
Damn I thought I fixed those. Will have a look. I'll leave the |
Allowing DeviceBinding
NONE
option on SimulatorsDefinition of Done Checklist: