From 539c1da98d88b8434085f2ef83beb990fcaa7733 Mon Sep 17 00:00:00 2001 From: Artem Titov Date: Tue, 20 Sep 2022 21:16:11 +0200 Subject: [PATCH] Rename Metric Units: kTimeMs->kMilliseconds and kSizeInBytes->kBytes Rename C++ API units to match new proto format units in metric.proto Bug: b/246095034 Change-Id: Ice5d388a080c474f275ef597f98fac1bcb98cf49 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/276100 Reviewed-by: Mirko Bonadei Commit-Queue: Artem Titov Cr-Commit-Position: refs/heads/main@{#38137} --- .../chrome_perf_dashboard_metrics_exporter.cc | 4 +- ...me_perf_dashboard_metrics_exporter_test.cc | 6 +-- api/test/metrics/metric.cc | 8 ++-- api/test/metrics/metric.h | 4 +- .../metrics_logger_and_exporter_test.cc | 44 ++++++++++--------- .../metrics_set_proto_file_exporter.cc | 4 +- .../metrics_set_proto_file_exporter_test.cc | 2 +- .../metrics/stdout_metrics_exporter_test.cc | 4 +- 8 files changed, 40 insertions(+), 36 deletions(-) diff --git a/api/test/metrics/chrome_perf_dashboard_metrics_exporter.cc b/api/test/metrics/chrome_perf_dashboard_metrics_exporter.cc index 0655c94edf..018d110b12 100644 --- a/api/test/metrics/chrome_perf_dashboard_metrics_exporter.cc +++ b/api/test/metrics/chrome_perf_dashboard_metrics_exporter.cc @@ -29,11 +29,11 @@ namespace { std::string ToChromePerfDashboardUnit(Unit unit) { switch (unit) { - case Unit::kTimeMs: + case Unit::kMilliseconds: return "msBestFitFormat"; case Unit::kPercent: return "n%"; - case Unit::kSizeInBytes: + case Unit::kBytes: return "sizeInBytes"; case Unit::kKilobitsPerSecond: // Chrome Perf Dashboard doesn't have kpbs units, so we change the unit diff --git a/api/test/metrics/chrome_perf_dashboard_metrics_exporter_test.cc b/api/test/metrics/chrome_perf_dashboard_metrics_exporter_test.cc index 89b14586ca..5d3136f49a 100644 --- a/api/test/metrics/chrome_perf_dashboard_metrics_exporter_test.cc +++ b/api/test/metrics/chrome_perf_dashboard_metrics_exporter_test.cc @@ -67,7 +67,7 @@ class ChromePerfDashboardMetricsExporterTest : public Test { TEST_F(ChromePerfDashboardMetricsExporterTest, ExportMetricFormatCorrect) { Metric metric1{ .name = "test_metric1", - .unit = Unit::kTimeMs, + .unit = Unit::kMilliseconds, .improvement_direction = ImprovementDirection::kBiggerIsBetter, .test_case = "test_case_name1", .metric_metadata = DefaultMetadata(), @@ -154,7 +154,7 @@ TEST_F(ChromePerfDashboardMetricsExporterTest, ExportMetricFormatCorrect) { TEST_F(ChromePerfDashboardMetricsExporterTest, ExportEmptyMetricExportsZeroValue) { Metric metric{.name = "test_metric", - .unit = Unit::kTimeMs, + .unit = Unit::kMilliseconds, .improvement_direction = ImprovementDirection::kBiggerIsBetter, .test_case = "test_case_name", .metric_metadata = DefaultMetadata(), @@ -184,7 +184,7 @@ TEST_F(ChromePerfDashboardMetricsExporterTest, TEST_F(ChromePerfDashboardMetricsExporterTest, ExportMetricWithOnlyStatsExportsMeanValues) { Metric metric{.name = "test_metric", - .unit = Unit::kTimeMs, + .unit = Unit::kMilliseconds, .improvement_direction = ImprovementDirection::kBiggerIsBetter, .test_case = "test_case_name", .metric_metadata = DefaultMetadata(), diff --git a/api/test/metrics/metric.cc b/api/test/metrics/metric.cc index 7bace78de7..3c30f36f49 100644 --- a/api/test/metrics/metric.cc +++ b/api/test/metrics/metric.cc @@ -16,12 +16,12 @@ namespace test { absl::string_view ToString(Unit unit) { switch (unit) { - case Unit::kTimeMs: - return "TimeMs"; + case Unit::kMilliseconds: + return "Milliseconds"; case Unit::kPercent: return "Percent"; - case Unit::kSizeInBytes: - return "SizeInBytes"; + case Unit::kBytes: + return "Bytes"; case Unit::kKilobitsPerSecond: return "KilobitsPerSecond"; case Unit::kHertz: diff --git a/api/test/metrics/metric.h b/api/test/metrics/metric.h index bce9e5089d..17c1755f95 100644 --- a/api/test/metrics/metric.h +++ b/api/test/metrics/metric.h @@ -22,9 +22,9 @@ namespace webrtc { namespace test { enum class Unit { - kTimeMs, + kMilliseconds, kPercent, - kSizeInBytes, + kBytes, kKilobitsPerSecond, kHertz, // General unitless value. Can be used either for dimensionless quantities diff --git a/api/test/metrics/metrics_logger_and_exporter_test.cc b/api/test/metrics/metrics_logger_and_exporter_test.cc index f984a125ac..a19a16203a 100644 --- a/api/test/metrics/metrics_logger_and_exporter_test.cc +++ b/api/test/metrics/metrics_logger_and_exporter_test.cc @@ -72,7 +72,8 @@ TEST(MetricsLoggerAndExporterTest, LogSingleValueMetricRecordsMetric) { std::move(exporters)); logger.LogSingleValueMetric( "metric_name", "test_case_name", - /*value=*/10, Unit::kTimeMs, ImprovementDirection::kBiggerIsBetter, + /*value=*/10, Unit::kMilliseconds, + ImprovementDirection::kBiggerIsBetter, std::map{{"key", "value"}}); } @@ -81,7 +82,7 @@ TEST(MetricsLoggerAndExporterTest, LogSingleValueMetricRecordsMetric) { const Metric& metric = metrics[0]; EXPECT_THAT(metric.name, Eq("metric_name")); EXPECT_THAT(metric.test_case, Eq("test_case_name")); - EXPECT_THAT(metric.unit, Eq(Unit::kTimeMs)); + EXPECT_THAT(metric.unit, Eq(Unit::kMilliseconds)); EXPECT_THAT(metric.improvement_direction, Eq(ImprovementDirection::kBiggerIsBetter)); EXPECT_THAT(metric.metric_metadata, @@ -116,8 +117,8 @@ TEST(MetricsLoggerAndExporterTest, .time = Clock::GetRealTimeClock()->CurrentTime(), .metadata = std::map{{"point_key2", "value2"}}}); - logger.LogMetric("metric_name", "test_case_name", values, Unit::kTimeMs, - ImprovementDirection::kBiggerIsBetter, + logger.LogMetric("metric_name", "test_case_name", values, + Unit::kMilliseconds, ImprovementDirection::kBiggerIsBetter, std::map{{"key", "value"}}); } @@ -126,7 +127,7 @@ TEST(MetricsLoggerAndExporterTest, const Metric& metric = metrics[0]; EXPECT_THAT(metric.name, Eq("metric_name")); EXPECT_THAT(metric.test_case, Eq("test_case_name")); - EXPECT_THAT(metric.unit, Eq(Unit::kTimeMs)); + EXPECT_THAT(metric.unit, Eq(Unit::kMilliseconds)); EXPECT_THAT(metric.improvement_direction, Eq(ImprovementDirection::kBiggerIsBetter)); EXPECT_THAT(metric.metric_metadata, @@ -153,7 +154,7 @@ TEST(MetricsLoggerAndExporterTest, LogMetricWithStatsRecordsMetric) { std::move(exporters)); Metric::Stats metric_stats{.mean = 15, .stddev = 5, .min = 10, .max = 20}; logger.LogMetric("metric_name", "test_case_name", metric_stats, - Unit::kTimeMs, ImprovementDirection::kBiggerIsBetter, + Unit::kMilliseconds, ImprovementDirection::kBiggerIsBetter, std::map{{"key", "value"}}); } @@ -162,7 +163,7 @@ TEST(MetricsLoggerAndExporterTest, LogMetricWithStatsRecordsMetric) { const Metric& metric = metrics[0]; EXPECT_THAT(metric.name, Eq("metric_name")); EXPECT_THAT(metric.test_case, Eq("test_case_name")); - EXPECT_THAT(metric.unit, Eq(Unit::kTimeMs)); + EXPECT_THAT(metric.unit, Eq(Unit::kMilliseconds)); EXPECT_THAT(metric.improvement_direction, Eq(ImprovementDirection::kBiggerIsBetter)); EXPECT_THAT(metric.metric_metadata, @@ -183,11 +184,11 @@ TEST(MetricsLoggerAndExporterTest, LogSingleValueMetricRecordsMultipleMetrics) { std::move(exporters)); logger.LogSingleValueMetric("metric_name1", "test_case_name1", - /*value=*/10, Unit::kTimeMs, + /*value=*/10, Unit::kMilliseconds, ImprovementDirection::kBiggerIsBetter, DefaultMetadata()); logger.LogSingleValueMetric("metric_name2", "test_case_name2", - /*value=*/10, Unit::kTimeMs, + /*value=*/10, Unit::kMilliseconds, ImprovementDirection::kBiggerIsBetter, DefaultMetadata()); } @@ -218,10 +219,12 @@ TEST(MetricsLoggerAndExporterTest, .time = Clock::GetRealTimeClock()->CurrentTime(), .metadata = DefaultMetadata()}); - logger.LogMetric("metric_name1", "test_case_name1", values, Unit::kTimeMs, - ImprovementDirection::kBiggerIsBetter, DefaultMetadata()); - logger.LogMetric("metric_name2", "test_case_name2", values, Unit::kTimeMs, - ImprovementDirection::kBiggerIsBetter, DefaultMetadata()); + logger.LogMetric("metric_name1", "test_case_name1", values, + Unit::kMilliseconds, ImprovementDirection::kBiggerIsBetter, + DefaultMetadata()); + logger.LogMetric("metric_name2", "test_case_name2", values, + Unit::kMilliseconds, ImprovementDirection::kBiggerIsBetter, + DefaultMetadata()); } std::vector metrics = exporter_factory.exported_metrics; @@ -242,10 +245,10 @@ TEST(MetricsLoggerAndExporterTest, LogMetricWithStatsRecordsMultipleMetrics) { Metric::Stats metric_stats{.mean = 15, .stddev = 5, .min = 10, .max = 20}; logger.LogMetric("metric_name1", "test_case_name1", metric_stats, - Unit::kTimeMs, ImprovementDirection::kBiggerIsBetter, + Unit::kMilliseconds, ImprovementDirection::kBiggerIsBetter, DefaultMetadata()); logger.LogMetric("metric_name2", "test_case_name2", metric_stats, - Unit::kTimeMs, ImprovementDirection::kBiggerIsBetter, + Unit::kMilliseconds, ImprovementDirection::kBiggerIsBetter, DefaultMetadata()); } @@ -277,13 +280,14 @@ TEST(MetricsLoggerAndExporterTest, Metric::Stats metric_stats{.mean = 15, .stddev = 5, .min = 10, .max = 20}; logger.LogSingleValueMetric("metric_name1", "test_case_name1", - /*value=*/10, Unit::kTimeMs, + /*value=*/10, Unit::kMilliseconds, ImprovementDirection::kBiggerIsBetter, DefaultMetadata()); - logger.LogMetric("metric_name2", "test_case_name2", values, Unit::kTimeMs, - ImprovementDirection::kBiggerIsBetter, DefaultMetadata()); + logger.LogMetric("metric_name2", "test_case_name2", values, + Unit::kMilliseconds, ImprovementDirection::kBiggerIsBetter, + DefaultMetadata()); logger.LogMetric("metric_name3", "test_case_name3", metric_stats, - Unit::kTimeMs, ImprovementDirection::kBiggerIsBetter, + Unit::kMilliseconds, ImprovementDirection::kBiggerIsBetter, DefaultMetadata()); } @@ -312,7 +316,7 @@ TEST(MetricsLoggerAndExporterTest, /*crash_on_export_failure=*/false); logger.LogSingleValueMetric("metric_name", "test_case_name", - /*value=*/10, Unit::kTimeMs, + /*value=*/10, Unit::kMilliseconds, ImprovementDirection::kBiggerIsBetter, DefaultMetadata()); } diff --git a/api/test/metrics/metrics_set_proto_file_exporter.cc b/api/test/metrics/metrics_set_proto_file_exporter.cc index 6a41098375..86e6f2e136 100644 --- a/api/test/metrics/metrics_set_proto_file_exporter.cc +++ b/api/test/metrics/metrics_set_proto_file_exporter.cc @@ -28,11 +28,11 @@ namespace { #if WEBRTC_ENABLE_PROTOBUF webrtc::test_metrics::Unit ToProtoUnit(Unit unit) { switch (unit) { - case Unit::kTimeMs: + case Unit::kMilliseconds: return webrtc::test_metrics::Unit::MILLISECONDS; case Unit::kPercent: return webrtc::test_metrics::Unit::PERCENT; - case Unit::kSizeInBytes: + case Unit::kBytes: return webrtc::test_metrics::Unit::BYTES; case Unit::kKilobitsPerSecond: return webrtc::test_metrics::Unit::KILOBITS_PER_SECOND; diff --git a/api/test/metrics/metrics_set_proto_file_exporter_test.cc b/api/test/metrics/metrics_set_proto_file_exporter_test.cc index 97987d228a..eb4d483068 100644 --- a/api/test/metrics/metrics_set_proto_file_exporter_test.cc +++ b/api/test/metrics/metrics_set_proto_file_exporter_test.cc @@ -81,7 +81,7 @@ TEST_F(MetricsSetProtoFileExporterTest, MetricsAreExportedCorrectly) { Metric metric1{ .name = "test_metric1", - .unit = Unit::kTimeMs, + .unit = Unit::kMilliseconds, .improvement_direction = ImprovementDirection::kBiggerIsBetter, .test_case = "test_case_name1", .metric_metadata = DefaultMetadata(), diff --git a/api/test/metrics/stdout_metrics_exporter_test.cc b/api/test/metrics/stdout_metrics_exporter_test.cc index 03387448a2..4e460b2981 100644 --- a/api/test/metrics/stdout_metrics_exporter_test.cc +++ b/api/test/metrics/stdout_metrics_exporter_test.cc @@ -46,7 +46,7 @@ Metric PsnrForTestFoo(double mean, double stddev) { TEST(StdoutMetricsExporterTest, ExportMetricFormatCorrect) { Metric metric1{ .name = "test_metric1", - .unit = Unit::kTimeMs, + .unit = Unit::kMilliseconds, .improvement_direction = ImprovementDirection::kBiggerIsBetter, .test_case = "test_case_name1", .metric_metadata = DefaultMetadata(), @@ -70,7 +70,7 @@ TEST(StdoutMetricsExporterTest, ExportMetricFormatCorrect) { std::string expected = "RESULT: test_case_name1/test_metric1= " - "{mean=15, stddev=5} TimeMs (BiggerIsBetter)\n" + "{mean=15, stddev=5} Milliseconds (BiggerIsBetter)\n" "RESULT: test_case_name2/test_metric2= " "{mean=30, stddev=10} KilobitsPerSecond (SmallerIsBetter)\n";