From 7736df03049c362c7275f7573de6d6a685630e0a Mon Sep 17 00:00:00 2001 From: Andreas Abel Date: Tue, 26 Sep 2023 14:43:23 +0200 Subject: [PATCH] Make json and csv output consistent. (#1662) * Make json and csv output consistent. Currently, the --benchmark_format=csv option does not output the correct value for the cv statistics. Also, the json output should not contain a time unit for the cv statistics. * fix formatting * undo json change --------- Co-authored-by: dominic <510002+dmah42@users.noreply.github.com> --- src/csv_reporter.cc | 14 +++++++++++--- test/output_test_helper.cc | 1 + test/reporter_output_test.cc | 2 +- test/user_counters_tabular_test.cc | 4 ++-- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/csv_reporter.cc b/src/csv_reporter.cc index 7b56da10..4b39e2c5 100644 --- a/src/csv_reporter.cc +++ b/src/csv_reporter.cc @@ -122,13 +122,21 @@ void CSVReporter::PrintRunData(const Run& run) { } Out << ","; - Out << run.GetAdjustedRealTime() << ","; - Out << run.GetAdjustedCPUTime() << ","; + if (run.run_type != Run::RT_Aggregate || + run.aggregate_unit == StatisticUnit::kTime) { + Out << run.GetAdjustedRealTime() << ","; + Out << run.GetAdjustedCPUTime() << ","; + } else { + assert(run.aggregate_unit == StatisticUnit::kPercentage); + Out << run.real_accumulated_time << ","; + Out << run.cpu_accumulated_time << ","; + } // Do not print timeLabel on bigO and RMS report if (run.report_big_o) { Out << GetBigOString(run.complexity); - } else if (!run.report_rms) { + } else if (!run.report_rms && + run.aggregate_unit != StatisticUnit::kPercentage) { Out << GetTimeUnitString(run.time_unit); } Out << ","; diff --git a/test/output_test_helper.cc b/test/output_test_helper.cc index 25673700..265f28aa 100644 --- a/test/output_test_helper.cc +++ b/test/output_test_helper.cc @@ -65,6 +65,7 @@ SubMap& GetSubstitutions() { {"%csv_us_report", "[0-9]+," + safe_dec_re + "," + safe_dec_re + ",us,,,,,"}, {"%csv_ms_report", "[0-9]+," + safe_dec_re + "," + safe_dec_re + ",ms,,,,,"}, {"%csv_s_report", "[0-9]+," + safe_dec_re + "," + safe_dec_re + ",s,,,,,"}, + {"%csv_cv_report", "[0-9]+," + safe_dec_re + "," + safe_dec_re + ",,,,,,"}, {"%csv_bytes_report", "[0-9]+," + safe_dec_re + "," + safe_dec_re + ",ns," + safe_dec_re + ",,,,"}, {"%csv_items_report", diff --git a/test/reporter_output_test.cc b/test/reporter_output_test.cc index 2eb545a8..657a9a10 100644 --- a/test/reporter_output_test.cc +++ b/test/reporter_output_test.cc @@ -1088,7 +1088,7 @@ ADD_CASES(TC_CSVOut, {{"^\"BM_UserPercentStats/iterations:5/repeats:3/" {"^\"BM_UserPercentStats/iterations:5/repeats:3/" "manual_time_stddev\",%csv_report$"}, {"^\"BM_UserPercentStats/iterations:5/repeats:3/" - "manual_time_\",%csv_report$"}}); + "manual_time_\",%csv_cv_report$"}}); // ========================================================================= // // ------------------------- Testing StrEscape JSON ------------------------ // diff --git a/test/user_counters_tabular_test.cc b/test/user_counters_tabular_test.cc index c98b769a..e7ada657 100644 --- a/test/user_counters_tabular_test.cc +++ b/test/user_counters_tabular_test.cc @@ -330,7 +330,7 @@ ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Tabular/repeats:2/threads:1_stddev\",%csv_report," "%float,%float,%float,%float,%float,%float$"}}); ADD_CASES(TC_CSVOut, - {{"^\"BM_Counters_Tabular/repeats:2/threads:1_cv\",%csv_report," + {{"^\"BM_Counters_Tabular/repeats:2/threads:1_cv\",%csv_cv_report," "%float,%float,%float,%float,%float,%float$"}}); ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Tabular/repeats:2/threads:2\",%csv_report," @@ -348,7 +348,7 @@ ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Tabular/repeats:2/threads:2_stddev\",%csv_report," "%float,%float,%float,%float,%float,%float$"}}); ADD_CASES(TC_CSVOut, - {{"^\"BM_Counters_Tabular/repeats:2/threads:2_cv\",%csv_report," + {{"^\"BM_Counters_Tabular/repeats:2/threads:2_cv\",%csv_cv_report," "%float,%float,%float,%float,%float,%float$"}}); // VS2013 does not allow this function to be passed as a lambda argument // to CHECK_BENCHMARK_RESULTS()