Skip to content

Commit 7cc04c5

Browse files
committed
code review
1 parent f206a7b commit 7cc04c5

File tree

3 files changed

+12
-16
lines changed

3 files changed

+12
-16
lines changed

firebase-perf/src/main/java/com/google/firebase/perf/session/gauges/GaugeCounter.kt

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
package com.google.firebase.perf.session.gauges
1616

17+
import androidx.annotation.VisibleForTesting
1718
import com.google.firebase.perf.logging.AndroidLogger
1819
import java.util.concurrent.atomic.AtomicInteger
1920

@@ -25,7 +26,9 @@ object GaugeCounter {
2526
private const val MAX_METRIC_COUNT = 25
2627
private val counter = AtomicInteger(0)
2728
private val logger = AndroidLogger.getInstance()
28-
// TODO(b/394127311): Setting this as a var for a unit test. Refactor it.
29+
30+
@set:VisibleForTesting(otherwise = VisibleForTesting.NONE)
31+
@set:JvmStatic
2932
var gaugeManager: GaugeManager = GaugeManager.getInstance()
3033

3134
fun incrementCounter() {
@@ -43,11 +46,10 @@ object GaugeCounter {
4346
logger.debug("Decremented logger to $curr")
4447
}
4548

46-
// TODO: Add annotation to only call from tests
49+
@VisibleForTesting(otherwise = VisibleForTesting.NONE)
4750
fun resetCounter() {
4851
counter.set(0)
4952
}
5053

51-
// TODO: Add annotation to only call from tests
52-
fun count(): Int = counter.get()
54+
@VisibleForTesting(otherwise = VisibleForTesting.NONE) fun count(): Int = counter.get()
5355
}

firebase-perf/src/test/java/com/google/firebase/perf/FirebasePerformanceTestBase.java

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,20 +25,13 @@
2525
import com.google.firebase.perf.config.ConfigResolver;
2626
import com.google.firebase.perf.session.PerfSession;
2727
import com.google.firebase.perf.session.SessionManager;
28-
import com.google.firebase.perf.session.gauges.GaugeCounter;
2928
import com.google.firebase.perf.util.ImmutableBundle;
3029
import org.junit.After;
3130
import org.junit.Before;
32-
import org.junit.BeforeClass;
3331
import org.robolectric.shadows.ShadowLog;
3432
import org.robolectric.shadows.ShadowPackageManager;
3533

3634
public class FirebasePerformanceTestBase {
37-
@BeforeClass
38-
public static void setUpBeforeClass() {
39-
GaugeCounter.INSTANCE.resetCounter();
40-
}
41-
4235
/**
4336
* The following values are needed by Firebase to identify the project and application that all
4437
* data stored in Firebase databases gets associated with. This is important to determine data

firebase-perf/src/test/java/com/google/firebase/perf/session/gauges/GaugeManagerTest.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ public void setUp() {
130130
@After
131131
public void tearDown() {
132132
shadowOf(Looper.getMainLooper()).idle();
133+
GaugeCounter.INSTANCE.resetCounter();
133134
}
134135

135136
@Test
@@ -331,11 +332,11 @@ public void stopCollectingGauges_invalidGaugeCollectionFrequency_appInForeground
331332
}
332333

333334
@Test
334-
public void testGaugeCounterStartsAJobToConsumeTheGeneratedMetrics() throws InterruptedException {
335+
public void testGaugeCounterStartsAJobToConsumeTheGeneratedMetrics() {
335336
PerfSession fakeSession = createTestSession(1);
336337
testGaugeManager.setApplicationProcessState(ApplicationProcessState.FOREGROUND);
337338
testGaugeManager.startCollectingGauges(fakeSession);
338-
GaugeCounter.INSTANCE.setGaugeManager(testGaugeManager);
339+
GaugeCounter.setGaugeManager(testGaugeManager);
339340

340341
// There's no job to log the gauges.
341342
assertThat(fakeScheduledExecutorService.isEmpty()).isTrue();
@@ -377,7 +378,7 @@ public void testUpdateAppStateHandlesMultipleAppStates() {
377378
fakeSession.setGaugeAndEventCollectionEnabled(true);
378379
testGaugeManager.setApplicationProcessState(ApplicationProcessState.FOREGROUND);
379380
testGaugeManager.startCollectingGauges(fakeSession);
380-
GaugeCounter.INSTANCE.setGaugeManager(testGaugeManager);
381+
GaugeCounter.setGaugeManager(testGaugeManager);
381382

382383
// Generate metrics that don't exceed the GaugeCounter.MAX_COUNT.
383384
generateMetricsAndIncrementCounter(10);
@@ -429,7 +430,7 @@ public void testGaugeManagerHandlesMultipleSessionIds() {
429430
fakeSession.setGaugeAndEventCollectionEnabled(true);
430431
testGaugeManager.setApplicationProcessState(ApplicationProcessState.BACKGROUND);
431432
testGaugeManager.startCollectingGauges(fakeSession);
432-
GaugeCounter.INSTANCE.setGaugeManager(testGaugeManager);
433+
GaugeCounter.setGaugeManager(testGaugeManager);
433434

434435
// Generate metrics that don't exceed the GaugeCounter.MAX_COUNT.
435436
generateMetricsAndIncrementCounter(10);
@@ -519,7 +520,7 @@ public void testStopCollectingGaugesCreatesOneLastJobToConsumeAnyPendingMetrics(
519520
+ recordedGaugeMetric.getCpuMetricReadingsCount();
520521
assertThat(recordedGaugeMetricsCount).isEqualTo(2);
521522

522-
// TODO(b/394127311): Investigate why this isn't 0 on local runs.
523+
// TODO(b/394127311): Investigate why this isn't 0.
523524
// assertThat(GaugeCounter.INSTANCE.count()).isEqualTo(0);
524525
}
525526

0 commit comments

Comments
 (0)