Set a continuous reporting spec for MetricsReporterTest#SampleRate* tests
The tests SampleRateEnable50 and SampleRateEnableAll have sometimes
been flaky because of the ShouldContinueReporting() assertion being
false when it was supposed to be true. Looking at the Boolean conditions
that compose the return value of that method, one that could turn out
to be false is the one related to the report interval index being still
within the period spec size (e.g., if the test for any reason ends up
taking more than one second before reaching that assertion, it could be
the case that the background thread has reported metrics and moved the
index outside the period spec). Considering these tests are exercising
the logic around the session_id and the reporting_mods values (and not
the reporting period itself), changing the period spec to be repeating
should remove one possible factor of flakiness for these tests. Also,
this commit fixes a few typos and incorrect statements in comments.
Bug: 198418961
Test: atest art_runtime_tests
Change-Id: I30cc0bccbd5e144542672b3deba6aca21d9f855c
diff --git a/runtime/metrics/reporter_test.cc b/runtime/metrics/reporter_test.cc
index 7d9377a..62131d8 100644
--- a/runtime/metrics/reporter_test.cc
+++ b/runtime/metrics/reporter_test.cc
@@ -220,7 +220,7 @@
WaitForReport(/*report_count=*/ 1, /*sleep_period_ms=*/ 50);
VerifyReports(/*size=*/ 1, /*with_metrics*/ true);
- // We still should not report at period.
+ // We should still not report continuously.
ASSERT_FALSE(ShouldContinueReporting());
}
@@ -241,11 +241,11 @@
WaitForReport(/*report_count=*/ 2, /*sleep_period_ms=*/ 500);
VerifyReports(/*size=*/ 2, /*with_metrics*/ true);
- // We should not longer report at period.
+ // We should no longer report continuously.
ASSERT_FALSE(ShouldContinueReporting());
}
-// LARGE TEST: This takes take 2s to run.
+// LARGE TEST: This test take 2s to run.
// Verifies startup reporting, followed by continuous reporting.
TEST_F(MetricsReporterTest, StartupAndPeriodContinuous) {
SetupReporter("S,1,*");
@@ -262,7 +262,7 @@
WaitForReport(/*report_count=*/ 3, /*sleep_period_ms=*/ 500);
VerifyReports(/*size=*/ 3, /*with_metrics*/ true);
- // We should keep reporting at period.
+ // We should keep reporting continuously.
ASSERT_TRUE(ShouldContinueReporting());
}
@@ -282,11 +282,11 @@
WaitForReport(/*report_count=*/ 1, /*sleep_period_ms=*/ 500);
VerifyReports(/*size=*/ 1, /*with_metrics*/ true);
- // We should not longer report at period.
+ // We should no longer report continuously.
ASSERT_FALSE(ShouldContinueReporting());
}
-// LARGE TEST: This takes take 5s to run.
+// LARGE TEST: This test takes 5s to run.
// Verifies a sequence of reporting, at different interval of times.
TEST_F(MetricsReporterTest, PeriodContinuous) {
SetupReporter("1,2,*");
@@ -303,7 +303,7 @@
WaitForReport(/*report_count=*/ 3, /*sleep_period_ms=*/ 500);
VerifyReports(/*size=*/ 3, /*with_metrics*/ true);
- // We should keep reporting at period.
+ // We should keep reporting continuously.
ASSERT_TRUE(ShouldContinueReporting());
}
@@ -323,13 +323,13 @@
WaitForReport(/*report_count=*/ 1, /*sleep_period_ms=*/ 500);
VerifyReports(/*size=*/ 1, /*with_metrics*/ false);
- // We should not longer report at period.
+ // We should no longer report continuously.
ASSERT_FALSE(ShouldContinueReporting());
}
// Verify we don't start reporting if the sample rate is set to 0.
TEST_F(MetricsReporterTest, SampleRateDisable) {
- SetupReporter("1", /*session_id=*/ 1, /*reporting_mods=*/ 0);
+ SetupReporter("1,*", /*session_id=*/ 1, /*reporting_mods=*/ 0);
// The background thread should not start.
ASSERT_FALSE(MaybeStartBackgroundThread(/*add_metrics=*/ false));
@@ -341,7 +341,7 @@
// Verify we don't start reporting if the sample rate is low and the session does
// not meet conditions.
TEST_F(MetricsReporterTest, SampleRateDisable24) {
- SetupReporter("1", /*session_id=*/ 125, /*reporting_mods=*/ 24);
+ SetupReporter("1,*", /*session_id=*/ 125, /*reporting_mods=*/ 24);
// The background thread should not start.
ASSERT_FALSE(MaybeStartBackgroundThread(/*add_metrics=*/ false));
@@ -353,9 +353,9 @@
// Verify we start reporting if the sample rate and the session meet
// reporting conditions
TEST_F(MetricsReporterTest, SampleRateEnable50) {
- SetupReporter("1", /*session_id=*/ 125, /*reporting_mods=*/ 50);
+ SetupReporter("1,*", /*session_id=*/ 125, /*reporting_mods=*/ 50);
- // The background thread should not start.
+ // The background thread should start.
ASSERT_TRUE(MaybeStartBackgroundThread(/*add_metrics=*/ false));
ASSERT_FALSE(ShouldReportAtStartup());
@@ -365,7 +365,7 @@
// Verify we start reporting if the sample rate and the session meet
// reporting conditions
TEST_F(MetricsReporterTest, SampleRateEnableAll) {
- SetupReporter("1", /*session_id=*/ 1099, /*reporting_mods=*/ 100);
+ SetupReporter("1,*", /*session_id=*/ 1099, /*reporting_mods=*/ 100);
// The background thread should start.
ASSERT_TRUE(MaybeStartBackgroundThread(/*add_metrics=*/ false));