From d029cc5a571d6314ba1706b4d0fd935cb5a6125f Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Tue, 28 Aug 2018 22:54:30 +0300 Subject: [PATCH 01/11] *Display* aggregates only. --- include/benchmark/benchmark.h | 13 +++++-- src/benchmark.cc | 68 ++++++++++++++++++++++++++--------- src/benchmark_register.cc | 5 +++ test/reporter_output_test.cc | 27 ++++++++++++++ 4 files changed, 94 insertions(+), 19 deletions(-) diff --git a/include/benchmark/benchmark.h b/include/benchmark/benchmark.h index 591b45c36b..317bec22fe 100644 --- a/include/benchmark/benchmark.h +++ b/include/benchmark/benchmark.h @@ -438,9 +438,12 @@ enum AggregationReportMode : unsigned #else #endif -{ ARM_Unspecified, // The mode has not been manually specified - ARM_Default, // The mode is user-specified as default. - ARM_ReportAggregatesOnly }; +{ ARM_Unspecified, // The mode has not been manually specified + ARM_Default, // The mode is user-specified as default. + ARM_ReportAggregatesOnly, // All reporters only display aggregates. + ARM_DisplayAggregatesOnly // Display reporter displays aggregates only, + // but file reporter still contains everything. +}; } // namespace internal // State is passed to a running Benchmark and contains state for the @@ -862,8 +865,12 @@ class Benchmark { // Specify if each repetition of the benchmark should be reported separately // or if only the final statistics should be reported. If the benchmark // is not repeated then the single result is always reported. + // Applies to *ALL* reporters (display and file). Benchmark* ReportAggregatesOnly(bool value = true); + // Same as ReportAggregatesOnly(), but applies to display reporter only. + Benchmark* DisplayAggregatesOnly(bool value = true); + // If a particular benchmark is I/O bound, runs multiple threads internally or // if for some reason CPU timings are not representative, call this method. If // called, the elapsed time will be used to control how many iterations are diff --git a/src/benchmark.cc b/src/benchmark.cc index c3f3a6d144..2f63cdfc51 100644 --- a/src/benchmark.cc +++ b/src/benchmark.cc @@ -34,6 +34,7 @@ #include #include #include +#include #include "check.h" #include "colorprint.h" @@ -72,10 +73,19 @@ DEFINE_int32(benchmark_repetitions, 1, "The number of runs of each benchmark. If greater than 1, the " "mean and standard deviation of the runs will be reported."); -DEFINE_bool(benchmark_report_aggregates_only, false, - "Report the result of each benchmark repetitions. When 'true' is " - "specified only the mean, standard deviation, and other statistics " - "are reported for repeated benchmarks."); +DEFINE_bool( + benchmark_report_aggregates_only, false, + "Report the result of each benchmark repetitions. When 'true' is specified " + "only the mean, standard deviation, and other statistics are reported for " + "repeated benchmarks. Affects all reporters."); + +DEFINE_bool( + benchmark_display_aggregates_only, false, + "Display the result of each benchmark repetitions. When 'true' is " + "specified only the mean, standard deviation, and other statistics are " + "displayed for repeated benchmarks. Unlike " + "benchmark_report_aggregates_only, only affects the display reporter, but " + "*NOT* file reporter, which will still contain all the output."); DEFINE_string(benchmark_format, "console", "The format to use for console output. Valid values are " @@ -193,9 +203,11 @@ void RunInThread(const benchmark::internal::Benchmark::Instance* b, manager->NotifyThreadComplete(); } -std::vector RunBenchmark( - const benchmark::internal::Benchmark::Instance& b, - std::vector* complexity_reports) { +std::pair /*everything*/, + std::vector /*aggregates_only*/> +RunBenchmark(const benchmark::internal::Benchmark::Instance& b, + std::vector* complexity_reports, + bool& display_aggregates_only) { std::vector reports; // return value const bool has_explicit_iteration_count = b.iterations != 0; @@ -209,6 +221,11 @@ std::vector RunBenchmark( (b.aggregation_report_mode == internal::ARM_Unspecified ? FLAGS_benchmark_report_aggregates_only : b.aggregation_report_mode == internal::ARM_ReportAggregatesOnly); + display_aggregates_only = + repeats != 1 && + (b.aggregation_report_mode == internal::ARM_Unspecified + ? FLAGS_benchmark_display_aggregates_only + : b.aggregation_report_mode == internal::ARM_DisplayAggregatesOnly); for (int repetition_num = 0; repetition_num < repeats; repetition_num++) { for (;;) { // Try benchmark @@ -304,18 +321,26 @@ std::vector RunBenchmark( iters = static_cast(next_iters + 0.5); } } + // Calculate additional statistics - auto stat_reports = ComputeStats(reports); + auto aggregate_reports = ComputeStats(reports); + + // Maybe calculate complexity report if ((b.complexity != oNone) && b.last_benchmark_instance) { auto additional_run_stats = ComputeBigO(*complexity_reports); - stat_reports.insert(stat_reports.end(), additional_run_stats.begin(), - additional_run_stats.end()); + aggregate_reports.insert(aggregate_reports.end(), + additional_run_stats.begin(), + additional_run_stats.end()); complexity_reports->clear(); } if (report_aggregates_only) reports.clear(); - reports.insert(reports.end(), stat_reports.begin(), stat_reports.end()); - return reports; + + // Append aggregate statistics to the report + reports.insert(reports.end(), aggregate_reports.begin(), + aggregate_reports.end()); + + return std::make_pair(reports, aggregate_reports); } } // namespace @@ -463,10 +488,18 @@ void RunBenchmarks(const std::vector& benchmarks, flushStreams(display_reporter); flushStreams(file_reporter); for (const auto& benchmark : benchmarks) { - std::vector reports = - RunBenchmark(benchmark, &complexity_reports); - display_reporter->ReportRuns(reports); - if (file_reporter) file_reporter->ReportRuns(reports); + bool display_aggregates_only; + std::pair /*everything*/, + std::vector /*aggregates_only*/> + reports = RunBenchmark(benchmark, &complexity_reports, + display_aggregates_only); + + const std::vector& report_for_display = + display_aggregates_only ? reports.second /*aggregates_only*/ + : reports.first /*everything*/; + + display_reporter->ReportRuns(report_for_display); + if (file_reporter) file_reporter->ReportRuns(reports.first); flushStreams(display_reporter); flushStreams(file_reporter); } @@ -596,6 +629,7 @@ void PrintUsageAndExit() { " [--benchmark_min_time=]\n" " [--benchmark_repetitions=]\n" " [--benchmark_report_aggregates_only={true|false}]\n" + " [--benchmark_display_aggregates_only={true|false}]\n" " [--benchmark_format=]\n" " [--benchmark_out=]\n" " [--benchmark_out_format=]\n" @@ -619,6 +653,8 @@ void ParseCommandLineFlags(int* argc, char** argv) { &FLAGS_benchmark_repetitions) || ParseBoolFlag(argv[i], "benchmark_report_aggregates_only", &FLAGS_benchmark_report_aggregates_only) || + ParseBoolFlag(argv[i], "benchmark_display_aggregates_only", + &FLAGS_benchmark_display_aggregates_only) || ParseStringFlag(argv[i], "benchmark_format", &FLAGS_benchmark_format) || ParseStringFlag(argv[i], "benchmark_out", &FLAGS_benchmark_out) || ParseStringFlag(argv[i], "benchmark_out_format", diff --git a/src/benchmark_register.cc b/src/benchmark_register.cc index 8ee30adf0f..ea33b27c7a 100644 --- a/src/benchmark_register.cc +++ b/src/benchmark_register.cc @@ -373,6 +373,11 @@ Benchmark* Benchmark::ReportAggregatesOnly(bool value) { return this; } +Benchmark* Benchmark::DisplayAggregatesOnly(bool value) { + aggregation_report_mode_ = value ? ARM_DisplayAggregatesOnly : ARM_Default; + return this; +} + Benchmark* Benchmark::UseRealTime() { CHECK(!use_manual_time_) << "Cannot set UseRealTime and UseManualTime simultaneously."; diff --git a/test/reporter_output_test.cc b/test/reporter_output_test.cc index a98fc42a45..8045cfc9ea 100644 --- a/test/reporter_output_test.cc +++ b/test/reporter_output_test.cc @@ -342,6 +342,33 @@ ADD_CASES(TC_CSVOut, {{".*BM_SummaryRepeat/repeats:3 ", MR_Not}, {"^\"BM_SummaryRepeat/repeats:3_median\",%csv_report$"}, {"^\"BM_SummaryRepeat/repeats:3_stddev\",%csv_report$"}}); +// Test that non-aggregate data is not displayed. +// NOTE: this test is kinda bad. we are only testing the display output. +// But we don't check that the file output still contains everything... +void BM_SummaryDisplay(benchmark::State& state) { + for (auto _ : state) { + } +} +BENCHMARK(BM_SummaryDisplay)->Repetitions(2)->DisplayAggregatesOnly(); +ADD_CASES(TC_ConsoleOut, + {{".*BM_SummaryDisplay/repeats:2 ", MR_Not}, + {"^BM_SummaryDisplay/repeats:2_mean %console_report$"}, + {"^BM_SummaryDisplay/repeats:2_median %console_report$"}, + {"^BM_SummaryDisplay/repeats:2_stddev %console_report$"}}); +ADD_CASES(TC_JSONOut, {{".*BM_SummaryDisplay/repeats:2 ", MR_Not}, + {"\"name\": \"BM_SummaryDisplay/repeats:2_mean\",$"}, + {"\"run_type\": \"aggregate\",$", MR_Next}, + {"\"name\": \"BM_SummaryDisplay/repeats:2_median\",$"}, + {"\"run_type\": \"aggregate\",$", MR_Next}, + {"\"name\": \"BM_SummaryDisplay/repeats:2_stddev\",$"}, + {"\"run_type\": \"aggregate\",$", MR_Next}}); +ADD_CASES(TC_CSVOut, + {{".*BM_SummaryDisplay/repeats:2 ", MR_Not}, + {"^\"BM_SummaryDisplay/repeats:2_mean\",%csv_report$"}, + {"^\"BM_SummaryDisplay/repeats:2_median\",%csv_report$"}, + {"^\"BM_SummaryDisplay/repeats:2_stddev\",%csv_report$"}}); + +// Test repeats with custom time unit. void BM_RepeatTimeUnit(benchmark::State& state) { for (auto _ : state) { } From 966f0e02b1a83e0f0442a96d7c6536bf3300132a Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Mon, 3 Sep 2018 16:19:43 +0300 Subject: [PATCH 02/11] Make AggregationReportMode a sudo-bitfield, and RunBenchmark() return struct. --- include/benchmark/benchmark.h | 19 ++++++-- src/benchmark.cc | 87 +++++++++++++++++++---------------- src/benchmark_register.cc | 13 +++++- 3 files changed, 73 insertions(+), 46 deletions(-) diff --git a/include/benchmark/benchmark.h b/include/benchmark/benchmark.h index 317bec22fe..b7a89988cb 100644 --- a/include/benchmark/benchmark.h +++ b/include/benchmark/benchmark.h @@ -438,12 +438,21 @@ enum AggregationReportMode : unsigned #else #endif -{ ARM_Unspecified, // The mode has not been manually specified - ARM_Default, // The mode is user-specified as default. - ARM_ReportAggregatesOnly, // All reporters only display aggregates. - ARM_DisplayAggregatesOnly // Display reporter displays aggregates only, - // but file reporter still contains everything. +{ + // The mode has not been manually specified + ARM_Unspecified = 0, + // The mode is user-specified. + // This may or may not be set when the following bit-flags are set. + ARM_Default = 1U << 0U, + // File reporter should only output aggregates. + ARM_FileReportAggregatesOnly = 1U << 1U, + // Display reporter should only output aggregates + ARM_DisplayReportAggregatesOnly = 1U << 2U, + // Both reporters should only display aggregates. + ARM_ReportAggregatesOnly = + ARM_FileReportAggregatesOnly | ARM_DisplayReportAggregatesOnly }; + } // namespace internal // State is passed to a running Benchmark and contains state for the diff --git a/src/benchmark.cc b/src/benchmark.cc index 2f63cdfc51..f13c75543c 100644 --- a/src/benchmark.cc +++ b/src/benchmark.cc @@ -203,12 +203,18 @@ void RunInThread(const benchmark::internal::Benchmark::Instance* b, manager->NotifyThreadComplete(); } -std::pair /*everything*/, - std::vector /*aggregates_only*/> -RunBenchmark(const benchmark::internal::Benchmark::Instance& b, - std::vector* complexity_reports, - bool& display_aggregates_only) { - std::vector reports; // return value +struct RunResults { + std::vector non_aggregates; + std::vector aggregates_only; + + bool display_report_aggregates_only; + bool file_report_aggregates_only; +}; + +RunResults RunBenchmark( + const benchmark::internal::Benchmark::Instance& b, + std::vector* complexity_reports) { + RunResults run_results; const bool has_explicit_iteration_count = b.iterations != 0; size_t iters = has_explicit_iteration_count ? b.iterations : 1; @@ -216,16 +222,17 @@ RunBenchmark(const benchmark::internal::Benchmark::Instance& b, std::vector pool(b.threads - 1); const int repeats = b.repetitions != 0 ? b.repetitions : FLAGS_benchmark_repetitions; - const bool report_aggregates_only = - repeats != 1 && - (b.aggregation_report_mode == internal::ARM_Unspecified - ? FLAGS_benchmark_report_aggregates_only - : b.aggregation_report_mode == internal::ARM_ReportAggregatesOnly); - display_aggregates_only = - repeats != 1 && - (b.aggregation_report_mode == internal::ARM_Unspecified - ? FLAGS_benchmark_display_aggregates_only - : b.aggregation_report_mode == internal::ARM_DisplayAggregatesOnly); + run_results.display_report_aggregates_only = + repeats != 1 && (b.aggregation_report_mode == internal::ARM_Unspecified + ? FLAGS_benchmark_report_aggregates_only || + FLAGS_benchmark_display_aggregates_only + : b.aggregation_report_mode & + internal::ARM_DisplayReportAggregatesOnly); + run_results.file_report_aggregates_only = + repeats != 1 && (b.aggregation_report_mode == internal::ARM_Unspecified + ? FLAGS_benchmark_report_aggregates_only + : b.aggregation_report_mode & + internal::ARM_FileReportAggregatesOnly); for (int repetition_num = 0; repetition_num < repeats; repetition_num++) { for (;;) { // Try benchmark @@ -298,7 +305,7 @@ RunBenchmark(const benchmark::internal::Benchmark::Instance& b, b, results, memory_iterations, memory_result, seconds); if (!report.error_occurred && b.complexity != oNone) complexity_reports->push_back(report); - reports.push_back(report); + run_results.non_aggregates.push_back(report); break; } @@ -323,24 +330,18 @@ RunBenchmark(const benchmark::internal::Benchmark::Instance& b, } // Calculate additional statistics - auto aggregate_reports = ComputeStats(reports); + run_results.aggregates_only = ComputeStats(run_results.non_aggregates); // Maybe calculate complexity report if ((b.complexity != oNone) && b.last_benchmark_instance) { auto additional_run_stats = ComputeBigO(*complexity_reports); - aggregate_reports.insert(aggregate_reports.end(), - additional_run_stats.begin(), - additional_run_stats.end()); + run_results.aggregates_only.insert(run_results.aggregates_only.end(), + additional_run_stats.begin(), + additional_run_stats.end()); complexity_reports->clear(); } - if (report_aggregates_only) reports.clear(); - - // Append aggregate statistics to the report - reports.insert(reports.end(), aggregate_reports.begin(), - aggregate_reports.end()); - - return std::make_pair(reports, aggregate_reports); + return run_results; } } // namespace @@ -487,19 +488,25 @@ void RunBenchmarks(const std::vector& benchmarks, (!file_reporter || file_reporter->ReportContext(context))) { flushStreams(display_reporter); flushStreams(file_reporter); + for (const auto& benchmark : benchmarks) { - bool display_aggregates_only; - std::pair /*everything*/, - std::vector /*aggregates_only*/> - reports = RunBenchmark(benchmark, &complexity_reports, - display_aggregates_only); - - const std::vector& report_for_display = - display_aggregates_only ? reports.second /*aggregates_only*/ - : reports.first /*everything*/; - - display_reporter->ReportRuns(report_for_display); - if (file_reporter) file_reporter->ReportRuns(reports.first); + RunResults run_results = RunBenchmark(benchmark, &complexity_reports); + + auto report = [&run_results](BenchmarkReporter* reporter, + bool report_aggregates_only) { + assert(reporter); + assert( + !(report_aggregates_only && run_results.aggregates_only.empty())); + if (!report_aggregates_only) + reporter->ReportRuns(run_results.non_aggregates); + if (!run_results.aggregates_only.empty()) + reporter->ReportRuns(run_results.aggregates_only); + }; + + report(display_reporter, run_results.display_report_aggregates_only); + if (file_reporter) + report(file_reporter, run_results.file_report_aggregates_only); + flushStreams(display_reporter); flushStreams(file_reporter); } diff --git a/src/benchmark_register.cc b/src/benchmark_register.cc index ea33b27c7a..ffe97a1416 100644 --- a/src/benchmark_register.cc +++ b/src/benchmark_register.cc @@ -374,7 +374,18 @@ Benchmark* Benchmark::ReportAggregatesOnly(bool value) { } Benchmark* Benchmark::DisplayAggregatesOnly(bool value) { - aggregation_report_mode_ = value ? ARM_DisplayAggregatesOnly : ARM_Default; + // If we were called, the report mode is no longer 'unspecified', in any case. + aggregation_report_mode_ = static_cast( + aggregation_report_mode_ | ARM_Default); + + if (value) { + aggregation_report_mode_ = static_cast( + aggregation_report_mode_ | ARM_DisplayReportAggregatesOnly); + } else { + aggregation_report_mode_ = static_cast( + aggregation_report_mode_ & ~ARM_DisplayReportAggregatesOnly); + } + return this; } From 87205740fa8da0db2a50fd4fba7e36ff95e77473 Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Mon, 3 Sep 2018 22:08:11 +0300 Subject: [PATCH 03/11] Cleanup ternaries --- src/benchmark.cc | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/benchmark.cc b/src/benchmark.cc index f13c75543c..0c92ae6aa9 100644 --- a/src/benchmark.cc +++ b/src/benchmark.cc @@ -207,8 +207,8 @@ struct RunResults { std::vector non_aggregates; std::vector aggregates_only; - bool display_report_aggregates_only; - bool file_report_aggregates_only; + bool display_report_aggregates_only = false; + bool file_report_aggregates_only = false; }; RunResults RunBenchmark( @@ -222,17 +222,20 @@ RunResults RunBenchmark( std::vector pool(b.threads - 1); const int repeats = b.repetitions != 0 ? b.repetitions : FLAGS_benchmark_repetitions; - run_results.display_report_aggregates_only = - repeats != 1 && (b.aggregation_report_mode == internal::ARM_Unspecified - ? FLAGS_benchmark_report_aggregates_only || - FLAGS_benchmark_display_aggregates_only - : b.aggregation_report_mode & - internal::ARM_DisplayReportAggregatesOnly); - run_results.file_report_aggregates_only = - repeats != 1 && (b.aggregation_report_mode == internal::ARM_Unspecified - ? FLAGS_benchmark_report_aggregates_only - : b.aggregation_report_mode & - internal::ARM_FileReportAggregatesOnly); + if (repeats != 1) { + run_results.display_report_aggregates_only = + (FLAGS_benchmark_report_aggregates_only || + FLAGS_benchmark_display_aggregates_only); + run_results.file_report_aggregates_only = + FLAGS_benchmark_report_aggregates_only; + if (b.aggregation_report_mode != internal::ARM_Unspecified) { + run_results.display_report_aggregates_only = + (b.aggregation_report_mode & + internal::ARM_DisplayReportAggregatesOnly); + run_results.file_report_aggregates_only = + (b.aggregation_report_mode & internal::ARM_FileReportAggregatesOnly); + } + } for (int repetition_num = 0; repetition_num < repeats; repetition_num++) { for (;;) { // Try benchmark From fa6e3427c19306c3c02ebb28750885fcbc8ce4e1 Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Sat, 8 Sep 2018 14:53:46 +0300 Subject: [PATCH 04/11] Add a test for the actual [file] contents of the file reporter --- test/CMakeLists.txt | 6 +++ test/display_aggregates_only_test.cc | 70 ++++++++++++++++++++++++++++ test/report_aggregates_only_test.cc | 67 ++++++++++++++++++++++++++ 3 files changed, 143 insertions(+) create mode 100644 test/display_aggregates_only_test.cc create mode 100644 test/report_aggregates_only_test.cc diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index aa2058adce..4269991755 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -125,6 +125,12 @@ add_test(templated_fixture_test templated_fixture_test --benchmark_min_time=0.01 compile_output_test(user_counters_test) add_test(user_counters_test user_counters_test --benchmark_min_time=0.01) +compile_output_test(report_aggregates_only_test) +add_test(report_aggregates_only_test report_aggregates_only_test --benchmark_min_time=0.01) + +compile_output_test(display_aggregates_only_test) +add_test(display_aggregates_only_test display_aggregates_only_test --benchmark_min_time=0.01) + compile_output_test(user_counters_tabular_test) add_test(user_counters_tabular_test user_counters_tabular_test --benchmark_counters_tabular=true --benchmark_min_time=0.01) diff --git a/test/display_aggregates_only_test.cc b/test/display_aggregates_only_test.cc new file mode 100644 index 0000000000..ba75eb2beb --- /dev/null +++ b/test/display_aggregates_only_test.cc @@ -0,0 +1,70 @@ + +#undef NDEBUG +#include +#include +#include +#include +#include + +#include "benchmark/benchmark.h" +#include "output_test.h" + +// Ok this test is super ugly. We want to check what happens with the file +// reporter in the presence of DisplayAggregatesOnly(). +// We do not care about console output, the normal tests check that already. + +void BM_SummaryRepeat(benchmark::State& state) { + for (auto _ : state) { + } +} +BENCHMARK(BM_SummaryRepeat)->Repetitions(3)->DisplayAggregatesOnly(); + +int SubstringCount(const std::string& haystack, const std::string& pat) { + if (pat.length() == 0) return 0; + int count = 0; + for (size_t offset = haystack.find(pat); offset != std::string::npos; + offset = haystack.find(pat, offset + pat.length())) + ++count; + return count; +} + +int main(int argc, char* argv[]) { + std::vector new_argv(argv, argv + argc); + assert(argc == new_argv.size()); + + std::string tmp_file_name = std::tmpnam(nullptr); + std::cout << "Will be using this as the tmp file: " << tmp_file_name << '\n'; + + std::string tmp = "--benchmark_out="; + tmp += tmp_file_name; + new_argv.emplace_back(const_cast(tmp.c_str())); + + argc = new_argv.size(); + + benchmark::Initialize(&argc, new_argv.data()); + benchmark::RunSpecifiedBenchmarks(); + + // Read the output back from the file, and delete the file. + std::ifstream tmp_stream(tmp_file_name); + std::string JsonOutput((std::istreambuf_iterator(tmp_stream)), + std::istreambuf_iterator()); + std::remove(tmp_file_name.c_str()); + + if (SubstringCount(JsonOutput, "BM_SummaryRepeat/repeats:3") != 6 || + SubstringCount(JsonOutput, "BM_SummaryRepeat/repeats:3_mean") != 1 || + SubstringCount(JsonOutput, "BM_SummaryRepeat/repeats:3_median") != 1 || + SubstringCount(JsonOutput, "BM_SummaryRepeat/repeats:3_stddev") != 1) { + std::cout << "Precondition mismatch. Expected to only find 6 " + "occurrences of \"BM_SummaryRepeat/repeats:3\" substring:\n" + "\"BM_SummaryRepeat/repeats:3\", " + "\"BM_SummaryRepeat/repeats:3\", " + "\"BM_SummaryRepeat/repeats:3\", " + "\"BM_SummaryRepeat/repeats:3_mean\", " + "\"BM_SummaryRepeat/repeats:3_median\", " + "\"BM_SummaryRepeat/repeats:3_stddev\"\nThe entire output:\n"; + std::cout << JsonOutput; + return 1; + } + + return 0; +} diff --git a/test/report_aggregates_only_test.cc b/test/report_aggregates_only_test.cc new file mode 100644 index 0000000000..77c3aa3e8a --- /dev/null +++ b/test/report_aggregates_only_test.cc @@ -0,0 +1,67 @@ + +#undef NDEBUG +#include +#include +#include +#include +#include + +#include "benchmark/benchmark.h" +#include "output_test.h" + +// Ok this test is super ugly. We want to check what happens with the file +// reporter in the presence of ReportAggregatesOnly(). +// We do not care about console output, the normal tests check that already. + +void BM_SummaryRepeat(benchmark::State& state) { + for (auto _ : state) { + } +} +BENCHMARK(BM_SummaryRepeat)->Repetitions(3)->ReportAggregatesOnly(); + +int SubstringCount(const std::string& haystack, const std::string& pat) { + if (pat.length() == 0) return 0; + int count = 0; + for (size_t offset = haystack.find(pat); offset != std::string::npos; + offset = haystack.find(pat, offset + pat.length())) + ++count; + return count; +} + +int main(int argc, char* argv[]) { + std::vector new_argv(argv, argv + argc); + assert(argc == new_argv.size()); + + std::string tmp_file_name = std::tmpnam(nullptr); + std::cout << "Will be using this as the tmp file: " << tmp_file_name << '\n'; + + std::string tmp = "--benchmark_out="; + tmp += tmp_file_name; + new_argv.emplace_back(const_cast(tmp.c_str())); + + argc = new_argv.size(); + + benchmark::Initialize(&argc, new_argv.data()); + benchmark::RunSpecifiedBenchmarks(); + + // Read the output back from the file, and delete the file. + std::ifstream tmp_stream(tmp_file_name); + std::string JsonOutput((std::istreambuf_iterator(tmp_stream)), + std::istreambuf_iterator()); + std::remove(tmp_file_name.c_str()); + + if (SubstringCount(JsonOutput, "BM_SummaryRepeat/repeats:3") != 3 || + SubstringCount(JsonOutput, "BM_SummaryRepeat/repeats:3_mean") != 1 || + SubstringCount(JsonOutput, "BM_SummaryRepeat/repeats:3_median") != 1 || + SubstringCount(JsonOutput, "BM_SummaryRepeat/repeats:3_stddev") != 1) { + std::cout << "Precondition mismatch. Expected to only find three " + "occurrences of \"BM_SummaryRepeat/repeats:3\" substring:\n" + "\"BM_SummaryRepeat/repeats:3_mean\", " + "\"BM_SummaryRepeat/repeats:3_median\", " + "\"BM_SummaryRepeat/repeats:3_stddev\"\nThe entire output:\n"; + std::cout << JsonOutput; + return 1; + } + + return 0; +} From d3dbd196d7d9f9d69986a4e17205c4e9d1eee793 Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Sat, 8 Sep 2018 14:58:29 +0300 Subject: [PATCH 05/11] Be more specific about the patters we want to count --- test/display_aggregates_only_test.cc | 11 ++++++----- test/report_aggregates_only_test.cc | 10 +++++----- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/test/display_aggregates_only_test.cc b/test/display_aggregates_only_test.cc index ba75eb2beb..d85b31225a 100644 --- a/test/display_aggregates_only_test.cc +++ b/test/display_aggregates_only_test.cc @@ -19,7 +19,7 @@ void BM_SummaryRepeat(benchmark::State& state) { } BENCHMARK(BM_SummaryRepeat)->Repetitions(3)->DisplayAggregatesOnly(); -int SubstringCount(const std::string& haystack, const std::string& pat) { +int SubstrCnt(const std::string& haystack, const std::string& pat) { if (pat.length() == 0) return 0; int count = 0; for (size_t offset = haystack.find(pat); offset != std::string::npos; @@ -50,10 +50,11 @@ int main(int argc, char* argv[]) { std::istreambuf_iterator()); std::remove(tmp_file_name.c_str()); - if (SubstringCount(JsonOutput, "BM_SummaryRepeat/repeats:3") != 6 || - SubstringCount(JsonOutput, "BM_SummaryRepeat/repeats:3_mean") != 1 || - SubstringCount(JsonOutput, "BM_SummaryRepeat/repeats:3_median") != 1 || - SubstringCount(JsonOutput, "BM_SummaryRepeat/repeats:3_stddev") != 1) { + if (SubstrCnt(JsonOutput, "BM_SummaryRepeat/repeats:3") != 6 || + SubstrCnt(JsonOutput, "\"BM_SummaryRepeat/repeats:3\"") != 3 || + SubstrCnt(JsonOutput, "\"BM_SummaryRepeat/repeats:3_mean\"") != 1 || + SubstrCnt(JsonOutput, "\"BM_SummaryRepeat/repeats:3_median\"") != 1 || + SubstrCnt(JsonOutput, "\"BM_SummaryRepeat/repeats:3_stddev\"") != 1) { std::cout << "Precondition mismatch. Expected to only find 6 " "occurrences of \"BM_SummaryRepeat/repeats:3\" substring:\n" "\"BM_SummaryRepeat/repeats:3\", " diff --git a/test/report_aggregates_only_test.cc b/test/report_aggregates_only_test.cc index 77c3aa3e8a..10799f3c17 100644 --- a/test/report_aggregates_only_test.cc +++ b/test/report_aggregates_only_test.cc @@ -19,7 +19,7 @@ void BM_SummaryRepeat(benchmark::State& state) { } BENCHMARK(BM_SummaryRepeat)->Repetitions(3)->ReportAggregatesOnly(); -int SubstringCount(const std::string& haystack, const std::string& pat) { +int SubstrCnt(const std::string& haystack, const std::string& pat) { if (pat.length() == 0) return 0; int count = 0; for (size_t offset = haystack.find(pat); offset != std::string::npos; @@ -50,10 +50,10 @@ int main(int argc, char* argv[]) { std::istreambuf_iterator()); std::remove(tmp_file_name.c_str()); - if (SubstringCount(JsonOutput, "BM_SummaryRepeat/repeats:3") != 3 || - SubstringCount(JsonOutput, "BM_SummaryRepeat/repeats:3_mean") != 1 || - SubstringCount(JsonOutput, "BM_SummaryRepeat/repeats:3_median") != 1 || - SubstringCount(JsonOutput, "BM_SummaryRepeat/repeats:3_stddev") != 1) { + if (SubstrCnt(JsonOutput, "BM_SummaryRepeat/repeats:3") != 3 || + SubstrCnt(JsonOutput, "\"BM_SummaryRepeat/repeats:3_mean\"") != 1 || + SubstrCnt(JsonOutput, "\"BM_SummaryRepeat/repeats:3_median\"") != 1 || + SubstrCnt(JsonOutput, "\"BM_SummaryRepeat/repeats:3_stddev\"") != 1) { std::cout << "Precondition mismatch. Expected to only find three " "occurrences of \"BM_SummaryRepeat/repeats:3\" substring:\n" "\"BM_SummaryRepeat/repeats:3_mean\", " From 77a285c8fff7ac788edb1a558c6a8167bffb3658 Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Sat, 8 Sep 2018 16:01:37 +0300 Subject: [PATCH 06/11] Update docs, and fix compiler warnings (why are they not an error for me, but an error in CI?) --- README.md | 19 +++++++++++++------ test/display_aggregates_only_test.cc | 4 ++-- test/report_aggregates_only_test.cc | 4 ++-- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 2f2d78f6ef..37c62682d1 100644 --- a/README.md +++ b/README.md @@ -545,12 +545,19 @@ The number of runs of each benchmark is specified globally by the `Repetitions` on the registered benchmark object. When a benchmark is run more than once the mean, median and standard deviation of the runs will be reported. -Additionally the `--benchmark_report_aggregates_only={true|false}` flag or -`ReportAggregatesOnly(bool)` function can be used to change how repeated tests -are reported. By default the result of each repeated run is reported. When this -option is `true` only the mean, median and standard deviation of the runs is reported. -Calling `ReportAggregatesOnly(bool)` on a registered benchmark object overrides -the value of the flag for that benchmark. +Additionally the `--benchmark_report_aggregates_only={true|false}`, `--benchmark_display_aggregates_only={true|false}` flags or +`ReportAggregatesOnly(bool)`, `DisplayAggregatesOnly(bool)` functions can be +used to change how repeated tests are reported. By default the result of each +repeated run is reported. When `report aggregates only` option is `true`, +only the aggregates (i.e. mean, median and standard deviation, maybe complexity +measurements if they were requested) of the runs is reported, to both the +reporters - standard output (console), and the file. +However when only the `display aggregates only` option is `true`, +only the aggregates are displayed in the standard output, while the file +output still contains everything. +Calling `ReportAggregatesOnly(bool)` / `DisplayAggregatesOnly(bool)` on a +registered benchmark object overrides the value of the appropriate flag for that +benchmark. ## User-defined statistics for repeated benchmarks While having mean, median and standard deviation is nice, this may not be diff --git a/test/display_aggregates_only_test.cc b/test/display_aggregates_only_test.cc index d85b31225a..2fa003dc1a 100644 --- a/test/display_aggregates_only_test.cc +++ b/test/display_aggregates_only_test.cc @@ -30,7 +30,7 @@ int SubstrCnt(const std::string& haystack, const std::string& pat) { int main(int argc, char* argv[]) { std::vector new_argv(argv, argv + argc); - assert(argc == new_argv.size()); + assert(static_cast(argc) == new_argv.size()); std::string tmp_file_name = std::tmpnam(nullptr); std::cout << "Will be using this as the tmp file: " << tmp_file_name << '\n'; @@ -39,7 +39,7 @@ int main(int argc, char* argv[]) { tmp += tmp_file_name; new_argv.emplace_back(const_cast(tmp.c_str())); - argc = new_argv.size(); + argc = int(new_argv.size()); benchmark::Initialize(&argc, new_argv.data()); benchmark::RunSpecifiedBenchmarks(); diff --git a/test/report_aggregates_only_test.cc b/test/report_aggregates_only_test.cc index 10799f3c17..e64a7cc3ce 100644 --- a/test/report_aggregates_only_test.cc +++ b/test/report_aggregates_only_test.cc @@ -30,7 +30,7 @@ int SubstrCnt(const std::string& haystack, const std::string& pat) { int main(int argc, char* argv[]) { std::vector new_argv(argv, argv + argc); - assert(argc == new_argv.size()); + assert(static_cast(argc) == new_argv.size()); std::string tmp_file_name = std::tmpnam(nullptr); std::cout << "Will be using this as the tmp file: " << tmp_file_name << '\n'; @@ -39,7 +39,7 @@ int main(int argc, char* argv[]) { tmp += tmp_file_name; new_argv.emplace_back(const_cast(tmp.c_str())); - argc = new_argv.size(); + argc = int(new_argv.size()); benchmark::Initialize(&argc, new_argv.data()); benchmark::RunSpecifiedBenchmarks(); From 5bf8345b6de6a0d41f91080fd8ead16df4d80998 Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Wed, 12 Sep 2018 13:25:14 +0300 Subject: [PATCH 07/11] 80-column wrap in readme.md --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 37c62682d1..38203958f1 100644 --- a/README.md +++ b/README.md @@ -545,7 +545,8 @@ The number of runs of each benchmark is specified globally by the `Repetitions` on the registered benchmark object. When a benchmark is run more than once the mean, median and standard deviation of the runs will be reported. -Additionally the `--benchmark_report_aggregates_only={true|false}`, `--benchmark_display_aggregates_only={true|false}` flags or +Additionally the `--benchmark_report_aggregates_only={true|false}`, +`--benchmark_display_aggregates_only={true|false}` flags or `ReportAggregatesOnly(bool)`, `DisplayAggregatesOnly(bool)` functions can be used to change how repeated tests are reported. By default the result of each repeated run is reported. When `report aggregates only` option is `true`, From c72a15d0077569ec546077a863345c562de14c59 Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Wed, 12 Sep 2018 13:42:11 +0300 Subject: [PATCH 08/11] Share SubstrCnt() in tests --- test/display_aggregates_only_test.cc | 9 --------- test/output_test.h | 9 +++++++++ test/report_aggregates_only_test.cc | 9 --------- 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/test/display_aggregates_only_test.cc b/test/display_aggregates_only_test.cc index 2fa003dc1a..ea548f0086 100644 --- a/test/display_aggregates_only_test.cc +++ b/test/display_aggregates_only_test.cc @@ -19,15 +19,6 @@ void BM_SummaryRepeat(benchmark::State& state) { } BENCHMARK(BM_SummaryRepeat)->Repetitions(3)->DisplayAggregatesOnly(); -int SubstrCnt(const std::string& haystack, const std::string& pat) { - if (pat.length() == 0) return 0; - int count = 0; - for (size_t offset = haystack.find(pat); offset != std::string::npos; - offset = haystack.find(pat, offset + pat.length())) - ++count; - return count; -} - int main(int argc, char* argv[]) { std::vector new_argv(argv, argv + argc); assert(static_cast(argc) == new_argv.size()); diff --git a/test/output_test.h b/test/output_test.h index 31a919991f..21fa00d23b 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -201,6 +201,15 @@ namespace { const char* const dec_re = "[0-9]*[.]?[0-9]+([eE][-+][0-9]+)?"; +inline int SubstrCnt(const std::string& haystack, const std::string& pat) { + if (pat.length() == 0) return 0; + int count = 0; + for (size_t offset = haystack.find(pat); offset != std::string::npos; + offset = haystack.find(pat, offset + pat.length())) + ++count; + return count; +} + } // end namespace #endif // TEST_OUTPUT_TEST_H diff --git a/test/report_aggregates_only_test.cc b/test/report_aggregates_only_test.cc index e64a7cc3ce..3480780858 100644 --- a/test/report_aggregates_only_test.cc +++ b/test/report_aggregates_only_test.cc @@ -19,15 +19,6 @@ void BM_SummaryRepeat(benchmark::State& state) { } BENCHMARK(BM_SummaryRepeat)->Repetitions(3)->ReportAggregatesOnly(); -int SubstrCnt(const std::string& haystack, const std::string& pat) { - if (pat.length() == 0) return 0; - int count = 0; - for (size_t offset = haystack.find(pat); offset != std::string::npos; - offset = haystack.find(pat, offset + pat.length())) - ++count; - return count; -} - int main(int argc, char* argv[]) { std::vector new_argv(argv, argv + argc); assert(static_cast(argc) == new_argv.size()); From 0985914860ad263aa82ffe40434687fdc2606e94 Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Wed, 12 Sep 2018 14:48:43 +0300 Subject: [PATCH 09/11] Apply some RAII abstraction goodness --- test/display_aggregates_only_test.cc | 40 +++++++-------------------- test/file_reporter_test_helper.h | 41 ++++++++++++++++++++++++++++ test/report_aggregates_only_test.cc | 36 ++++++------------------ 3 files changed, 59 insertions(+), 58 deletions(-) create mode 100644 test/file_reporter_test_helper.h diff --git a/test/display_aggregates_only_test.cc b/test/display_aggregates_only_test.cc index ea548f0086..d9112b670f 100644 --- a/test/display_aggregates_only_test.cc +++ b/test/display_aggregates_only_test.cc @@ -1,12 +1,10 @@ #undef NDEBUG #include -#include -#include #include -#include #include "benchmark/benchmark.h" +#include "file_reporter_test_helper.h" #include "output_test.h" // Ok this test is super ugly. We want to check what happens with the file @@ -20,32 +18,14 @@ void BM_SummaryRepeat(benchmark::State& state) { BENCHMARK(BM_SummaryRepeat)->Repetitions(3)->DisplayAggregatesOnly(); int main(int argc, char* argv[]) { - std::vector new_argv(argv, argv + argc); - assert(static_cast(argc) == new_argv.size()); - - std::string tmp_file_name = std::tmpnam(nullptr); - std::cout << "Will be using this as the tmp file: " << tmp_file_name << '\n'; - - std::string tmp = "--benchmark_out="; - tmp += tmp_file_name; - new_argv.emplace_back(const_cast(tmp.c_str())); - - argc = int(new_argv.size()); - - benchmark::Initialize(&argc, new_argv.data()); - benchmark::RunSpecifiedBenchmarks(); - - // Read the output back from the file, and delete the file. - std::ifstream tmp_stream(tmp_file_name); - std::string JsonOutput((std::istreambuf_iterator(tmp_stream)), - std::istreambuf_iterator()); - std::remove(tmp_file_name.c_str()); - - if (SubstrCnt(JsonOutput, "BM_SummaryRepeat/repeats:3") != 6 || - SubstrCnt(JsonOutput, "\"BM_SummaryRepeat/repeats:3\"") != 3 || - SubstrCnt(JsonOutput, "\"BM_SummaryRepeat/repeats:3_mean\"") != 1 || - SubstrCnt(JsonOutput, "\"BM_SummaryRepeat/repeats:3_median\"") != 1 || - SubstrCnt(JsonOutput, "\"BM_SummaryRepeat/repeats:3_stddev\"") != 1) { + TestBenchmarkFileReporter helper(argc, argv); + const std::string& output = helper.getOutput(); + + if (SubstrCnt(output, "BM_SummaryRepeat/repeats:3") != 6 || + SubstrCnt(output, "\"BM_SummaryRepeat/repeats:3\"") != 3 || + SubstrCnt(output, "\"BM_SummaryRepeat/repeats:3_mean\"") != 1 || + SubstrCnt(output, "\"BM_SummaryRepeat/repeats:3_median\"") != 1 || + SubstrCnt(output, "\"BM_SummaryRepeat/repeats:3_stddev\"") != 1) { std::cout << "Precondition mismatch. Expected to only find 6 " "occurrences of \"BM_SummaryRepeat/repeats:3\" substring:\n" "\"BM_SummaryRepeat/repeats:3\", " @@ -54,7 +34,7 @@ int main(int argc, char* argv[]) { "\"BM_SummaryRepeat/repeats:3_mean\", " "\"BM_SummaryRepeat/repeats:3_median\", " "\"BM_SummaryRepeat/repeats:3_stddev\"\nThe entire output:\n"; - std::cout << JsonOutput; + std::cout << output; return 1; } diff --git a/test/file_reporter_test_helper.h b/test/file_reporter_test_helper.h new file mode 100644 index 0000000000..45854d76ab --- /dev/null +++ b/test/file_reporter_test_helper.h @@ -0,0 +1,41 @@ + +#undef NDEBUG +#include +#include +#include +#include +#include + +#include "benchmark/benchmark.h" +#include "output_test.h" + +class TestBenchmarkFileReporter { + std::string output; + + public: + TestBenchmarkFileReporter(int argc, char* argv[]) { + std::vector new_argv(argv, argv + argc); + assert(static_cast(argc) == new_argv.size()); + + std::string tmp_file_name = std::tmpnam(nullptr); + std::cout << "Will be using this as the tmp file: " << tmp_file_name + << '\n'; + + std::string tmp = "--benchmark_out="; + tmp += tmp_file_name; + new_argv.emplace_back(const_cast(tmp.c_str())); + + argc = int(new_argv.size()); + + benchmark::Initialize(&argc, new_argv.data()); + benchmark::RunSpecifiedBenchmarks(); + + // Read the output back from the file, and delete the file. + std::ifstream tmp_stream(tmp_file_name); + output = std::string((std::istreambuf_iterator(tmp_stream)), + std::istreambuf_iterator()); + std::remove(tmp_file_name.c_str()); + } + + const std::string& getOutput() const { return output; } +}; diff --git a/test/report_aggregates_only_test.cc b/test/report_aggregates_only_test.cc index 3480780858..9d356dd09e 100644 --- a/test/report_aggregates_only_test.cc +++ b/test/report_aggregates_only_test.cc @@ -1,12 +1,10 @@ #undef NDEBUG #include -#include -#include #include -#include #include "benchmark/benchmark.h" +#include "file_reporter_test_helper.h" #include "output_test.h" // Ok this test is super ugly. We want to check what happens with the file @@ -20,37 +18,19 @@ void BM_SummaryRepeat(benchmark::State& state) { BENCHMARK(BM_SummaryRepeat)->Repetitions(3)->ReportAggregatesOnly(); int main(int argc, char* argv[]) { - std::vector new_argv(argv, argv + argc); - assert(static_cast(argc) == new_argv.size()); + TestBenchmarkFileReporter helper(argc, argv); + const std::string& output = helper.getOutput(); - std::string tmp_file_name = std::tmpnam(nullptr); - std::cout << "Will be using this as the tmp file: " << tmp_file_name << '\n'; - - std::string tmp = "--benchmark_out="; - tmp += tmp_file_name; - new_argv.emplace_back(const_cast(tmp.c_str())); - - argc = int(new_argv.size()); - - benchmark::Initialize(&argc, new_argv.data()); - benchmark::RunSpecifiedBenchmarks(); - - // Read the output back from the file, and delete the file. - std::ifstream tmp_stream(tmp_file_name); - std::string JsonOutput((std::istreambuf_iterator(tmp_stream)), - std::istreambuf_iterator()); - std::remove(tmp_file_name.c_str()); - - if (SubstrCnt(JsonOutput, "BM_SummaryRepeat/repeats:3") != 3 || - SubstrCnt(JsonOutput, "\"BM_SummaryRepeat/repeats:3_mean\"") != 1 || - SubstrCnt(JsonOutput, "\"BM_SummaryRepeat/repeats:3_median\"") != 1 || - SubstrCnt(JsonOutput, "\"BM_SummaryRepeat/repeats:3_stddev\"") != 1) { + if (SubstrCnt(output, "BM_SummaryRepeat/repeats:3") != 3 || + SubstrCnt(output, "\"BM_SummaryRepeat/repeats:3_mean\"") != 1 || + SubstrCnt(output, "\"BM_SummaryRepeat/repeats:3_median\"") != 1 || + SubstrCnt(output, "\"BM_SummaryRepeat/repeats:3_stddev\"") != 1) { std::cout << "Precondition mismatch. Expected to only find three " "occurrences of \"BM_SummaryRepeat/repeats:3\" substring:\n" "\"BM_SummaryRepeat/repeats:3_mean\", " "\"BM_SummaryRepeat/repeats:3_median\", " "\"BM_SummaryRepeat/repeats:3_stddev\"\nThe entire output:\n"; - std::cout << JsonOutput; + std::cout << output; return 1; } From 44d5f7a6b5e295b8c1a5bae0a22d895af2519fd9 Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Wed, 12 Sep 2018 15:25:02 +0300 Subject: [PATCH 10/11] And make it a simple function --- test/display_aggregates_only_test.cc | 4 +-- test/file_reporter_test_helper.h | 41 ---------------------------- test/output_test.h | 28 +++++++++++++++++++ test/report_aggregates_only_test.cc | 4 +-- 4 files changed, 30 insertions(+), 47 deletions(-) delete mode 100644 test/file_reporter_test_helper.h diff --git a/test/display_aggregates_only_test.cc b/test/display_aggregates_only_test.cc index d9112b670f..46ab2e310f 100644 --- a/test/display_aggregates_only_test.cc +++ b/test/display_aggregates_only_test.cc @@ -4,7 +4,6 @@ #include #include "benchmark/benchmark.h" -#include "file_reporter_test_helper.h" #include "output_test.h" // Ok this test is super ugly. We want to check what happens with the file @@ -18,8 +17,7 @@ void BM_SummaryRepeat(benchmark::State& state) { BENCHMARK(BM_SummaryRepeat)->Repetitions(3)->DisplayAggregatesOnly(); int main(int argc, char* argv[]) { - TestBenchmarkFileReporter helper(argc, argv); - const std::string& output = helper.getOutput(); + const std::string output = GetFileReporterOutput(argc, argv); if (SubstrCnt(output, "BM_SummaryRepeat/repeats:3") != 6 || SubstrCnt(output, "\"BM_SummaryRepeat/repeats:3\"") != 3 || diff --git a/test/file_reporter_test_helper.h b/test/file_reporter_test_helper.h deleted file mode 100644 index 45854d76ab..0000000000 --- a/test/file_reporter_test_helper.h +++ /dev/null @@ -1,41 +0,0 @@ - -#undef NDEBUG -#include -#include -#include -#include -#include - -#include "benchmark/benchmark.h" -#include "output_test.h" - -class TestBenchmarkFileReporter { - std::string output; - - public: - TestBenchmarkFileReporter(int argc, char* argv[]) { - std::vector new_argv(argv, argv + argc); - assert(static_cast(argc) == new_argv.size()); - - std::string tmp_file_name = std::tmpnam(nullptr); - std::cout << "Will be using this as the tmp file: " << tmp_file_name - << '\n'; - - std::string tmp = "--benchmark_out="; - tmp += tmp_file_name; - new_argv.emplace_back(const_cast(tmp.c_str())); - - argc = int(new_argv.size()); - - benchmark::Initialize(&argc, new_argv.data()); - benchmark::RunSpecifiedBenchmarks(); - - // Read the output back from the file, and delete the file. - std::ifstream tmp_stream(tmp_file_name); - output = std::string((std::istreambuf_iterator(tmp_stream)), - std::istreambuf_iterator()); - std::remove(tmp_file_name.c_str()); - } - - const std::string& getOutput() const { return output; } -}; diff --git a/test/output_test.h b/test/output_test.h index 21fa00d23b..d4b292bd1a 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -2,10 +2,13 @@ #define TEST_OUTPUT_TEST_H #undef NDEBUG +#include +#include #include #include #include #include +#include #include #include #include @@ -210,6 +213,31 @@ inline int SubstrCnt(const std::string& haystack, const std::string& pat) { return count; } +inline std::string GetFileReporterOutput(int argc, char* argv[]) { + std::vector new_argv(argv, argv + argc); + assert(static_cast(argc) == new_argv.size()); + + std::string tmp_file_name = std::tmpnam(nullptr); + std::cout << "Will be using this as the tmp file: " << tmp_file_name << '\n'; + + std::string tmp = "--benchmark_out="; + tmp += tmp_file_name; + new_argv.emplace_back(const_cast(tmp.c_str())); + + argc = int(new_argv.size()); + + benchmark::Initialize(&argc, new_argv.data()); + benchmark::RunSpecifiedBenchmarks(); + + // Read the output back from the file, and delete the file. + std::ifstream tmp_stream(tmp_file_name); + std::string output = std::string((std::istreambuf_iterator(tmp_stream)), + std::istreambuf_iterator()); + std::remove(tmp_file_name.c_str()); + + return output; +} + } // end namespace #endif // TEST_OUTPUT_TEST_H diff --git a/test/report_aggregates_only_test.cc b/test/report_aggregates_only_test.cc index 9d356dd09e..8eb8bde2e2 100644 --- a/test/report_aggregates_only_test.cc +++ b/test/report_aggregates_only_test.cc @@ -4,7 +4,6 @@ #include #include "benchmark/benchmark.h" -#include "file_reporter_test_helper.h" #include "output_test.h" // Ok this test is super ugly. We want to check what happens with the file @@ -18,8 +17,7 @@ void BM_SummaryRepeat(benchmark::State& state) { BENCHMARK(BM_SummaryRepeat)->Repetitions(3)->ReportAggregatesOnly(); int main(int argc, char* argv[]) { - TestBenchmarkFileReporter helper(argc, argv); - const std::string& output = helper.getOutput(); + const std::string output = GetFileReporterOutput(argc, argv); if (SubstrCnt(output, "BM_SummaryRepeat/repeats:3") != 3 || SubstrCnt(output, "\"BM_SummaryRepeat/repeats:3_mean\"") != 1 || From 4f05864e0fd91d5d92df0a0f6f28b288e709dc1a Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Wed, 12 Sep 2018 15:51:57 +0300 Subject: [PATCH 11/11] Be less inline happy --- test/output_test.h | 44 ++++++-------------------------------- test/output_test_helper.cc | 37 ++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 37 deletions(-) diff --git a/test/output_test.h b/test/output_test.h index d4b292bd1a..9385761b21 100644 --- a/test/output_test.h +++ b/test/output_test.h @@ -2,13 +2,10 @@ #define TEST_OUTPUT_TEST_H #undef NDEBUG -#include -#include #include #include #include #include -#include #include #include #include @@ -63,6 +60,13 @@ int SetSubstitutions( // Run all output tests. void RunOutputTests(int argc, char* argv[]); +// Count the number of 'pat' substrings in the 'haystack' string. +int SubstrCnt(const std::string& haystack, const std::string& pat); + +// Run registered benchmarks with file reporter enabled, and return the content +// outputted by the file reporter. +std::string GetFileReporterOutput(int argc, char* argv[]); + // ========================================================================= // // ------------------------- Results checking ------------------------------ // // ========================================================================= // @@ -204,40 +208,6 @@ namespace { const char* const dec_re = "[0-9]*[.]?[0-9]+([eE][-+][0-9]+)?"; -inline int SubstrCnt(const std::string& haystack, const std::string& pat) { - if (pat.length() == 0) return 0; - int count = 0; - for (size_t offset = haystack.find(pat); offset != std::string::npos; - offset = haystack.find(pat, offset + pat.length())) - ++count; - return count; -} - -inline std::string GetFileReporterOutput(int argc, char* argv[]) { - std::vector new_argv(argv, argv + argc); - assert(static_cast(argc) == new_argv.size()); - - std::string tmp_file_name = std::tmpnam(nullptr); - std::cout << "Will be using this as the tmp file: " << tmp_file_name << '\n'; - - std::string tmp = "--benchmark_out="; - tmp += tmp_file_name; - new_argv.emplace_back(const_cast(tmp.c_str())); - - argc = int(new_argv.size()); - - benchmark::Initialize(&argc, new_argv.data()); - benchmark::RunSpecifiedBenchmarks(); - - // Read the output back from the file, and delete the file. - std::ifstream tmp_stream(tmp_file_name); - std::string output = std::string((std::istreambuf_iterator(tmp_stream)), - std::istreambuf_iterator()); - std::remove(tmp_file_name.c_str()); - - return output; -} - } // end namespace #endif // TEST_OUTPUT_TEST_H diff --git a/test/output_test_helper.cc b/test/output_test_helper.cc index 394c4f5d1a..f84bd34521 100644 --- a/test/output_test_helper.cc +++ b/test/output_test_helper.cc @@ -1,8 +1,11 @@ +#include #include +#include #include #include #include #include +#include #include "../src/benchmark_api_internal.h" #include "../src/check.h" // NOTE: check.h is for internal use only! @@ -423,3 +426,37 @@ void RunOutputTests(int argc, char* argv[]) { CHECK(std::strcmp(csv.name, "CSVReporter") == 0); internal::GetResultsChecker().CheckResults(csv.out_stream); } + +int SubstrCnt(const std::string& haystack, const std::string& pat) { + if (pat.length() == 0) return 0; + int count = 0; + for (size_t offset = haystack.find(pat); offset != std::string::npos; + offset = haystack.find(pat, offset + pat.length())) + ++count; + return count; +} + +std::string GetFileReporterOutput(int argc, char* argv[]) { + std::vector new_argv(argv, argv + argc); + assert(static_cast(argc) == new_argv.size()); + + std::string tmp_file_name = std::tmpnam(nullptr); + std::cout << "Will be using this as the tmp file: " << tmp_file_name << '\n'; + + std::string tmp = "--benchmark_out="; + tmp += tmp_file_name; + new_argv.emplace_back(const_cast(tmp.c_str())); + + argc = int(new_argv.size()); + + benchmark::Initialize(&argc, new_argv.data()); + benchmark::RunSpecifiedBenchmarks(); + + // Read the output back from the file, and delete the file. + std::ifstream tmp_stream(tmp_file_name); + std::string output = std::string((std::istreambuf_iterator(tmp_stream)), + std::istreambuf_iterator()); + std::remove(tmp_file_name.c_str()); + + return output; +}