-
Notifications
You must be signed in to change notification settings - Fork 1.3k
For #2165 - Implement pull-to-refresh gesture to sync history. #11386
For #2165 - Implement pull-to-refresh gesture to sync history. #11386
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11386 +/- ##
============================================
+ Coverage 20.81% 20.86% +0.04%
- Complexity 680 681 +1
============================================
Files 373 373
Lines 15013 15038 +25
Branches 2026 2031 +5
============================================
+ Hits 3125 3137 +12
- Misses 11606 11617 +11
- Partials 282 284 +2
Continue to review full report at Codecov.
|
app/src/main/java/org/mozilla/fenix/library/history/HistoryController.kt
Outdated
Show resolved
Hide resolved
0bc3012
to
8587153
Compare
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.
LGTM!
Definitely a welcomed feature, thanks!
Waiting for Tiger's review also.
controller.handleRequestSync() | ||
|
||
verify { | ||
syncHistory.invoke(any()) |
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.
I think we should also verify here that the right actions are emitted.
StartSync
is easy to be verified, but verifying that syncHistory gets passed the FinishSync
callback is a bit trickier.. maybe we could return that store.dispatch call from a method and then in the test verify if
store.dispatch(HistoryFragmentAction.StartSync)
was calledstore.dispatch(any())
was called 2 times- the method returning
store.dispatch(HistoryFragmentAction.FinishSync)
was called ?
8587153
to
548e955
Compare
You need to use a real coroutine scope instead of a mock, like this: private val scope = TestCoroutineScope() You'll also need to add an |
548e955
to
3283a03
Compare
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!
Note: This is only 1/2 gestures described in #2165
Pull Request checklist
Tests: This PR includes thorough tests or an explanation of why it does not
Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features. In addition, it includes a screenshot of a successful accessibility scan to ensure no new defects are added to the product.
After merge
To download an APK when reviewing a PR: