Skip to content

Commit

Permalink
[Android] Create HistogramWatcher to simplify UMA asserts in tests
Browse files Browse the repository at this point in the history
The extra functionality over HistogramDelta are:
- Manage multiple histograms with fewer objects
- Work with total counts
- expectNoFurtherRecords() and
  expectNoFurtherRecordsForHistogramsAbove()

I will migrate usages of HistogramDelta to this. I'll add a convenience factory method to watch one single histogram in a follow-up since it is a common use case.

Bug: 1412573
Change-Id: I2b306b5a73e12bf10efdd2879ebd7c78e9d5e8ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4214659
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1101175}
  • Loading branch information
Henrique Nakashima authored and Chromium LUCI CQ committed Feb 3, 2023
1 parent a1cc370 commit 04f8a92
Show file tree
Hide file tree
Showing 4 changed files with 277 additions and 114 deletions.
1 change: 1 addition & 0 deletions base/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -4516,6 +4516,7 @@ if (is_android) {
"test/android/javatests/src/org/chromium/base/test/util/Feature.java",
"test/android/javatests/src/org/chromium/base/test/util/Features.java",
"test/android/javatests/src/org/chromium/base/test/util/FeaturesBase.java",
"test/android/javatests/src/org/chromium/base/test/util/HistogramWatcher.java",
"test/android/javatests/src/org/chromium/base/test/util/InMemorySharedPreferences.java",
"test/android/javatests/src/org/chromium/base/test/util/InMemorySharedPreferencesContext.java",
"test/android/javatests/src/org/chromium/base/test/util/InstrumentationUtils.java",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,226 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package org.chromium.base.test.util;

import static org.junit.Assert.fail;

import androidx.annotation.Nullable;

import org.chromium.base.metrics.RecordHistogram;

import java.util.HashMap;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;

/**
* Watches a number of histograms in tests to assert later that the expected values were recorded.
*
* Uses the delta of records between startWatching() and assertExpectedHistogramsRecorded(), so
* that records logged in previous tests (in batched tests) don't interfere with the counting.
*
* Usage:
*
* HistogramWatcher histogramWatcher = HistogramWatcher.newBuilder()
* .expectIntRecord("Histogram1", 555)
* .expectIntRecord("Histogram1", 666)
* .expectBooleanRecord("Histogram2", true)
* .expectAnyRecord("Histogram3")
* .build();
*
* // Act
* [code under test that is expected to record the histograms above]
*
* // Assert
* histogramWatcher.assertExpectedHistogramsRecorded();
*/
public class HistogramWatcher {
/**
* Create a new {@link HistogramWatcher.Builder} to instantiate {@link HistogramWatcher}.
*/
public static HistogramWatcher.Builder newBuilder() {
return new HistogramWatcher.Builder();
}

/**
* Builder for {@link HistogramWatcher}. Use to list the expectations of records.
*/
public static class Builder {
private final Map<HistogramAndValue, Integer> mRecordsExpected = new HashMap<>();
private final Map<String, Integer> mTotalRecordsExpected = new HashMap<>();

/**
* Use {@link HistogramWatcher#newBuilder()} to instantiate.
*/
private Builder() {}

/**
* Build the {@link HistogramWatcher} and snapshot current number of records of the expected
* histograms to calculate the delta later.
*/
public HistogramWatcher build() {
return new HistogramWatcher(mRecordsExpected, mTotalRecordsExpected);
}

/**
* Add an expectation that {@code histogram} will be recorded once with a boolean {@code
* value}.
*/
public Builder expectBooleanRecord(String histogram, boolean value) {
return expectBooleanRecords(histogram, value, 1);
}

/**
* Add an expectation that {@code histogram} will be recorded a number of {@code times} with
* a boolean {@code value}.
*/
public Builder expectBooleanRecords(String histogram, boolean value, int times) {
return expectIntRecords(histogram, value ? 1 : 0, times);
}

/**
* Add an expectation that {@code histogram} will be recorded once with an int {@code
* value}.
*/
public Builder expectIntRecord(String histogram, int value) {
return expectIntRecords(histogram, value, 1);
}

/**
* Add an expectation that {@code histogram} will be recorded a number of {@code times} with
* an int {@code value}.
*/
public Builder expectIntRecords(String histogram, int value, int times) {
HistogramAndValue histogramAndValue = new HistogramAndValue(histogram, value);
incrementRecordsExpected(histogramAndValue, times);
incrementTotalRecordsExpected(histogram, times);
return this;
}

/**
* Add an expectation that {@code histogram} will be recorded once with any value.
*/
public Builder expectAnyRecord(String histogram) {
return expectAnyRecords(histogram, 1);
}

/**
* Add an expectation that {@code histogram} will be recorded a number of {@code times} with
* any values.
*/
public Builder expectAnyRecords(String histogram, int times) {
incrementTotalRecordsExpected(histogram, times);
return this;
}

/**
* Add an expectation that {@code histogram} will not be recorded with any values.
*/
public Builder expectNoRecords(String histogram) {
if (mTotalRecordsExpected.getOrDefault(histogram, 0) != 0) {
throw new IllegalStateException(
"Cannot expect no records but also expect records in previous calls.");
}

mTotalRecordsExpected.put(histogram, 0);
return this;
}

private void incrementRecordsExpected(HistogramAndValue histogramAndValue, int increase) {
int previousCountExpected = mRecordsExpected.getOrDefault(histogramAndValue, 0);
mRecordsExpected.put(histogramAndValue, previousCountExpected + increase);
}

private void incrementTotalRecordsExpected(String histogram, int increase) {
int previousCountExpected = mTotalRecordsExpected.getOrDefault(histogram, 0);
mTotalRecordsExpected.put(histogram, previousCountExpected + increase);
}
}

private final Map<HistogramAndValue, Integer> mRecordsExpected;
private final Map<String, Integer> mTotalRecordsExpected;

private final Map<HistogramAndValue, Integer> mStartingCounts = new HashMap<>();
private final Map<String, Integer> mStartingTotalCounts = new HashMap<>();

private HistogramWatcher(Map<HistogramAndValue, Integer> recordsExpected,
Map<String, Integer> totalRecordsExpected) {
mRecordsExpected = recordsExpected;
mTotalRecordsExpected = totalRecordsExpected;

takeSnapshot();
}

private void takeSnapshot() {
for (HistogramAndValue histogramAndValue : mRecordsExpected.keySet()) {
int currentCount = histogramAndValue.getHistogramValueCountForTesting();
mStartingCounts.put(histogramAndValue, currentCount);
}
for (String histogram : mTotalRecordsExpected.keySet()) {
int currentCount = RecordHistogram.getHistogramTotalCountForTesting(histogram);
mStartingTotalCounts.put(histogram, currentCount);
}
}

/**
* Assert that the watched histograms were recorded as expected.
*/
public void assertExpected() {
for (Entry<HistogramAndValue, Integer> kv : mRecordsExpected.entrySet()) {
HistogramAndValue histogramAndValue = kv.getKey();
int expectedDelta = kv.getValue();

int actualFinalCount = histogramAndValue.getHistogramValueCountForTesting();
int actualDelta = actualFinalCount - mStartingCounts.get(histogramAndValue);

if (expectedDelta != actualDelta) {
fail(String.format(
"Expected delta of <%d record(s)> of histogram \"%s\" with value [%d], "
+ "but saw a delta of <%d record(s)> in that value's bucket.",
expectedDelta, histogramAndValue.mHistogram, histogramAndValue.mValue,
actualDelta));
}
}

for (Entry<String, Integer> kv : mTotalRecordsExpected.entrySet()) {
String histogram = kv.getKey();
int expectedDelta = kv.getValue();

int actualFinalCount = RecordHistogram.getHistogramTotalCountForTesting(histogram);
int actualDelta = actualFinalCount - mStartingTotalCounts.get(histogram);

if (expectedDelta != actualDelta) {
fail(String.format(
"Expected delta of <%d total record(s)> of histogram \"%s\", but saw a "
+ "delta of <%d total record(s)>.",
expectedDelta, histogram, actualDelta));
}
}
}

private static class HistogramAndValue {
private final String mHistogram;
private final int mValue;

private HistogramAndValue(String histogram, int value) {
mHistogram = histogram;
mValue = value;
}

@Override
public int hashCode() {
return Objects.hash(mHistogram, mValue);
}

@Override
public boolean equals(@Nullable Object obj) {
return Objects.equals(this, obj);
}

private int getHistogramValueCountForTesting() {
return RecordHistogram.getHistogramValueCountForTesting(mHistogram, mValue);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,13 @@
*/
public class MetricsUtils {
/**
*
* Helper class that snapshots the given bucket of the given UMA histogram on its creation,
* allowing to inspect the number of samples recorded during its lifetime.
*
* @deprecated Use {@link HistogramWatcher} instead.
*/
@Deprecated
public static class HistogramDelta {
private final String mHistogram;
private final int mSampleValue;
Expand Down

0 comments on commit 04f8a92

Please sign in to comment.