Merge #4786

4786: Fix the Glean instrumented tests r=Dexterp37 a=Dexterp37

The tests were mistakenly re-initializing Glean when running each test, in order to send pings to
a local pingserver. However this is problematic as instrumented tests run as a separate thread in the
application process and Glean now asserts if it is initialized off the main thread.
Co-authored-by: Alessio Placitelli <alessio.placitelli@gmail.com>

components/service/glean/src/main/java/mozilla/components/service/glean/Glean.kt

@@ -631,6 +631,26 @@ open class GleanInternalAPI internal constructor () {
         Glean.setUploadEnabled(true)
         Glean.initialize(context, config)
     }
+
+    /**
+     * TEST ONLY FUNCTION.
+     * Sets the server endpoint to a local address for ingesting test pings.
+     *
+     * The endpoint will be set as "http://localhost:<port>".
+     *
+     * @param port the local address to send pings to
+     */
+    @VisibleForTesting(otherwise = VisibleForTesting.NONE)
+    internal fun testSetLocalEndpoint(port: Int) {
+        Glean.enableTestingMode()
+
+        // We can't set the configuration unless we're initialized.
+        assert(isInitialized())
+
+        val endpointUrl = "http://localhost:$port"
+
+        Glean.configuration = configuration.copy(serverEndpoint = endpointUrl)
+    }
 }
 
 @SuppressLint("StaticFieldLeak")

components/service/glean/src/main/java/mozilla/components/service/glean/testing/GleanTestLocalServer.kt

+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package mozilla.components.service.glean.testing
+
+import androidx.annotation.VisibleForTesting
+import mozilla.components.service.glean.Glean
+import org.junit.rules.TestWatcher
+import org.junit.runner.Description
+
+/**
+ * This implements a JUnit rule for writing tests for Glean SDK metrics.
+ *
+ * The rule takes care of sending Glean SDK pings to a local server, at the
+ * address: "http://localhost:<port>".
+ *
+ * This is useful for Android instrumented tests, where we don't want to
+ * initialize Glean more than once but still want to send pings to a local
+ * server for validation.
+ *
+ * Example usage:
+ *
+ * ```
+ * // Add the following lines to you test class.
+ * @get:Rule
+ * val gleanRule = GleanTestLocalServer(3785)
+ * ```
+ *
+ * @param localPort the port of the local ping server
+ */
+@VisibleForTesting(otherwise = VisibleForTesting.NONE)
+class GleanTestLocalServer(
+    private val localPort: Int
+) : TestWatcher() {
+    override fun starting(description: Description?) {
+        Glean.testSetLocalEndpoint(localPort)
+    }
+}

samples/glean/src/androidTest/java/org/mozilla/samples/glean/GleanTestRunner.kt

@@ -38,14 +38,14 @@ internal fun getPingServer(): MockWebServer {
 }
 
 /**
- * Returns the address the local ping server is listening to.
+ * Returns the port the local ping server is listening to.
  *
- * @return a `String` containing the server address.
+ * @return an `Int` containing the server port.
  */
 @VisibleForTesting(otherwise = VisibleForTesting.NONE)
-internal fun getPingServerAddress(): String {
+internal fun getPingServerPort(): Int {
     val testRunner: GleanTestRunner = InstrumentationRegistry.getInstrumentation() as GleanTestRunner
-    return testRunner.pingServerAddress!!
+    return testRunner.pingServerPort!!
 }
 
 /**
@@ -57,7 +57,7 @@ class GleanTestRunner : AndroidJUnitRunner() {
     // Add a lazy ping server to the app runner. This is only initialized once
     // since the `Application` object is re-used.
     internal val pingServer: MockWebServer by lazy { createMockWebServer() }
-    internal var pingServerAddress: String? = null
+    internal var pingServerPort: Int? = null
 
     init {
         // We need to start the server off the main thread, otherwise
@@ -65,7 +65,7 @@ class GleanTestRunner : AndroidJUnitRunner() {
         // a thread and joining seems fine.
         val thread = Thread {
             pingServer.start()
-            pingServerAddress = "http://${pingServer.hostName}:${pingServer.port}"
+            pingServerPort = pingServer.port
         }
         thread.start()
         thread.join()

samples/glean/src/androidTest/java/org/mozilla/samples/glean/MainActivityTest.kt

@@ -4,14 +4,11 @@
 
 package org.mozilla.samples.glean
 
-import androidx.test.core.app.ApplicationProvider
 import androidx.test.espresso.Espresso.onView
 import androidx.test.espresso.action.ViewActions.click
 import androidx.test.espresso.matcher.ViewMatchers.withId
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.rule.ActivityTestRule
-import mozilla.components.service.glean.config.Configuration
-import mozilla.components.service.glean.testing.GleanTestRule
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
 import org.mozilla.samples.glean.GleanMetrics.Test as GleanTestMetrics
@@ -25,26 +22,22 @@ class MainActivityTest {
     @get:Rule
     val activityRule: ActivityTestRule<MainActivity> = ActivityTestRule(MainActivity::class.java)
 
-    @get:Rule
-    val gleanRule = GleanTestRule(
-        ApplicationProvider.getApplicationContext(),
-        Configuration(serverEndpoint = getPingServerAddress())
-    )
-
     @Test
     fun checkGleanClickData() {
+        // We don't reset the storage in this test as the GleanTestRule does not
+        // work nicely in instrumented test. Just check the current value, increment
+        // by one and make it the expected value.
+        val expectedValue = if (GleanTestMetrics.counter.testHasValue()) {
+            GleanTestMetrics.counter.testGetValue() + 1
+        } else {
+            1
+        }
+
         // Simulate a click on the button.
         onView(withId(R.id.buttonGenerateData)).perform(click())
 
         // Use the Glean testing API to check if the expected data was recorded.
         assertTrue(GleanTestMetrics.counter.testHasValue())
-        assertEquals(1, GleanTestMetrics.counter.testGetValue())
-    }
-
-    @Test
-    fun checkGleanClickDataAgain() {
-        // Repeat the same test as above: this will fail if the GleanTestRule fails to
-        // clear the Glean SDK state.
-        checkGleanClickData()
+        assertEquals(expectedValue, GleanTestMetrics.counter.testGetValue())
     }
 }

samples/glean/src/androidTest/java/org/mozilla/samples/glean/pings/BaselinePingTest.kt

@@ -22,13 +22,12 @@ import androidx.work.WorkManager
 import androidx.work.testing.WorkManagerTestInitHelper
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.withTimeout
-import mozilla.components.service.glean.config.Configuration
-import mozilla.components.service.glean.testing.GleanTestRule
+import mozilla.components.service.glean.testing.GleanTestLocalServer
 import org.json.JSONObject
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertTrue
 import org.junit.Before
-import org.mozilla.samples.glean.getPingServerAddress
+import org.mozilla.samples.glean.getPingServerPort
 import java.util.concurrent.TimeUnit
 
 @RunWith(AndroidJUnit4::class)
@@ -37,10 +36,7 @@ class BaselinePingTest {
     val activityRule: ActivityTestRule<MainActivity> = ActivityTestRule(MainActivity::class.java)
 
     @get:Rule
-    val gleanRule = GleanTestRule(
-        ApplicationProvider.getApplicationContext(),
-        Configuration(serverEndpoint = getPingServerAddress())
-    )
+    val gleanRule = GleanTestLocalServer(getPingServerPort())
 
     private val context: Context
         get() = ApplicationProvider.getApplicationContext()