-
Notifications
You must be signed in to change notification settings - Fork 704
Refactor CardAccountRangeService to Flow-based interface for improved testability #12206
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
Conversation
|
Diffuse output: APKDEX |
86efc22 to
c55c8ae
Compare
| @Test | ||
| fun `Entering VISA BIN does not call accountRangeRepository`() { | ||
| val fakeRepository = FakeCardAccountRangeRepository() | ||
| val cardNumberController = createController(repository = fakeRepository) | ||
|
|
||
| cardNumberController.onValueChange("42424242424242424242") | ||
| idleLooper() | ||
| assertThat(fakeRepository.numberOfCalls).isEqualTo(0) | ||
| } | ||
|
|
||
| @Test | ||
| fun `Entering valid 19 digit UnionPay BIN returns accountRange of 19`() { | ||
| val cardNumberController = createController() | ||
| cardNumberController.onValueChange("6216828050000000000") | ||
| idleLooper() | ||
| assertThat(cardNumberController.accountRangeService.accountRange!!.panLength).isEqualTo(19) | ||
| } | ||
|
|
||
| @Test | ||
| fun `Entering valid 16 digit UnionPay BIN returns accountRange of 16`() { | ||
| val cardNumberController = createController() | ||
| cardNumberController.onValueChange("6282000000000000") | ||
| idleLooper() | ||
| assertThat(cardNumberController.accountRangeService.accountRange!!.panLength).isEqualTo(16) | ||
| } |
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.
These are testing implementation details of accountRangeService and they already exist in CardAccountRangeServiceTest.kt
c55c8ae to
2450ba3
Compare
| assertThat(cardNumberEditText.accountRangeService.accountRangeRepositoryJob) | ||
| .isNull() | ||
| assertThat(accountRangeService.cancelAccountRangeRepositoryJobCount).isEqualTo(1) | ||
| } |
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.
We should not have access to accountRangeRepositoryJob job here
0a1071a to
aaa1fbf
Compare
aaa1fbf to
f09313d
Compare
Summary
This is a structural change to make the card funding work simpler (especially from a testing perspective).
This PR refactors
CardAccountRangeServicefrom a concrete callback-based implementation to an interface-based architecture with Kotlin Flow reactivity:Architectural Changes:
CardAccountRangeServiceinterface withDefaultCardAccountRangeServiceimplementationAccountRangeResultListenerwith reactive FlowsTest Infrastructure:
FakeCardAccountRangeServicefor easier test injectionCountDownLatch/CompletableDeferredwithflow.test { awaitItem() }patternsIntegration Updates:
Motivation
The existing
CardAccountRangeServiceimplementation uses a concrete class with callback-based state updates, making it difficult to test components that depend on it. This refactoring:Testing
Changelog