For #24211: Remove wrapper from "has open tabs" boolean metric.

mozilla-mobile · Apr 12, 2022 · 0015690 · 0015690
1 parent 8c42f7f
commit 0015690
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 25 deletions.
diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt
@@ -28,9 +28,6 @@ sealed class Event {
 
     object TabSettingsOpened : Event()
 
-    object HaveOpenTabs : Event()
-    object HaveNoOpenTabs : Event()
-
     object ContextMenuCopyTapped : Event()
     object ContextMenuSearchTapped : Event()
     object ContextMenuSelectAllTapped : Event()

diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt
@@ -131,12 +131,6 @@ private val Event.wrapper: EventWrapper<*>?
         is Event.ContextMenuShareTapped -> EventWrapper<NoExtraKeys>(
             { ContextualMenu.shareTapped.record(it) }
         )
-        Event.HaveOpenTabs -> EventWrapper<NoExtraKeys>(
-            { Metrics.hasOpenTabs.set(true) }
-        )
-        Event.HaveNoOpenTabs -> EventWrapper<NoExtraKeys>(
-            { Metrics.hasOpenTabs.set(false) }
-        )
         is Event.SyncedTabSuggestionClicked -> EventWrapper<NoExtraKeys>(
             { SyncedTabs.syncedTabsSuggestionClicked.record(it) }
         )

diff --git a/app/src/main/java/org/mozilla/fenix/telemetry/TelemetryMiddleware.kt b/app/src/main/java/org/mozilla/fenix/telemetry/TelemetryMiddleware.kt
@@ -21,7 +21,7 @@ import mozilla.components.lib.state.MiddlewareContext
 import mozilla.components.support.base.android.Clock
 import mozilla.components.support.base.log.logger.Logger
 import org.mozilla.fenix.GleanMetrics.Events
-import org.mozilla.fenix.components.metrics.Event
+import org.mozilla.fenix.GleanMetrics.Metrics
 import org.mozilla.fenix.components.metrics.MetricController
 import org.mozilla.fenix.utils.Settings
 import org.mozilla.fenix.GleanMetrics.EngineTab as EngineMetrics
@@ -75,10 +75,10 @@ class TelemetryMiddleware(
             is TabListAction.RestoreAction -> {
                 // Update/Persist tabs count whenever it changes
                 settings.openTabsCount = context.state.normalTabs.count()
-                if (context.state.normalTabs.count() > 0) {
-                    metrics.track(Event.HaveOpenTabs)
+                if (context.state.normalTabs.isNotEmpty()) {
+                    Metrics.hasOpenTabs.set(true)
                 } else {
-                    metrics.track(Event.HaveNoOpenTabs)
+                    Metrics.hasOpenTabs.set(false)
                 }
             }
         }

diff --git a/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt b/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt
@@ -6,7 +6,6 @@ package org.mozilla.fenix.telemetry
 
 import androidx.test.core.app.ApplicationProvider
 import io.mockk.mockk
-import io.mockk.verify
 import mozilla.components.browser.state.action.ContentAction
 import mozilla.components.browser.state.action.EngineAction
 import mozilla.components.browser.state.action.TabListAction
@@ -30,7 +29,7 @@ import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mozilla.fenix.GleanMetrics.Events
-import org.mozilla.fenix.components.metrics.Event
+import org.mozilla.fenix.GleanMetrics.Metrics
 import org.mozilla.fenix.components.metrics.MetricController
 import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
 import org.mozilla.fenix.utils.Settings
@@ -76,24 +75,32 @@ class TelemetryMiddlewareTest {
     @Test
     fun `WHEN a tab is added THEN the open tab count is updated`() {
         assertEquals(0, settings.openTabsCount)
+        assertFalse(Metrics.hasOpenTabs.testHasValue())
 
         store.dispatch(TabListAction.AddTabAction(createTab("https://mozilla.org"))).joinBlocking()
         assertEquals(1, settings.openTabsCount)
-        verify(exactly = 1) { metrics.track(Event.HaveOpenTabs) }
+
+        assertTrue(Metrics.hasOpenTabs.testHasValue())
+        assertTrue(Metrics.hasOpenTabs.testGetValue())
     }
 
     @Test
     fun `WHEN a private tab is added THEN the open tab count is not updated`() {
         assertEquals(0, settings.openTabsCount)
+        assertFalse(Metrics.hasOpenTabs.testHasValue())
 
         store.dispatch(TabListAction.AddTabAction(createTab("https://mozilla.org", private = true))).joinBlocking()
         assertEquals(0, settings.openTabsCount)
-        verify(exactly = 1) { metrics.track(Event.HaveNoOpenTabs) }
+
+        assertTrue(Metrics.hasOpenTabs.testHasValue())
+        assertFalse(Metrics.hasOpenTabs.testGetValue())
     }
 
     @Test
     fun `WHEN multiple tabs are added THEN the open tab count is updated`() {
         assertEquals(0, settings.openTabsCount)
+        assertFalse(Metrics.hasOpenTabs.testHasValue())
+
         store.dispatch(
             TabListAction.AddMultipleTabsAction(
                 listOf(
@@ -104,11 +111,15 @@ class TelemetryMiddlewareTest {
         ).joinBlocking()
 
         assertEquals(2, settings.openTabsCount)
-        verify(exactly = 1) { metrics.track(Event.HaveOpenTabs) }
+
+        assertTrue(Metrics.hasOpenTabs.testHasValue())
+        assertTrue(Metrics.hasOpenTabs.testGetValue())
     }
 
     @Test
     fun `WHEN a tab is removed THEN the open tab count is updated`() {
+        assertFalse(Metrics.hasOpenTabs.testHasValue())
+
         store.dispatch(
             TabListAction.AddMultipleTabsAction(
                 listOf(
@@ -118,15 +129,18 @@ class TelemetryMiddlewareTest {
             )
         ).joinBlocking()
         assertEquals(2, settings.openTabsCount)
-        verify(exactly = 1) { metrics.track(Event.HaveOpenTabs) }
 
         store.dispatch(TabListAction.RemoveTabAction("1")).joinBlocking()
         assertEquals(1, settings.openTabsCount)
-        verify(exactly = 2) { metrics.track(Event.HaveOpenTabs) }
+
+        assertTrue(Metrics.hasOpenTabs.testHasValue())
+        assertTrue(Metrics.hasOpenTabs.testGetValue())
     }
 
     @Test
     fun `WHEN all tabs are removed THEN the open tab count is updated`() {
+        assertFalse(Metrics.hasOpenTabs.testHasValue())
+
         store.dispatch(
             TabListAction.AddMultipleTabsAction(
                 listOf(
@@ -136,15 +150,21 @@ class TelemetryMiddlewareTest {
             )
         ).joinBlocking()
         assertEquals(2, settings.openTabsCount)
-        verify(exactly = 1) { metrics.track(Event.HaveOpenTabs) }
+
+        assertTrue(Metrics.hasOpenTabs.testHasValue())
+        assertTrue(Metrics.hasOpenTabs.testGetValue())
 
         store.dispatch(TabListAction.RemoveAllTabsAction()).joinBlocking()
         assertEquals(0, settings.openTabsCount)
-        verify(exactly = 1) { metrics.track(Event.HaveNoOpenTabs) }
+
+        assertTrue(Metrics.hasOpenTabs.testHasValue())
+        assertFalse(Metrics.hasOpenTabs.testGetValue())
     }
 
     @Test
     fun `WHEN all normal tabs are removed THEN the open tab count is updated`() {
+        assertFalse(Metrics.hasOpenTabs.testHasValue())
+
         store.dispatch(
             TabListAction.AddMultipleTabsAction(
                 listOf(
@@ -155,16 +175,20 @@ class TelemetryMiddlewareTest {
             )
         ).joinBlocking()
         assertEquals(2, settings.openTabsCount)
-        verify(exactly = 1) { metrics.track(Event.HaveOpenTabs) }
+        assertTrue(Metrics.hasOpenTabs.testHasValue())
+        assertTrue(Metrics.hasOpenTabs.testGetValue())
 
         store.dispatch(TabListAction.RemoveAllNormalTabsAction).joinBlocking()
         assertEquals(0, settings.openTabsCount)
-        verify(exactly = 1) { metrics.track(Event.HaveNoOpenTabs) }
+        assertTrue(Metrics.hasOpenTabs.testHasValue())
+        assertFalse(Metrics.hasOpenTabs.testGetValue())
     }
 
     @Test
     fun `WHEN tabs are restored THEN the open tab count is updated`() {
         assertEquals(0, settings.openTabsCount)
+        assertFalse(Metrics.hasOpenTabs.testHasValue())
+
         val tabsToRestore = listOf(
             RecoverableTab(null, TabState(url = "https://mozilla.org", id = "1")),
             RecoverableTab(null, TabState(url = "https://firefox.com", id = "2"))
@@ -177,7 +201,9 @@ class TelemetryMiddlewareTest {
             )
         ).joinBlocking()
         assertEquals(2, settings.openTabsCount)
-        verify(exactly = 1) { metrics.track(Event.HaveOpenTabs) }
+
+        assertTrue(Metrics.hasOpenTabs.testHasValue())
+        assertTrue(Metrics.hasOpenTabs.testGetValue())
     }
 
     @Test