From 01f3841e387d809236a2662900013e61b7741906 Mon Sep 17 00:00:00 2001
From: Artem Titov <titovartem@webrtc.org>
Date: Tue, 20 Sep 2022 18:46:51 +0200
Subject: [PATCH] Fix: convert mean and stddev values when exporting to CPD

Also simplify exporter implementation

Bug: b/246095034
Change-Id: I656f17f7ee63d26445f3e420f3d22c52f336fa21
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/276049
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38135}
---
 .../chrome_perf_dashboard_metrics_exporter.cc | 48 +++++++++++--------
 ...me_perf_dashboard_metrics_exporter_test.cc | 31 ++++++++++++
 2 files changed, 59 insertions(+), 20 deletions(-)

diff --git a/api/test/metrics/chrome_perf_dashboard_metrics_exporter.cc b/api/test/metrics/chrome_perf_dashboard_metrics_exporter.cc
index 380249c221..0655c94edf 100644
--- a/api/test/metrics/chrome_perf_dashboard_metrics_exporter.cc
+++ b/api/test/metrics/chrome_perf_dashboard_metrics_exporter.cc
@@ -105,31 +105,39 @@ bool ChromePerfDashboardMetricsExporter::Export(
   std::unique_ptr<PerfTestResultWriter> writer =
       absl::WrapUnique<PerfTestResultWriter>(CreateHistogramWriter());
   for (const Metric& metric : metrics) {
+    if (metric.time_series.samples.empty() && IsEmpty(metric.stats)) {
+      // If there were no data collected for the metric it is expected that 0
+      // will be exported, so add 0 to the samples.
+      writer->LogResult(
+          metric.name, metric.test_case,
+          ToChromePerfDashboardValue(0, metric.unit),
+          ToChromePerfDashboardUnit(metric.unit),
+          /*important=*/false,
+          ToChromePerfDashboardImproveDirection(metric.improvement_direction));
+      continue;
+    }
+
+    if (metric.time_series.samples.empty()) {
+      writer->LogResultMeanAndError(
+          metric.name, metric.test_case,
+          ToChromePerfDashboardValue(*metric.stats.mean, metric.unit),
+          ToChromePerfDashboardValue(*metric.stats.stddev, metric.unit),
+          ToChromePerfDashboardUnit(metric.unit),
+          /*important=*/false,
+          ToChromePerfDashboardImproveDirection(metric.improvement_direction));
+      continue;
+    }
+
     std::vector<double> samples(metric.time_series.samples.size());
     for (size_t i = 0; i < metric.time_series.samples.size(); ++i) {
       samples[i] = ToChromePerfDashboardValue(
           metric.time_series.samples[i].value, metric.unit);
     }
-
-    if (samples.empty() && IsEmpty(metric.stats)) {
-      // If there were no data collected for the metric it is expected that 0
-      // will be exported, so add 0 to the samples.
-      samples.push_back(ToChromePerfDashboardValue(0, metric.unit));
-    }
-
-    if (!samples.empty()) {
-      writer->LogResultList(
-          metric.name, metric.test_case, samples,
-          ToChromePerfDashboardUnit(metric.unit),
-          /*important=*/false,
-          ToChromePerfDashboardImproveDirection(metric.improvement_direction));
-    } else {
-      writer->LogResultMeanAndError(
-          metric.name, metric.test_case, *metric.stats.mean,
-          *metric.stats.stddev, ToChromePerfDashboardUnit(metric.unit),
-          /*important=*/false,
-          ToChromePerfDashboardImproveDirection(metric.improvement_direction));
-    }
+    writer->LogResultList(
+        metric.name, metric.test_case, samples,
+        ToChromePerfDashboardUnit(metric.unit),
+        /*important=*/false,
+        ToChromePerfDashboardImproveDirection(metric.improvement_direction));
   }
   return WriteMetricsToFile(export_file_path_, writer->Serialize());
 }
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 acaa4cd0e0..89b14586ca 100644
--- a/api/test/metrics/chrome_perf_dashboard_metrics_exporter_test.cc
+++ b/api/test/metrics/chrome_perf_dashboard_metrics_exporter_test.cc
@@ -212,6 +212,37 @@ TEST_F(ChromePerfDashboardMetricsExporterTest,
   EXPECT_THAT(actual_histogram_set.histograms(0).running().variance(), Eq(0));
 }
 
+TEST_F(ChromePerfDashboardMetricsExporterTest,
+       ExportMetricWithOnlyStatsConvertsMeanValuesWhenRequired) {
+  Metric metric{.name = "test_metric",
+                .unit = Unit::kKilobitsPerSecond,
+                .improvement_direction = ImprovementDirection::kBiggerIsBetter,
+                .test_case = "test_case_name",
+                .metric_metadata = DefaultMetadata(),
+                .time_series = Metric::TimeSeries{.samples = {}},
+                .stats = Metric::Stats{
+                    .mean = 15.0, .stddev = 5.0, .min = 10.0, .max = 20.0}};
+
+  ChromePerfDashboardMetricsExporter exporter(temp_filename_);
+
+  ASSERT_TRUE(exporter.Export(std::vector<Metric>{metric}));
+  proto::HistogramSet actual_histogram_set;
+  actual_histogram_set.ParseFromString(ReadFileAsString(temp_filename_));
+  EXPECT_THAT(actual_histogram_set.histograms().size(), Eq(1));
+
+  // Validate values for `metric`
+  EXPECT_THAT(actual_histogram_set.histograms(0).sample_values().size(), Eq(1));
+  EXPECT_THAT(actual_histogram_set.histograms(0).sample_values(0), Eq(1875.0));
+  EXPECT_THAT(actual_histogram_set.histograms(0).running().count(), Eq(1));
+  EXPECT_THAT(actual_histogram_set.histograms(0).running().max(), Eq(1875));
+  EXPECT_THAT(actual_histogram_set.histograms(0).running().meanlogs(),
+              DoubleNear(7.53636, 0.1));
+  EXPECT_THAT(actual_histogram_set.histograms(0).running().mean(), Eq(1875));
+  EXPECT_THAT(actual_histogram_set.histograms(0).running().min(), Eq(1875));
+  EXPECT_THAT(actual_histogram_set.histograms(0).running().sum(), Eq(1875));
+  EXPECT_THAT(actual_histogram_set.histograms(0).running().variance(), Eq(0));
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace webrtc