Validate listener in ShadowAccountManager#addOnAccountsUpdatedListener()
#10361
Conversation
…er()` This commit addresses the TODO in `ShadowAccountManager#addOnAccountsUpdatedListener()` to: - Check that the provided listener is not `null`. - Check that the provided listener was not already added. These changes align our implementation with the Android one: https://developer.android.com/reference/kotlin/android/accounts/AccountManager?hl=en#addonaccountsupdatedlistener_1
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the TODO in ShadowAccountManager#addOnAccountsUpdatedListener() by introducing validation for null and duplicate listeners. The changes align Robolectric's shadow behavior more closely with the official Android framework documentation, which is a valuable improvement for test fidelity.
The implementation is clear, concise, and the new behavior is well-covered by updated and new unit tests. Specifically:
- The null check for the listener now correctly throws an
IllegalArgumentException. - Attempting to add a duplicate listener now correctly throws an
IllegalStateException, and the exception message"this listener is already added"precisely matches the Android O+ framework behavior. - The accompanying tests in
ShadowAccountManagerTest.javahave been updated to assert these new exception-throwing behaviors.
Overall, this is a high-quality contribution that enhances the correctness of the ShadowAccountManager. Well done!
Merge Readiness
The changes in this pull request are well-implemented, directly address the intended improvements, and are accompanied by appropriate tests. Based on this review, the code appears to be of high quality and aligns well with Android framework behavior, making it ready for merging from a technical standpoint.
However, as an AI reviewer, I am not authorized to approve pull requests. It is important that this PR undergoes further review and approval by human team members before being merged.
|
@hoisie Do you want to do an internal verification or just adapting internal code if there are some breaks when you sync code? |
|
Sounds good I will run this against Google's tests for some extra validation. |
This commit addresses the TODO in
ShadowAccountManager#addOnAccountsUpdatedListener()to:null.These changes align our implementation with the Android one: https://developer.android.com/reference/kotlin/android/accounts/AccountManager?hl=en#addonaccountsupdatedlistener_1