kou commented on code in PR #47204:
URL: https://github.com/apache/arrow/pull/47204#discussion_r2312112892


##########
cpp/src/arrow/record_batch_test.cc:
##########
@@ -62,7 +63,49 @@ using util::Float16;
 class TestRecordBatch : public ::testing::Test {};
 
 TEST_F(TestRecordBatch, Equals) {
-  const int length = 10;
+  int length = 10;
+
+  auto f0 = field("f0", int32());
+  auto f1 = field("f1", uint8());
+  auto f2 = field("f2", int16());
+
+  auto schema0 = schema({f0, f1, f2});
+  auto schema1 = schema({f0, f1, f2});
+  auto schema2 = schema({f0, f1});
+
+  random::RandomArrayGenerator gen(42);
+
+  auto a0 = gen.ArrayOf(int32(), length);
+  auto a1 = gen.ArrayOf(uint8(), length);
+  auto a2 = gen.ArrayOf(int16(), length);
+  auto a3 = a0->Slice(0, length / 2);
+  auto a4 = a1->Slice(0, length / 2);
+  auto a5 = gen.ArrayOf(int32(), length);
+  auto a6 = gen.ArrayOf(uint8(), length);
+
+  auto b0 = RecordBatch::Make(schema0, length, {a0, a1, a2});
+  auto b1 = RecordBatch::Make(schema1, length, {a0, a1, a2});
+  auto b2 = RecordBatch::Make(schema2, length, {a0, a1});
+  auto b3 = RecordBatch::Make(schema2, length, {a3, a4});
+  auto b4 = RecordBatch::Make(schema2, length, {a5, a6});

Review Comment:
   Could you meaningful variable names only in newly added tests?
   You don't need to change existing tests.



##########
cpp/src/arrow/record_batch_test.cc:
##########
@@ -62,7 +63,49 @@ using util::Float16;
 class TestRecordBatch : public ::testing::Test {};
 
 TEST_F(TestRecordBatch, Equals) {
-  const int length = 10;
+  int length = 10;
+
+  auto f0 = field("f0", int32());
+  auto f1 = field("f1", uint8());
+  auto f2 = field("f2", int16());
+
+  auto schema0 = schema({f0, f1, f2});
+  auto schema1 = schema({f0, f1, f2});
+  auto schema2 = schema({f0, f1});
+
+  random::RandomArrayGenerator gen(42);
+
+  auto a0 = gen.ArrayOf(int32(), length);
+  auto a1 = gen.ArrayOf(uint8(), length);
+  auto a2 = gen.ArrayOf(int16(), length);
+  auto a3 = a0->Slice(0, length / 2);
+  auto a4 = a1->Slice(0, length / 2);
+  auto a5 = gen.ArrayOf(int32(), length);
+  auto a6 = gen.ArrayOf(uint8(), length);
+
+  auto b0 = RecordBatch::Make(schema0, length, {a0, a1, a2});
+  auto b1 = RecordBatch::Make(schema1, length, {a0, a1, a2});
+  auto b2 = RecordBatch::Make(schema2, length, {a0, a1});
+  auto b3 = RecordBatch::Make(schema2, length, {a3, a4});
+  auto b4 = RecordBatch::Make(schema2, length, {a5, a6});

Review Comment:
   Could you use meaningful variable names only in newly added tests?
   You don't need to change existing tests.



-- 
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]

Reply via email to