In LoggerTestUtil rely on ShadowRecordHistogram

This change reduces dependency on RecordHistogram.Natives and unblocks
its refactoring in
https://chromium-review.googlesource.com/c/chromium/src/+/1915193

Change-Id: I5b532083c22b94298dc087b8515709be787dbd79
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1928377Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Commit-Queue: bttk <bttk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718806}

base/android/junit/src/org/chromium/base/metrics/test/ShadowRecordHistogram.java

@@ -14,6 +14,7 @@ import org.chromium.base.metrics.CachedMetrics;
 import org.chromium.base.metrics.RecordHistogram;
 
 import java.util.HashMap;
+import java.util.Map;
 
 /**
  * Implementation of RecordHistogram which does not rely on native and still enables testing of
@@ -21,49 +22,50 @@ import java.util.HashMap;
  */
 @Implements(RecordHistogram.class)
 public class ShadowRecordHistogram {
-    private static HashMap<Pair<String, Integer>, Integer> sSamples =
-            new HashMap<Pair<String, Integer>, Integer>();
+    private static final Map<Pair<String, Integer>, Integer> sSamples = new HashMap<>();
+    private static final Map<String, Integer> sTotals = new HashMap<>();
 
     @Resetter
     public static void reset() {
         sSamples.clear();
+        sTotals.clear();
     }
 
     @Implementation
     public static void recordBooleanHistogram(String name, boolean sample) {
         Pair<String, Integer> key = Pair.create(name, sample ? 1 : 0);
-        incrementSampleCount(key);
+        recordSample(key);
     }
 
     @Implementation
     public static void recordCountHistogram(String name, int sample) {
         Pair<String, Integer> key = Pair.create(name, sample);
-        incrementSampleCount(key);
+        recordSample(key);
     }
 
     @Implementation
     public static void recordCount100Histogram(String name, int sample) {
         Pair<String, Integer> key = Pair.create(name, sample);
-        incrementSampleCount(key);
+        recordSample(key);
     }
 
     @Implementation
     public static void recordCustomCountHistogram(
             String name, int sample, int min, int max, int numBuckets) {
         Pair<String, Integer> key = Pair.create(name, sample);
-        incrementSampleCount(key);
+        recordSample(key);
     }
 
     @Implementation
     public static void recordEnumeratedHistogram(String name, int sample, int boundary) {
         assert sample < boundary : "Sample " + sample + " is not within boundary " + boundary + "!";
-        incrementSampleCount(Pair.create(name, sample));
+        recordSample(Pair.create(name, sample));
     }
 
     @Implementation
     public static void recordLongTimesHistogram100(String name, long durationMs) {
         Pair<String, Integer> key = Pair.create(name, (int) durationMs);
-        incrementSampleCount(key);
+        recordSample(key);
     }
 
     @Implementation
@@ -73,11 +75,24 @@ public class ShadowRecordHistogram {
         return (i != null) ? i : 0;
     }
 
-    private static void incrementSampleCount(Pair<String, Integer> key) {
-        Integer i = sSamples.get(key);
-        if (i == null) {
-            i = 0;
+    @Implementation
+    public static int getHistogramTotalCountForTesting(String name) {
+        CachedMetrics.commitCachedMetrics();
+        Integer i = sTotals.get(name);
+        return (i != null) ? i : 0;
+    }
+
+    private static void recordSample(Pair<String, Integer> key) {
+        Integer bucketValue = sSamples.get(key);
+        if (bucketValue == null) {
+            bucketValue = 0;
+        }
+        sSamples.put(key, bucketValue + 1);
+
+        Integer totalCount = sTotals.get(key.first);
+        if (totalCount == null) {
+            totalCount = 0;
         }
-        sSamples.put(key, i + 1);
+        sTotals.put(key.first, totalCount + 1);
     }
 }

components/module_installer/android/junit/src/org/chromium/components/module_installer/logger/LoggerTestUtil.java

@@ -4,37 +4,40 @@
 
 package org.chromium.components.module_installer.logger;
 
-import static org.junit.Assert.assertEquals;
-import static org.mockito.Mockito.atLeast;
-import static org.mockito.Mockito.verify;
-
-import org.mockito.ArgumentCaptor;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.greaterThan;
 
 import org.chromium.base.metrics.CachedMetrics;
 import org.chromium.base.metrics.RecordHistogram;
 
 /**
  * Util class for supporting logger testing.
+ *
+ * TODO(bttk): remove in favor of a general purpose metrics test util
  */
 public class LoggerTestUtil {
-    public static int getHistogramStatus(
-            RecordHistogram.Natives mockHistogram, String expectedName, Integer expectedBoundary) {
-        ArgumentCaptor<String> name = ArgumentCaptor.forClass(String.class);
-        ArgumentCaptor<Long> key = ArgumentCaptor.forClass(Long.class);
-        ArgumentCaptor<Integer> sample = ArgumentCaptor.forClass(Integer.class);
-        ArgumentCaptor<Integer> boundary = ArgumentCaptor.forClass(Integer.class);
-
+    /**
+     * Asserts that enumerated histogram {@code name} has at least one sample.
+     *
+     * @param name Name of the enumerated histogram.
+     * @param boundary The smallest value not in the enumeration.
+     * @return The smallest recorded sample.
+     */
+    public static int getHistogramStatus(String name, int boundary) {
         // Make sure the metrics are flushed.
         // Needed by the EnumeratedHistogramSample but not for RecordHistogram.
         CachedMetrics.commitCachedMetrics();
 
-        verify(mockHistogram, atLeast(1))
-                .recordEnumeratedHistogram(
-                        name.capture(), key.capture(), sample.capture(), boundary.capture());
+        int sampleCount = RecordHistogram.getHistogramTotalCountForTesting(name);
+        assertThat(sampleCount, greaterThan(0));
 
-        assertEquals(expectedName, name.getValue());
-        assertEquals(expectedBoundary, boundary.getValue());
-
-        return sample.getValue();
+        for (int i = 0; i < boundary; i++) {
+            if (RecordHistogram.getHistogramValueCountForTesting(name, i) > 0) {
+                return i;
+            }
+        }
+        // There are samples only in the overflow bucket. More context:
+        // https://chromium.googlesource.com/chromium/src.git/+/HEAD/tools/metrics/histograms/README.md#count-histograms_choosing-min-and-max
+        return Integer.MAX_VALUE;
     }
 }

components/module_installer/android/junit/src/org/chromium/components/module_installer/logger/SplitInstallFailureLoggerTest.java

@@ -9,31 +9,22 @@ import static org.junit.Assert.assertEquals;
 import com.google.android.play.core.splitinstall.model.SplitInstallErrorCode;
 
 import org.junit.Before;
-import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.mockito.MockitoAnnotations;
+import org.robolectric.annotation.Config;
 
-import org.chromium.base.metrics.RecordHistogram;
-import org.chromium.base.metrics.RecordHistogramJni;
+import org.chromium.base.metrics.test.ShadowRecordHistogram;
 import org.chromium.base.test.BaseRobolectricTestRunner;
-import org.chromium.base.test.util.JniMocker;
 
 /**
  * Test suite for the SplitInstallFailureLogger class.
  */
 @RunWith(BaseRobolectricTestRunner.class)
+@Config(manifest = Config.NONE, shadows = {ShadowRecordHistogram.class})
 public class SplitInstallFailureLoggerTest {
-    @Mock
-    private RecordHistogram.Natives mRecordHistogramMock;
-
-    @Rule
-    public JniMocker mocker = new JniMocker();
-
     private SplitInstallFailureLogger mFailureLogger;
 
-    private int mErrorCodeMapping[][] = {
+    private final int mErrorCodeMapping[][] = {
             {4, SplitInstallErrorCode.ACCESS_DENIED},
             {5, SplitInstallErrorCode.ACTIVE_SESSIONS_LIMIT_EXCEEDED},
             {6, SplitInstallErrorCode.API_NOT_AVAILABLE},
@@ -53,10 +44,7 @@ public class SplitInstallFailureLoggerTest {
 
     @Before
     public void setUp() {
-        MockitoAnnotations.initMocks(this);
-
-        mocker.mock(RecordHistogramJni.TEST_HOOKS, mRecordHistogramMock);
-
+        ShadowRecordHistogram.reset();
         mFailureLogger = new SplitInstallFailureLogger();
     }
 
@@ -108,12 +96,14 @@ public class SplitInstallFailureLoggerTest {
 
         // Act & Assert.
         for (int[] tuple : mErrorCodeMapping) {
+            ShadowRecordHistogram.reset();
             int expectedOutputCode = tuple[0];
             int inputCode = tuple[1];
             mFailureLogger.logStatusFailure(moduleName, inputCode);
             assertEquals(expectedOutputCode, getHistogramStatus(moduleName));
         }
 
+        ShadowRecordHistogram.reset();
         mFailureLogger.logStatusFailure(moduleName, unknownCode);
         assertEquals(expectedUnknownCode, getHistogramStatus(moduleName));
     }
@@ -127,12 +117,14 @@ public class SplitInstallFailureLoggerTest {
 
         // Act & Assert.
         for (int[] tuple : mErrorCodeMapping) {
+            ShadowRecordHistogram.reset();
             int expectedOutputCode = tuple[0];
             int inputCode = tuple[1];
             mFailureLogger.logRequestFailure(moduleName, inputCode);
             assertEquals(expectedOutputCode, getHistogramStatus(moduleName));
         }
 
+        ShadowRecordHistogram.reset();
         mFailureLogger.logRequestFailure(moduleName, unknownCode);
         assertEquals(expectedUnknownCode, getHistogramStatus(moduleName));
     }
@@ -140,6 +132,6 @@ public class SplitInstallFailureLoggerTest {
     private int getHistogramStatus(String moduleName) {
         String expName = "Android.FeatureModules.InstallStatus." + moduleName;
         Integer expBoundary = 22;
-        return LoggerTestUtil.getHistogramStatus(mRecordHistogramMock, expName, expBoundary);
+        return LoggerTestUtil.getHistogramStatus(expName, expBoundary);
     }
 }

components/module_installer/android/junit/src/org/chromium/components/module_installer/logger/SplitInstallStatusLoggerTest.java

@@ -9,36 +9,24 @@ import static org.junit.Assert.assertEquals;
 import com.google.android.play.core.splitinstall.model.SplitInstallSessionStatus;
 
 import org.junit.Before;
-import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.mockito.MockitoAnnotations;
+import org.robolectric.annotation.Config;
 
-import org.chromium.base.metrics.RecordHistogram;
-import org.chromium.base.metrics.RecordHistogramJni;
+import org.chromium.base.metrics.test.ShadowRecordHistogram;
 import org.chromium.base.test.BaseRobolectricTestRunner;
-import org.chromium.base.test.util.JniMocker;
 
 /**
  * Test suite for the SplitInstallStatusLogger class.
  */
 @RunWith(BaseRobolectricTestRunner.class)
+@Config(manifest = Config.NONE, shadows = {ShadowRecordHistogram.class})
 public class SplitInstallStatusLoggerTest {
-    @Mock
-    private RecordHistogram.Natives mRecordHistogramMock;
-
-    @Rule
-    public JniMocker mocker = new JniMocker();
-
     private SplitInstallStatusLogger mStatusLogger;
 
     @Before
     public void setUp() {
-        MockitoAnnotations.initMocks(this);
-
-        mocker.mock(RecordHistogramJni.TEST_HOOKS, mRecordHistogramMock);
-
+        ShadowRecordHistogram.reset();
         mStatusLogger = new SplitInstallStatusLogger();
     }
 
@@ -89,6 +77,7 @@ public class SplitInstallStatusLoggerTest {
     }
 
     private int logStatusChange(String moduleName, int status) {
+        ShadowRecordHistogram.reset();
         mStatusLogger.logStatusChange(moduleName, status);
         return getHistogramStatus(moduleName);
     }
@@ -96,6 +85,6 @@ public class SplitInstallStatusLoggerTest {
     private int getHistogramStatus(String moduleName) {
         String expName = "Android.FeatureModules.InstallingStatus." + moduleName;
         Integer expBoundary = 12;
-        return LoggerTestUtil.getHistogramStatus(mRecordHistogramMock, expName, expBoundary);
+        return LoggerTestUtil.getHistogramStatus(expName, expBoundary);
     }
 }