Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Add tests for sync, wifi, and push #12581

Merged
merged 1 commit into from
Jul 16, 2020

Conversation

NotWoods
Copy link
Contributor

In addition to lots of tests, there has been some cleanup:

  • SyncedTabsIntegration takes in syncedTabsStorage, instead of using context to get syncedTabsStorage from components.
  • WifiConnectionMonitor now uses the Observable code from AC instead of re-implementing the observable pattern.
  • SitePermissionsWifiIntegration has been updated to call AC observation methods

@NotWoods NotWoods added the eng:test-debt Test debt. Unit test creation or maintenance. label Jul 15, 2020
@NotWoods NotWoods requested a review from ekager July 15, 2020 03:36
@NotWoods NotWoods force-pushed the test-sync-wifi-push branch from 7fc67f1 to 0bac0f4 Compare July 15, 2020 18:35
@NotWoods NotWoods force-pushed the test-sync-wifi-push branch from 0bac0f4 to 47a90f3 Compare July 16, 2020 01:07
@codecov-commenter
Copy link

codecov-commenter commented Jul 16, 2020

Codecov Report

Merging #12581 into master will increase coverage by 0.39%.
The diff coverage is 94.73%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #12581      +/-   ##
============================================
+ Coverage     25.15%   25.54%   +0.39%     
- Complexity      845      866      +21     
============================================
  Files           389      389              
  Lines         15700    15687      -13     
  Branches       2022     2020       -2     
============================================
+ Hits           3949     4008      +59     
+ Misses        11444    11371      -73     
- Partials        307      308       +1     
Impacted Files Coverage Δ Complexity Δ
...va/org/mozilla/fenix/sync/SyncedTabsIntegration.kt 100.00% <ø> (+100.00%) 2.00 <0.00> (+2.00)
...va/org/mozilla/fenix/wifi/WifiConnectionMonitor.kt 94.44% <87.50%> (+94.44%) 5.00 <1.00> (+5.00)
...in/java/org/mozilla/fenix/components/Components.kt 30.00% <100.00%> (ø) 1.00 <0.00> (ø)
...ava/org/mozilla/fenix/sync/SyncedTabsViewHolder.kt 100.00% <100.00%> (+100.00%) 1.00 <0.00> (+1.00)
...zilla/fenix/wifi/SitePermissionsWifiIntegration.kt 100.00% <100.00%> (+100.00%) 10.00 <5.00> (+10.00)
...n/java/org/mozilla/fenix/sync/SyncedTabsAdapter.kt 10.00% <0.00%> (+10.00%) 0.00% <0.00%> (ø%)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a0d9cdc...729ff2e. Read the comment docs.

@@ -159,7 +159,10 @@ class BackgroundServices(
notificationManager.showReceivedTabs(context, device, tabs)
}

SyncedTabsIntegration(context, accountManager).launch()
SyncedTabsIntegration(
syncedTabsStorage = syncedTabsStorage,
Copy link
Contributor

@jonalmeida jonalmeida Jul 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't land this without testing the feature carefully. If I recall correctly, this causes a cyclic dependency, which is why we accessed the storage via the context instead of passing it in through he constructor.

@NotWoods NotWoods force-pushed the test-sync-wifi-push branch from 47a90f3 to 0cc825d Compare July 16, 2020 16:47
@NotWoods NotWoods force-pushed the test-sync-wifi-push branch from 0cc825d to 729ff2e Compare July 16, 2020 18:38
@NotWoods NotWoods merged commit 2d066d7 into mozilla-mobile:master Jul 16, 2020
@NotWoods NotWoods deleted the test-sync-wifi-push branch July 16, 2020 19:25
@liuche liuche mentioned this pull request Jul 20, 2020
9 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
eng:test-debt Test debt. Unit test creation or maintenance.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants