Skip to content

Commit 9a2a5fb

Browse files
committed
CSV reporter: escape counter names and keep skipped rows full width
Two ways the CSV output could stop being well-formed: The header wrote each user counter name as "\"" << name << "\"", so a counter whose name contains a double quote produced a broken header field. Write it with CsvEscape, like every other string field. Names without quotes come out exactly as before. A skipped benchmark's row stopped after error_message, without the empty fields for the user counters that every other row and the header have. Readers then see a row with fewer columns than the header (the test helper's own ResultsChecker asserts that the widths match). Append one empty field per user counter. user_counters_test gains a skipped benchmark; its CSV row now ends with the two empty "bar" and "foo" fields, and the test fails with "End of output reached before match" without this change.
1 parent 82a8950 commit 9a2a5fb

2 files changed

Lines changed: 19 additions & 4 deletions

File tree

‎src/csv_reporter.cc‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,8 @@ void CSVReporter::ReportRuns(const std::vector<Run>& reports) {
7878
Out << ",";
7979
}
8080
}
81-
for (auto B = user_counter_names_.begin();
82-
B != user_counter_names_.end();) {
83-
Out << ",\"" << *B++ << "\"";
81+
for (const auto& name : user_counter_names_) {
82+
Out << "," << CsvEscape(name);
8483
}
8584
Out << "\n";
8685

@@ -115,7 +114,9 @@ void CSVReporter::PrintRunData(const Run& run) {
115114
if (run.skipped != 0u) {
116115
Out << std::string(elements.size() - 3, ',');
117116
Out << std::boolalpha << (internal::SkippedWithError == run.skipped) << ",";
118-
Out << CsvEscape(run.skip_message) << "\n";
117+
Out << CsvEscape(run.skip_message);
118+
// Keep the row as wide as the header: one empty field per user counter.
119+
Out << std::string(user_counter_names_.size(), ',') << "\n";
119120
return;
120121
}
121122

‎test/user_counters_test.cc‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,20 @@ CHECK_BENCHMARK_RESULTS("BM_Counters_kAvgIterationsRate",
587587
&CheckAvgIterationsRate);
588588
} // end namespace
589589

590+
// ========================================================================= //
591+
// ------------------------- Skipped Benchmark Output ---------------------- //
592+
// ========================================================================= //
593+
594+
void BM_Counters_Skipped(benchmark::State& state) {
595+
state.SkipWithError("skipped");
596+
for (auto _ : state) {
597+
}
598+
}
599+
BENCHMARK(BM_Counters_Skipped);
600+
// The row for a skipped benchmark has one empty field per user counter in the
601+
// header ("bar" and "foo"), like every other row.
602+
ADD_CASES(TC_CSVOut, {{"^\"BM_Counters_Skipped\",,,,,,,,true,\"skipped\",,$"}});
603+
590604
// ========================================================================= //
591605
// --------------------------- TEST CASES END ------------------------------ //
592606
// ========================================================================= //

0 commit comments

Comments
 (0)