ARROW-15030: [C++] CSV writer test failures

[johanpel]

No description provided.

[github-actions]

https://issues.apache.org/jira/browse/ARROW-15030

[pitrou]

This doesn't fix the issue. Actually, you should be able to reproduce it by enabling -DARROW_EXTRA_ERROR_CONTEXT=on in the CMake config. It adds context lines to error messages which you can see in the failed comparison:

[ RUN      ] MultiColumnWriteCSVTest/TestWriteCSV.TestWrite/13
/home/antoine/arrow/dev/cpp/src/arrow/csv/writer_test.cc:246: Failure
Expected equality of these values:
  ToCsvString(*record_batch, options)
    Which is: Invalid: CSV values may not contain structural characters if quoting style is "None". See RFC4180. Invalid value: foo
/home/antoine/arrow/dev/cpp/src/arrow/csv/writer.cc:195  CheckStringHasNoStructuralChars(s)
/home/antoine/arrow/dev/cpp/src/arrow/util/bit_block_counter.h:443  visit_not_null(position)
/home/antoine/arrow/dev/cpp/src/arrow/csv/writer.cc:515  (*populator) ->PopulateColumns(reinterpret_cast<char*>(data_buffer_->mutable_data()), offsets_.data())
/home/antoine/arrow/dev/cpp/src/arrow/csv/writer.cc:416  TranslateMinimalBatch(*slice)
/home/antoine/arrow/dev/cpp/src/arrow/csv/writer.cc:547  writer->WriteRecordBatch(batch)
/home/antoine/arrow/dev/cpp/src/arrow/csv/writer_test.cc:212  WriteCSV(data, options, out.get())
  GetParam().expected_status
    Which is: Invalid: CSV values may not contain structural characters if quoting style is "None". See RFC4180. Invalid value: foo
[  FAILED  ] MultiColumnWriteCSVTest/TestWriteCSV.TestWrite/13, where GetParam() = WriterTestParams(0x55e9414b7230) (0 ms)

[johanpel]

I can reproduce it now indeed, thanks.

If I understand correctly, erroneous status messages created through the ARROW_RETURN_IF_ macro are different depending on this flag.

In this case, it means you cannot check error messages that are contained within such a status, so you can only check whether the StatusCode matches, but not whether the message is as expected.

I will update the PR to fix this.

Since the context information only exists for debugging purposes, would it be an idea to somehow exclude it from status equality predicates, such that tests with/without the flag do not differ in outcome?

[pitrou]

@johanpel You can actually test the message using something like:

EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid, ::testing::HasSubstr("some substring"), ...);

[pitrou]

+1, thanks for the update @johanpel

[ursabot]

Benchmark runs are scheduled for baseline = fa5bbb7 and contender = b15a2f3. b15a2f3 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.0% ⬆️0.04%] ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java