Repository navigation
[local_auth] Convert tests to Kotlin - #12884
auto-submit[bot] merged 13 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the Android unit tests (AuthenticationHelperTest and LocalAuthTest) from Java to Kotlin, deleting the obsolete Java test files and the ResultCompat helper class. To support this migration, the mockito-kotlin dependency is added to build.gradle.kts, and @VisibleForTesting annotations in LocalAuthPlugin.kt are replaced with Kotlin's internal visibility modifier. Additionally, a null check in getEnrolledBiometrics is simplified, and the package version is bumped to 2.2.0. There are no review comments, and I have no feedback to provide.
| } | ||
| } | ||
|
|
||
| private val dummyStrings = AuthStrings("a reason", "a hint", "cancel", "sign in") |
There was a problem hiding this comment.
Can we put these in a companion object?
There was a problem hiding this comment.
Restored the companion object in both places.
FWIW, the reason I removed it originally was that when I asked Gemini to review for idiomatic Kotlin it indicated that private file-level constants are more idiomatic that companion objects for things like this, and that seemed reasonable (and there seemed to be some evidence to support that); I don't have a strong feeling either way.
There was a problem hiding this comment.
huh, I would have thought companion objects would be more idiomatic.
|
|
||
| private val dummyStrings = AuthStrings("a reason", "a hint", "cancel", "sign in") | ||
| private val defaultOptions = | ||
| AuthOptions(biometricOnly = false, sensitiveTransaction = false, sticky = false) |
| @@ -1,3 +1,7 @@ | |||
| ## 2.2.0 | |||
|
|
|||
| * Adjusts the code slightly to support the migration of unit tests to Kotlin. | |||
There was a problem hiding this comment.
I'd rather this be a bit more specific to prevent any PR from saying that it "adjusts the code". Maybe:
Adjusts visibility modifiers to support the migration of unit tests to Kotlin.
There was a problem hiding this comment.
Yep, good call. Fixed.
stuartmorgan-g
added
the
autosubmit
2 participants
Following up from #12635, this converts the unit tests from Java to Kotlin. As with that PR, this started with a deterministic Android Studio conversion, then made follow-up fixes manually, including suggestions from a local Gemini review.
The non-test changes are to replace
@VisibleForTestingwithinternalnow that the tests are Kotlin and can therefore accessinternalmembers. (Also, one very minor refactoring to fix a code suggestion warning in Android Studio.)Part of flutter/flutter#188630
Pre-Review Checklist
[shared_preferences]///).Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2