westonpace commented on a change in pull request #11663:
URL: https://github.com/apache/arrow/pull/11663#discussion_r750139221
##########
File path: cpp/src/arrow/compute/exec/hash_join_node_test.cc
##########
@@ -1061,16 +1061,18 @@ TEST(HashJoin, Random) {
// Print test case parameters
// print num_rows, batch_size, join_type, join_cmp
- std::cout << join_type_name << " " << key_cmp_str << " ";
- key_types.Print();
- std::cout << " payload_l: ";
- payload_types[0].Print();
- std::cout << " payload_r: ";
- payload_types[1].Print();
- std::cout << " num_rows_l = " << num_rows_l << " num_rows_r = " <<
num_rows_r
- << " batch size = " << batch_size
- << " parallel = " << (parallel ? "true" : "false");
- std::cout << std::endl;
+ // std::cout << join_type_name << " " << key_cmp_str << " ";
+ // key_types.Print();
+ // std::cout << " payload_l: ";
+ // payload_types[0].Print();
+ // std::cout << " payload_r: ";
+ // payload_types[1].Print();
+ // std::cout << " num_rows_l = " << num_rows_l << " num_rows_r = " <<
num_rows_r
+ // << " batch size = " << batch_size
+ // << " parallel = " << (parallel ? "true" : "false");
+ // std::cout << std::endl;
Review comment:
Go ahead and just delete these lines entirely. No need to leave the
commented out code.
##########
File path: cpp/src/gandiva/tests/date_time_test.cc
##########
@@ -452,7 +452,8 @@ TEST_F(TestProjector, TestTimestampDiffMonth) {
std::shared_ptr<Projector> projector;
auto status =
Projector::Make(schema, {diff_months_expr}, TestConfiguration(),
&projector);
- std::cout << status.message();
+ // std::cout << status.message();
+ SCOPED_TRACE(status.message());
Review comment:
Just delete this, no need for scoped_trace here. If status is not `ok`
then the test will fail and it will be obvious where it failed. Status is not
a "test parameter"
Same with the next two occurrences.
##########
File path: cpp/src/parquet/column_writer_test.cc
##########
@@ -254,7 +254,8 @@ class TestPrimitiveWriter : public
PrimitiveTypedTest<TestType> {
for (size_t i = 0; i < this->values_.size(); i++) {
if (comparator->Compare(this->values_[i], this->values_out_[i]) ||
comparator->Compare(this->values_out_[i], this->values_[i])) {
- std::cout << "Failed at " << i << std::endl;
+ // std::cout << "Failed at " << i << std::endl;
+ ARROW_SCOPED_TRACE("Failed at ", i);
Review comment:
The message should be more like `ARROW_SCOPED_TRACE("i=",i);`. The
trace should tell use the parameter values, it doesn't need to include "failed
at" as that will be implied (the trace info is only shown on a failure).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
