[local_auth] Fix callback thread handling #3778
Conversation
|
|
| if (_authContextOverride != nil) { | ||
| return _authContextOverride; | ||
| } | ||
| return [[LAContext alloc] init]; |
There was a problem hiding this comment.
Each self.authContext is going to call the constructor (when no _authContextOverride is provided)
If we always wants a new instance (which i think that's the case), we should rename it do something doesn't sound too much like a getter. Maybe: - (LAContext *)getNewAuthContext`?
There was a problem hiding this comment.
Renamed to createAuthContext since create is a standard idiom. Also future-proofed the override by making it better fit the fact that this is really, as you point out, a factory rather than a getter; it now uses an array of overrides, so a test could run a flow with multiple API calls.
| [plugin handleMethodCall:call | ||
| result:^(id _Nullable result) { | ||
| XCTAssertTrue([NSThread isMainThread]); | ||
| [expectation fulfill]; |
There was a problem hiding this comment.
Shouldn't this fulfill be the last call of the block? After XCTAssertTrue([result boolValue]);? Unless I'm missing something? Ditto below if make sense.
There was a problem hiding this comment.
Good call; done. It's been a long time since I've done async XCTests!
| XCTAssertTrue([result isKindOfClass:[NSNumber class]]); | ||
| XCTAssertTrue([result boolValue]); | ||
| }]; | ||
| [self waitForExpectationsWithTimeout:1.0 handler:nil]; |
There was a problem hiding this comment.
We should increase the timeout as running tests on CI might be slow and we don't want to many false negatives (flake) because of the environment is slow. How about 30s? and set continueAfterFailure to NO in setup
Ensure that all auth replies, which are sent on an internal framework queue per documentation, are dispatched back to the main thread for handling, as all resulting operations (method channel callbacks, display of UI) are things that must be done on the main thread In order to test this, sets up local_auth with XCTest-based tests, and adds the ability to inject a mock LAContext. (This does not do full unit test backfill, to limit the scope of the PR.) Fixes flutter/flutter#47465
Ensure that all auth replies, which are sent on an internal framework queue per documentation, are dispatched back to the main thread for handling, as all resulting operations (method channel callbacks, display of UI) are things that must be done on the main thread
In order to test this, sets up local_auth with XCTest-based tests, and adds the ability to inject a mock
LAContext. (This does not do full unit test backfill, to limit the scope of the PR.)Fixes flutter/flutter#47465
Pre-launch Checklist
[shared_preferences]///).