[GitHub] [arrow] pitrou commented on a change in pull request #7555: ARROW-9238: [C++][CI][FlightRPC] increase test coverage of round-robin under IPC and Flight

[GitBox <gi...@apache.org>]

pitrou commented on a change in pull request #7555:
URL: https://github.com/apache/arrow/pull/7555#discussion_r450267748



##########
File path: cpp/src/arrow/testing/random.h
##########
@@ -140,6 +140,17 @@ class ARROW_TESTING_EXPORT RandomArrayGenerator {
   std::shared_ptr<Array> Int64(int64_t size, int64_t min, int64_t max,
                                double null_probability = 0);
 
+  /// \brief Generates a random HalfFloatArray
+  ///
+  /// \param[in] size the size of the array to generate
+  /// \param[in] min the lower bound of the uniform distribution
+  /// \param[in] max the upper bound of the uniform distribution

Review comment:
       Hmm... this will not really be a *uniform* float16 distribution if you're selecting random values for the underlying int16 representation, right?

##########
File path: cpp/src/arrow/ipc/test_common.cc
##########
@@ -79,6 +79,39 @@ Status MakeRandomInt32Array(int64_t length, bool include_nulls, MemoryPool* pool
   return Status::OK();
 }
 
+template <typename ArrayType>
+static Status MakeRandomArray(int64_t length, bool include_nulls, MemoryPool* pool,
+                              std::shared_ptr<Array>* out, uint32_t seed) {
+  random::RandomArrayGenerator rand(seed);
+  const double null_probability = include_nulls ? 0.5 : 0.0;
+
+  *out = rand.Numeric<ArrayType>(length, 0, 1000, null_probability);
+
+  return Status::OK();
+}
+
+template <>
+Status MakeRandomArray<Int8Type>(int64_t length, bool include_nulls, MemoryPool* pool,

Review comment:
       Make those static as well? (or put them in the anonymous namespace)

##########
File path: cpp/src/arrow/ipc/feather_test.cc
##########
@@ -294,6 +325,26 @@ INSTANTIATE_TEST_SUITE_P(
                       TestParam(kFeatherV2Version, Compression::LZ4_FRAME),
                       TestParam(kFeatherV2Version, Compression::ZSTD)));
 
+#define BATCH_CASES()                                                                    \

Review comment:
       `using kBatchCases = ::testing::Values(...)` perhaps?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org