This is an automated email from the ASF dual-hosted git repository. kou pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push: new 3453943 ARROW-10746: [C++] Bump gtest version + use GTEST_SKIP in tests 3453943 is described below commit 34539432a3f49672cd352e2fa8f626489ea3e954 Author: Andrew Wieteska <andrew.r.wiete...@gmail.com> AuthorDate: Mon Dec 7 14:00:53 2020 +0900 ARROW-10746: [C++] Bump gtest version + use GTEST_SKIP in tests As per a TODO left in ARROW-3769 / #3721 we can now use the `GTEST_SKIP` macro in `parquet/encoding-test.cpp`. `GTEST_SKIP` was added in gtest 1.10.0 so this involves bumping our minimal gtest version from 1.8.1 Closes #8782 from arw2019/ARROW-10746-GTEST_SKIP Lead-authored-by: Andrew Wieteska <andrew.r.wiete...@gmail.com> Co-authored-by: Sutou Kouhei <k...@clear-code.com> Signed-off-by: Sutou Kouhei <k...@clear-code.com> --- ci/conda_env_cpp.yml | 4 +- cpp/cmake_modules/SetupCxxFlags.cmake | 21 +++++++ cpp/cmake_modules/ThirdpartyToolchain.cmake | 93 ++++++++++++++++++----------- cpp/src/arrow/io/hdfs_test.cc | 7 +-- cpp/src/arrow/util/compression_test.cc | 33 ++++------ cpp/src/parquet/column_scanner_test.cc | 6 +- cpp/src/parquet/column_writer_test.cc | 2 - cpp/src/parquet/encoding_test.cc | 75 ++++++++++++----------- cpp/src/parquet/file_serialize_test.cc | 2 - cpp/src/parquet/statistics_test.cc | 48 +++++++-------- cpp/src/parquet/test_util.h | 20 +++---- cpp/thirdparty/versions.txt | 2 +- 12 files changed, 171 insertions(+), 142 deletions(-) diff --git a/ci/conda_env_cpp.yml b/ci/conda_env_cpp.yml index 870d851..390eb7d 100644 --- a/ci/conda_env_cpp.yml +++ b/ci/conda_env_cpp.yml @@ -24,9 +24,9 @@ c-ares cmake gflags glog -gmock>=1.8.1 +gmock>=1.10.0 grpc-cpp>=1.27.3 -gtest=1.8.1 +gtest=1.10.0 libprotobuf libutf8proc lz4-c diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake b/cpp/cmake_modules/SetupCxxFlags.cmake index a5cd95b..402d18f 100644 --- a/cpp/cmake_modules/SetupCxxFlags.cmake +++ b/cpp/cmake_modules/SetupCxxFlags.cmake @@ -159,6 +159,23 @@ if(WIN32) set(CXX_COMMON_FLAGS "/W3 /EHsc") endif() + # Disable C5105 (macro expansion producing 'defined' has undefined + # behavior) warning because there are codes that produce this + # warning in Windows Kits. e.g.: + # + # #define _CRT_INTERNAL_NONSTDC_NAMES \ + # ( \ + # ( defined _CRT_DECLARE_NONSTDC_NAMES && _CRT_DECLARE_NONSTDC_NAMES) || \ + # (!defined _CRT_DECLARE_NONSTDC_NAMES && !__STDC__ ) \ + # ) + # + # See also: + # * C5105: https://docs.microsoft.com/en-US/cpp/error-messages/compiler-warnings/c5105 + # * Related reports: + # * https://developercommunity.visualstudio.com/content/problem/387684/c5105-with-stdioh-and-experimentalpreprocessor.html + # * https://developercommunity.visualstudio.com/content/problem/1249671/stdc17-generates-warning-compiling-windowsh.html + set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} /wd5105") + if(ARROW_USE_STATIC_CRT) foreach(c_flag CMAKE_CXX_FLAGS @@ -177,6 +194,10 @@ if(WIN32) # Support large object code set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} /bigobj") + + # We may use UTF-8 in source code such as + # cpp/src/arrow/compute/kernels/scalar_string_test.cc + set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} /utf-8") else() # MinGW check_cxx_compiler_flag(-Wa,-mbig-obj CXX_SUPPORTS_BIG_OBJ) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index df03c31..47e6be2 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -1603,21 +1603,17 @@ macro(build_gtest) set(GTEST_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/googletest_ep-prefix") set(GTEST_INCLUDE_DIR "${GTEST_PREFIX}/include") - set(_GTEST_RUNTIME_DIR ${BUILD_OUTPUT_ROOT_DIRECTORY}) + set(_GTEST_LIBRARY_DIR "${GTEST_PREFIX}/lib") if(MSVC) set(_GTEST_IMPORTED_TYPE IMPORTED_IMPLIB) set(_GTEST_LIBRARY_SUFFIX "${CMAKE_GTEST_DEBUG_EXTENSION}${CMAKE_IMPORT_LIBRARY_SUFFIX}") - # Use the import libraries from the EP - set(_GTEST_LIBRARY_DIR "${GTEST_PREFIX}/lib") else() set(_GTEST_IMPORTED_TYPE IMPORTED_LOCATION) set(_GTEST_LIBRARY_SUFFIX "${CMAKE_GTEST_DEBUG_EXTENSION}${CMAKE_SHARED_LIBRARY_SUFFIX}") - # Library and runtime same on non-Windows - set(_GTEST_LIBRARY_DIR "${_GTEST_RUNTIME_DIR}") endif() set(GTEST_SHARED_LIB @@ -1630,38 +1626,16 @@ macro(build_gtest) ) set(GTEST_CMAKE_ARGS ${EP_COMMON_TOOLCHAIN} - -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} - "-DCMAKE_INSTALL_PREFIX=${GTEST_PREFIX}" -DBUILD_SHARED_LIBS=ON + -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} -DCMAKE_CXX_FLAGS=${GTEST_CMAKE_CXX_FLAGS} - -DCMAKE_CXX_FLAGS_${UPPERCASE_BUILD_TYPE}=${GTEST_CMAKE_CXX_FLAGS}) + -DCMAKE_CXX_FLAGS_${UPPERCASE_BUILD_TYPE}=${GTEST_CMAKE_CXX_FLAGS} + -DCMAKE_INSTALL_LIBDIR=lib + -DCMAKE_INSTALL_NAME_DIR=$<INSTALL_PREFIX$<ANGLE-R>/lib + -DCMAKE_INSTALL_PREFIX=${GTEST_PREFIX} + -DCMAKE_MACOSX_RPATH=OFF) set(GMOCK_INCLUDE_DIR "${GTEST_PREFIX}/include") - if(APPLE) - set(GTEST_CMAKE_ARGS ${GTEST_CMAKE_ARGS} "-DCMAKE_MACOSX_RPATH:BOOL=ON") - endif() - - if(CMAKE_GENERATOR STREQUAL "Xcode") - # Xcode projects support multi-configuration builds. This forces the gtest build - # to use the same output directory as a single-configuration Makefile driven build. - list( - APPEND GTEST_CMAKE_ARGS "-DCMAKE_LIBRARY_OUTPUT_DIRECTORY=${_GTEST_LIBRARY_DIR}" - "-DCMAKE_LIBRARY_OUTPUT_DIRECTORY_${CMAKE_BUILD_TYPE}=${_GTEST_RUNTIME_DIR}") - endif() - - if(MSVC) - if(NOT ("${CMAKE_GENERATOR}" STREQUAL "Ninja")) - set(_GTEST_RUNTIME_DIR ${_GTEST_RUNTIME_DIR}/${CMAKE_BUILD_TYPE}) - endif() - set(GTEST_CMAKE_ARGS - ${GTEST_CMAKE_ARGS} "-DCMAKE_RUNTIME_OUTPUT_DIRECTORY=${_GTEST_RUNTIME_DIR}" - "-DCMAKE_RUNTIME_OUTPUT_DIRECTORY_${CMAKE_BUILD_TYPE}=${_GTEST_RUNTIME_DIR}") - else() - list( - APPEND GTEST_CMAKE_ARGS "-DCMAKE_LIBRARY_OUTPUT_DIRECTORY=${_GTEST_RUNTIME_DIR}" - "-DCMAKE_RUNTIME_OUTPUT_DIRECTORY_${CMAKE_BUILD_TYPE}=${_GTEST_RUNTIME_DIR}") - endif() - add_definitions(-DGTEST_LINKED_AS_SHARED_LIBRARY=1) if(MSVC AND NOT ARROW_USE_STATIC_CRT) @@ -1673,6 +1647,57 @@ macro(build_gtest) BUILD_BYPRODUCTS ${GTEST_SHARED_LIB} ${GTEST_MAIN_SHARED_LIB} ${GMOCK_SHARED_LIB} CMAKE_ARGS ${GTEST_CMAKE_ARGS} ${EP_LOG_OPTIONS}) + if(WIN32) + # Copy the built shared libraries to the same directory as our + # test programs because Windows doesn't provided rpath (run-time + # search path) feature. We need to put these shared libraries to + # the same directory as our test programs or add + # _GTEST_LIBRARY_DIR to PATH when we run our test programs. We + # choose the former because the latter may be forgotten. + set(_GTEST_RUNTIME_DIR "${GTEST_PREFIX}/bin") + set(_GTEST_RUNTIME_SUFFIX + "${CMAKE_GTEST_DEBUG_EXTENSION}${CMAKE_SHARED_LIBRARY_SUFFIX}") + set( + _GTEST_RUNTIME_LIB + "${_GTEST_RUNTIME_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest${_GTEST_RUNTIME_SUFFIX}") + set( + _GMOCK_RUNTIME_LIB + "${_GTEST_RUNTIME_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gmock${_GTEST_RUNTIME_SUFFIX}") + set( + _GTEST_MAIN_RUNTIME_LIB + "${_GTEST_RUNTIME_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest_main${_GTEST_RUNTIME_SUFFIX}" + ) + if(CMAKE_VERSION VERSION_LESS 3.9) + message( + FATAL_ERROR + "Building GoogleTest from source on Windows requires at least CMake 3.9") + endif() + get_property(_GENERATOR_IS_MULTI_CONFIG GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG) + if(_GENERATOR_IS_MULTI_CONFIG) + set(_GTEST_RUNTIME_OUTPUT_DIR "${BUILD_OUTPUT_ROOT_DIRECTORY}/${CMAKE_BUILD_TYPE}") + else() + set(_GTEST_RUNTIME_OUTPUT_DIR ${BUILD_OUTPUT_ROOT_DIRECTORY}) + endif() + externalproject_add_step(googletest_ep copy + COMMAND ${CMAKE_COMMAND} -E make_directory + ${_GTEST_RUNTIME_OUTPUT_DIR} + COMMAND ${CMAKE_COMMAND} + -E + copy + ${_GTEST_RUNTIME_LIB} + ${_GTEST_RUNTIME_OUTPUT_DIR} + COMMAND ${CMAKE_COMMAND} + -E + copy + ${_GMOCK_RUNTIME_LIB} + ${_GTEST_RUNTIME_OUTPUT_DIR} + COMMAND ${CMAKE_COMMAND} + -E + copy + ${_GTEST_MAIN_RUNTIME_LIB} + ${_GTEST_RUNTIME_OUTPUT_DIR} + DEPENDEES install) + endif() # The include directory must exist before it is referenced by a target. file(MAKE_DIRECTORY "${GTEST_INCLUDE_DIR}") @@ -1698,7 +1723,7 @@ macro(build_gtest) endmacro() if(ARROW_TESTING) - resolve_dependency(GTest) + resolve_dependency(GTest REQUIRED_VERSION 1.10.0) if(NOT GTEST_VENDORED) # TODO(wesm): This logic does not work correctly with the MSVC static libraries diff --git a/cpp/src/arrow/io/hdfs_test.cc b/cpp/src/arrow/io/hdfs_test.cc index ac71007..2ebf950 100644 --- a/cpp/src/arrow/io/hdfs_test.cc +++ b/cpp/src/arrow/io/hdfs_test.cc @@ -136,10 +136,9 @@ class TestHadoopFileSystem : public ::testing::Test { std::shared_ptr<HadoopFileSystem> client_; }; -#define SKIP_IF_NO_DRIVER() \ - if (!this->loaded_driver_) { \ - std::cout << "Driver not loaded, skipping" << std::endl; \ - return; \ +#define SKIP_IF_NO_DRIVER() \ + if (!this->loaded_driver_) { \ + GTEST_SKIP() << "Driver not loaded, skipping"; \ } TEST_F(TestHadoopFileSystem, ConnectsAgain) { diff --git a/cpp/src/arrow/util/compression_test.cc b/cpp/src/arrow/util/compression_test.cc index 8b184f2..ef34701 100644 --- a/cpp/src/arrow/util/compression_test.cc +++ b/cpp/src/arrow/util/compression_test.cc @@ -349,8 +349,7 @@ TEST(TestCodecMisc, GetCompressionType) { TEST_P(CodecTest, CodecRoundtrip) { const auto compression = GetCompression(); if (compression == Compression::BZ2) { - // SKIP: BZ2 doesn't support one-shot compression - return; + GTEST_SKIP() << "BZ2 does not support one-shot compression"; } int sizes[] = {0, 10000, 100000}; @@ -429,17 +428,14 @@ TEST_P(CodecTest, OutputBufferIsSmall) { TEST_P(CodecTest, StreamingCompressor) { if (GetCompression() == Compression::SNAPPY) { - // SKIP: snappy doesn't support streaming compression - return; + GTEST_SKIP() << "snappy doesn't support streaming compression"; } if (GetCompression() == Compression::BZ2) { - // SKIP: BZ2 doesn't support one-shot decompression - return; + GTEST_SKIP() << "Z2 doesn't support one-shot decompression"; } if (GetCompression() == Compression::LZ4 || GetCompression() == Compression::LZ4_HADOOP) { - // SKIP: LZ4 raw format doesn't support streaming compression. - return; + GTEST_SKIP() << "LZ4 raw format doesn't support streaming compression."; } int sizes[] = {0, 10, 100000}; @@ -456,17 +452,14 @@ TEST_P(CodecTest, StreamingCompressor) { TEST_P(CodecTest, StreamingDecompressor) { if (GetCompression() == Compression::SNAPPY) { - // SKIP: snappy doesn't support streaming decompression - return; + GTEST_SKIP() << "snappy doesn't support streaming decompression."; } if (GetCompression() == Compression::BZ2) { - // SKIP: BZ2 doesn't support one-shot compression - return; + GTEST_SKIP() << "Z2 doesn't support one-shot compression"; } if (GetCompression() == Compression::LZ4 || GetCompression() == Compression::LZ4_HADOOP) { - // SKIP: LZ4 raw format doesn't support streaming decompression. - return; + GTEST_SKIP() << "LZ4 raw format doesn't support streaming decompression."; } int sizes[] = {0, 10, 100000}; @@ -483,13 +476,11 @@ TEST_P(CodecTest, StreamingDecompressor) { TEST_P(CodecTest, StreamingRoundtrip) { if (GetCompression() == Compression::SNAPPY) { - // SKIP: snappy doesn't support streaming decompression - return; + GTEST_SKIP() << "snappy doesn't support streaming decompression"; } if (GetCompression() == Compression::LZ4 || GetCompression() == Compression::LZ4_HADOOP) { - // SKIP: LZ4 raw format doesn't support streaming compression. - return; + GTEST_SKIP() << "LZ4 raw format doesn't support streaming compression."; } int sizes[] = {0, 10, 100000}; @@ -506,13 +497,11 @@ TEST_P(CodecTest, StreamingRoundtrip) { TEST_P(CodecTest, StreamingDecompressorReuse) { if (GetCompression() == Compression::SNAPPY) { - // SKIP: snappy doesn't support streaming decompression - return; + GTEST_SKIP() << "snappy doesn't support streaming decompression"; } if (GetCompression() == Compression::LZ4 || GetCompression() == Compression::LZ4_HADOOP) { - // SKIP: LZ4 raw format doesn't support streaming decompression. - return; + GTEST_SKIP() << "LZ4 raw format doesn't support streaming decompression."; } auto codec = MakeCodec(); diff --git a/cpp/src/parquet/column_scanner_test.cc b/cpp/src/parquet/column_scanner_test.cc index 1d57c9b..ea54319 100644 --- a/cpp/src/parquet/column_scanner_test.cc +++ b/cpp/src/parquet/column_scanner_test.cc @@ -48,7 +48,7 @@ void InitDictValues<bool>(int num_values, int dict_per_page, std::vector<bool>& template <typename Type> class TestFlatScanner : public ::testing::Test { public: - typedef typename Type::c_type T; + using c_type = typename Type::c_type; void InitScanner(const ColumnDescriptor* d) { std::unique_ptr<PageReader> pager(new test::MockPageReader(pages_)); @@ -57,7 +57,7 @@ class TestFlatScanner : public ::testing::Test { void CheckResults(int batch_size, const ColumnDescriptor* d) { TypedScanner<Type>* scanner = reinterpret_cast<TypedScanner<Type>*>(scanner_.get()); - T val; + c_type val; bool is_null = false; int16_t def_level; int16_t rep_level; @@ -131,7 +131,7 @@ class TestFlatScanner : public ::testing::Test { int num_values_; std::vector<std::shared_ptr<Page>> pages_; std::shared_ptr<Scanner> scanner_; - std::vector<T> values_; + std::vector<c_type> values_; std::vector<int16_t> def_levels_; std::vector<int16_t> rep_levels_; std::vector<uint8_t> data_buffer_; // For BA and FLBA diff --git a/cpp/src/parquet/column_writer_test.cc b/cpp/src/parquet/column_writer_test.cc index 6694a59..752d5c4 100644 --- a/cpp/src/parquet/column_writer_test.cc +++ b/cpp/src/parquet/column_writer_test.cc @@ -64,8 +64,6 @@ const int64_t DICTIONARY_PAGE_SIZE = 1024 * 1024; template <typename TestType> class TestPrimitiveWriter : public PrimitiveTypedTest<TestType> { public: - typedef typename TestType::c_type T; - void SetUp() { this->SetupValuesOut(SMALL_SIZE); writer_properties_ = default_writer_properties(); diff --git a/cpp/src/parquet/encoding_test.cc b/cpp/src/parquet/encoding_test.cc index 7e440fa..3397ced 100644 --- a/cpp/src/parquet/encoding_test.cc +++ b/cpp/src/parquet/encoding_test.cc @@ -43,13 +43,6 @@ using arrow::default_memory_pool; using arrow::MemoryPool; using arrow::internal::checked_cast; -// TODO(hatemhelal): investigate whether this can be replaced with GTEST_SKIP in a future -// gtest release that contains https://github.com/google/googletest/pull/1544 -#define SKIP_TEST_IF(condition) \ - if (condition) { \ - return; \ - } - namespace parquet { namespace test { @@ -182,7 +175,7 @@ std::shared_ptr<ColumnDescriptor> ExampleDescr<FLBAType>() { template <typename Type> class TestEncodingBase : public ::testing::Test { public: - typedef typename Type::c_type T; + using c_type = typename Type::c_type; static constexpr int TYPE = Type::type_num; void SetUp() { @@ -195,11 +188,11 @@ class TestEncodingBase : public ::testing::Test { void InitData(int nvalues, int repeats) { num_values_ = nvalues * repeats; - input_bytes_.resize(num_values_ * sizeof(T)); - output_bytes_.resize(num_values_ * sizeof(T)); - draws_ = reinterpret_cast<T*>(input_bytes_.data()); - decode_buf_ = reinterpret_cast<T*>(output_bytes_.data()); - GenerateData<T>(nvalues, draws_, &data_buffer_); + input_bytes_.resize(num_values_ * sizeof(c_type)); + output_bytes_.resize(num_values_ * sizeof(c_type)); + draws_ = reinterpret_cast<c_type*>(input_bytes_.data()); + decode_buf_ = reinterpret_cast<c_type*>(output_bytes_.data()); + GenerateData<c_type>(nvalues, draws_, &data_buffer_); // add some repeated values for (int j = 1; j < repeats; ++j) { @@ -237,8 +230,8 @@ class TestEncodingBase : public ::testing::Test { int num_values_; int type_length_; - T* draws_; - T* decode_buf_; + c_type* draws_; + c_type* decode_buf_; std::vector<uint8_t> input_bytes_; std::vector<uint8_t> output_bytes_; std::vector<uint8_t> data_buffer_; @@ -262,7 +255,7 @@ class TestEncodingBase : public ::testing::Test { template <typename Type> class TestPlainEncoding : public TestEncodingBase<Type> { public: - typedef typename Type::c_type T; + using c_type = typename Type::c_type; static constexpr int TYPE = Type::type_num; virtual void CheckRoundtrip() { @@ -275,7 +268,7 @@ class TestPlainEncoding : public TestEncodingBase<Type> { static_cast<int>(encode_buffer_->size())); int values_decoded = decoder->Decode(decode_buf_, num_values_); ASSERT_EQ(num_values_, values_decoded); - ASSERT_NO_FATAL_FAILURE(VerifyResults<T>(decode_buf_, draws_, num_values_)); + ASSERT_NO_FATAL_FAILURE(VerifyResults<c_type>(decode_buf_, draws_, num_values_)); } void CheckRoundtripSpaced(const uint8_t* valid_bits, int64_t valid_bits_offset) { @@ -295,8 +288,8 @@ class TestPlainEncoding : public TestEncodingBase<Type> { auto values_decoded = decoder->DecodeSpaced(decode_buf_, num_values_, null_count, valid_bits, valid_bits_offset); ASSERT_EQ(num_values_, values_decoded); - ASSERT_NO_FATAL_FAILURE(VerifyResultsSpaced<T>(decode_buf_, draws_, num_values_, - valid_bits, valid_bits_offset)); + ASSERT_NO_FATAL_FAILURE(VerifyResultsSpaced<c_type>(decode_buf_, draws_, num_values_, + valid_bits, valid_bits_offset)); } protected: @@ -337,7 +330,7 @@ typedef ::testing::Types<Int32Type, Int64Type, Int96Type, FloatType, DoubleType, template <typename Type> class TestDictionaryEncoding : public TestEncodingBase<Type> { public: - typedef typename Type::c_type T; + using c_type = typename Type::c_type; static constexpr int TYPE = Type::type_num; void CheckRoundtrip() { @@ -378,14 +371,14 @@ class TestDictionaryEncoding : public TestEncodingBase<Type> { // TODO(wesm): The DictionaryDecoder must stay alive because the decoded // values' data is owned by a buffer inside the DictionaryEncoder. We // should revisit when data lifetime is reviewed more generally. - ASSERT_NO_FATAL_FAILURE(VerifyResults<T>(decode_buf_, draws_, num_values_)); + ASSERT_NO_FATAL_FAILURE(VerifyResults<c_type>(decode_buf_, draws_, num_values_)); // Also test spaced decoding decoder->SetData(num_values_, indices->data(), static_cast<int>(indices->size())); values_decoded = decoder->DecodeSpaced(decode_buf_, num_values_, 0, valid_bits.data(), 0); ASSERT_EQ(num_values_, values_decoded); - ASSERT_NO_FATAL_FAILURE(VerifyResults<T>(decode_buf_, draws_, num_values_)); + ASSERT_NO_FATAL_FAILURE(VerifyResults<c_type>(decode_buf_, draws_, num_values_)); } protected: @@ -495,7 +488,9 @@ class TestArrowBuilderDecoding : public ::testing::Test { void CheckDecodeArrowNonNullUsingDenseBuilder() { for (auto np : null_probabilities_) { InitTestCase(np); - SKIP_TEST_IF(null_count_ > 0) + if (null_count_ > 0) { + continue; + } typename EncodingTraits<ByteArrayType>::Accumulator acc; acc.builder.reset(new ::arrow::BinaryBuilder); auto actual_num_values = decoder_->DecodeArrowNonNull(num_values_, &acc); @@ -508,7 +503,9 @@ class TestArrowBuilderDecoding : public ::testing::Test { void CheckDecodeArrowNonNullUsingDictBuilder() { for (auto np : null_probabilities_) { InitTestCase(np); - SKIP_TEST_IF(null_count_ > 0) + if (null_count_ > 0) { + continue; + } auto builder = CreateDictBuilder(); auto actual_num_values = decoder_->DecodeArrowNonNull(num_values_, builder.get()); CheckDict(actual_num_values, *builder); @@ -1022,7 +1019,7 @@ TEST_F(DictEncoding, CheckDecodeIndicesNoNulls) { template <typename Type> class TestByteStreamSplitEncoding : public TestEncodingBase<Type> { public: - typedef typename Type::c_type T; + using c_type = typename Type::c_type; static constexpr int TYPE = Type::type_num; void CheckRoundtrip() override { @@ -1037,7 +1034,7 @@ class TestByteStreamSplitEncoding : public TestEncodingBase<Type> { static_cast<int>(encode_buffer_->size())); int values_decoded = decoder->Decode(decode_buf_, num_values_); ASSERT_EQ(num_values_, values_decoded); - ASSERT_NO_FATAL_FAILURE(VerifyResults<T>(decode_buf_, draws_, num_values_)); + ASSERT_NO_FATAL_FAILURE(VerifyResults<c_type>(decode_buf_, draws_, num_values_)); } { @@ -1049,14 +1046,15 @@ class TestByteStreamSplitEncoding : public TestEncodingBase<Type> { for (int i = 0; i < num_values_; i += step) { int num_decoded = decoder->Decode(decode_buf_, step); ASSERT_EQ(num_decoded, std::min(step, remaining)); - ASSERT_NO_FATAL_FAILURE(VerifyResults<T>(decode_buf_, &draws_[i], num_decoded)); + ASSERT_NO_FATAL_FAILURE( + VerifyResults<c_type>(decode_buf_, &draws_[i], num_decoded)); remaining -= num_decoded; } } { std::vector<uint8_t> valid_bits(::arrow::BitUtil::BytesForBits(num_values_), 0); - std::vector<T> expected_filtered_output; + std::vector<c_type> expected_filtered_output; const int every_nth = 5; expected_filtered_output.reserve((num_values_ + every_nth - 1) / every_nth); ::arrow::internal::BitmapWriter writer{valid_bits.data(), 0, num_values_}; @@ -1077,8 +1075,8 @@ class TestByteStreamSplitEncoding : public TestEncodingBase<Type> { static_cast<int>(encode_buffer_->size())); int values_decoded = decoder->Decode(decode_buf_, num_values_); ASSERT_EQ(expected_size, values_decoded); - ASSERT_NO_FATAL_FAILURE( - VerifyResults<T>(decode_buf_, expected_filtered_output.data(), expected_size)); + ASSERT_NO_FATAL_FAILURE(VerifyResults<c_type>( + decode_buf_, expected_filtered_output.data(), expected_size)); } } @@ -1089,11 +1087,11 @@ class TestByteStreamSplitEncoding : public TestEncodingBase<Type> { USING_BASE_MEMBERS(); void CheckDecode(const uint8_t* encoded_data, const int64_t encoded_data_size, - const T* expected_decoded_data, const int num_elements) { + const c_type* expected_decoded_data, const int num_elements) { std::unique_ptr<TypedDecoder<Type>> decoder = MakeTypedDecoder<Type>(Encoding::BYTE_STREAM_SPLIT); decoder->SetData(num_elements, encoded_data, static_cast<int>(encoded_data_size)); - std::vector<T> decoded_data(num_elements); + std::vector<c_type> decoded_data(num_elements); int num_decoded_elements = decoder->Decode(decoded_data.data(), num_elements); ASSERT_EQ(num_elements, num_decoded_elements); for (size_t i = 0U; i < decoded_data.size(); ++i) { @@ -1102,7 +1100,7 @@ class TestByteStreamSplitEncoding : public TestEncodingBase<Type> { ASSERT_EQ(0, decoder->values_left()); } - void CheckEncode(const T* data, const int num_elements, + void CheckEncode(const c_type* data, const int num_elements, const uint8_t* expected_encoded_data, const int64_t encoded_data_size) { std::unique_ptr<TypedEncoder<Type>> encoder = @@ -1117,11 +1115,12 @@ class TestByteStreamSplitEncoding : public TestEncodingBase<Type> { } }; -template <typename T> -static std::vector<T> ToLittleEndian(const std::vector<T>& input) { - std::vector<T> data(input.size()); - std::transform(input.begin(), input.end(), data.begin(), - [](const T& value) { return ::arrow::BitUtil::ToLittleEndian(value); }); +template <typename c_type> +static std::vector<c_type> ToLittleEndian(const std::vector<c_type>& input) { + std::vector<c_type> data(input.size()); + std::transform(input.begin(), input.end(), data.begin(), [](const c_type& value) { + return ::arrow::BitUtil::ToLittleEndian(value); + }); return data; } diff --git a/cpp/src/parquet/file_serialize_test.cc b/cpp/src/parquet/file_serialize_test.cc index ad9e02e..90e074f 100644 --- a/cpp/src/parquet/file_serialize_test.cc +++ b/cpp/src/parquet/file_serialize_test.cc @@ -40,8 +40,6 @@ namespace test { template <typename TestType> class TestSerialize : public PrimitiveTypedTest<TestType> { public: - typedef typename TestType::c_type T; - void SetUp() { num_columns_ = 4; num_rowgroups_ = 4; diff --git a/cpp/src/parquet/statistics_test.cc b/cpp/src/parquet/statistics_test.cc index 0828f36..2ebce7d 100644 --- a/cpp/src/parquet/statistics_test.cc +++ b/cpp/src/parquet/statistics_test.cc @@ -254,13 +254,13 @@ TEST(Comparison, UnknownSortOrder) { template <typename TestType> class TestStatistics : public PrimitiveTypedTest<TestType> { public: - using T = typename TestType::c_type; + using c_type = typename TestType::c_type; - std::vector<T> GetDeepCopy( - const std::vector<T>&); // allocates new memory for FLBA/ByteArray + std::vector<c_type> GetDeepCopy( + const std::vector<c_type>&); // allocates new memory for FLBA/ByteArray - T* GetValuesPointer(std::vector<T>&); - void DeepFree(std::vector<T>&); + c_type* GetValuesPointer(std::vector<c_type>&); + void DeepFree(std::vector<c_type>&); void TestMinMaxEncode() { this->GenerateData(1000); @@ -358,8 +358,8 @@ class TestStatistics : public PrimitiveTypedTest<TestType> { batch_num_values - batch_null_count, 1); auto beg = this->values_.begin() + i * num_values / 2; auto end = beg + batch_num_values; - std::vector<T> batch = GetDeepCopy(std::vector<T>(beg, end)); - T* batch_values_ptr = GetValuesPointer(batch); + std::vector<c_type> batch = GetDeepCopy(std::vector<c_type>(beg, end)); + c_type* batch_values_ptr = GetValuesPointer(batch); column_writer->WriteBatch(batch_num_values, definition_levels.data(), nullptr, batch_values_ptr); DeepFree(batch); @@ -482,10 +482,10 @@ void TestStatistics<ByteArrayType>::TestMinMaxEncode() { ASSERT_EQ(statistics1->max(), statistics2->max()); } -using TestTypes = ::testing::Types<Int32Type, Int64Type, FloatType, DoubleType, - ByteArrayType, FLBAType, BooleanType>; +using Types = ::testing::Types<Int32Type, Int64Type, FloatType, DoubleType, ByteArrayType, + FLBAType, BooleanType>; -TYPED_TEST_SUITE(TestStatistics, TestTypes); +TYPED_TEST_SUITE(TestStatistics, Types); TYPED_TEST(TestStatistics, MinMaxEncode) { this->SetUpSchema(Repetition::REQUIRED); @@ -609,7 +609,7 @@ static const int NUM_VALUES = 10; template <typename TestType> class TestStatisticsSortOrder : public ::testing::Test { public: - typedef typename TestType::c_type T; + using c_type = typename TestType::c_type; void AddNodes(std::string name) { fields_.push_back(schema::PrimitiveNode::Make( @@ -670,7 +670,7 @@ class TestStatisticsSortOrder : public ::testing::Test { } protected: - std::vector<T> values_; + std::vector<c_type> values_; std::vector<uint8_t> values_buf_; std::vector<schema::NodePtr> fields_; std::shared_ptr<schema::GroupNode> schema_; @@ -700,13 +700,13 @@ void TestStatisticsSortOrder<Int32Type>::SetValues() { // Write UINT32 min/max values stats_[0] - .set_min(std::string(reinterpret_cast<const char*>(&values_[5]), sizeof(T))) - .set_max(std::string(reinterpret_cast<const char*>(&values_[4]), sizeof(T))); + .set_min(std::string(reinterpret_cast<const char*>(&values_[5]), sizeof(c_type))) + .set_max(std::string(reinterpret_cast<const char*>(&values_[4]), sizeof(c_type))); // Write INT32 min/max values stats_[1] - .set_min(std::string(reinterpret_cast<const char*>(&values_[0]), sizeof(T))) - .set_max(std::string(reinterpret_cast<const char*>(&values_[9]), sizeof(T))); + .set_min(std::string(reinterpret_cast<const char*>(&values_[0]), sizeof(c_type))) + .set_max(std::string(reinterpret_cast<const char*>(&values_[9]), sizeof(c_type))); } // TYPE::INT64 @@ -728,13 +728,13 @@ void TestStatisticsSortOrder<Int64Type>::SetValues() { // Write UINT64 min/max values stats_[0] - .set_min(std::string(reinterpret_cast<const char*>(&values_[5]), sizeof(T))) - .set_max(std::string(reinterpret_cast<const char*>(&values_[4]), sizeof(T))); + .set_min(std::string(reinterpret_cast<const char*>(&values_[5]), sizeof(c_type))) + .set_max(std::string(reinterpret_cast<const char*>(&values_[4]), sizeof(c_type))); // Write INT64 min/max values stats_[1] - .set_min(std::string(reinterpret_cast<const char*>(&values_[0]), sizeof(T))) - .set_max(std::string(reinterpret_cast<const char*>(&values_[9]), sizeof(T))); + .set_min(std::string(reinterpret_cast<const char*>(&values_[0]), sizeof(c_type))) + .set_max(std::string(reinterpret_cast<const char*>(&values_[9]), sizeof(c_type))); } // TYPE::FLOAT @@ -747,8 +747,8 @@ void TestStatisticsSortOrder<FloatType>::SetValues() { // Write Float min/max values stats_[0] - .set_min(std::string(reinterpret_cast<const char*>(&values_[0]), sizeof(T))) - .set_max(std::string(reinterpret_cast<const char*>(&values_[9]), sizeof(T))); + .set_min(std::string(reinterpret_cast<const char*>(&values_[0]), sizeof(c_type))) + .set_max(std::string(reinterpret_cast<const char*>(&values_[9]), sizeof(c_type))); } // TYPE::DOUBLE @@ -761,8 +761,8 @@ void TestStatisticsSortOrder<DoubleType>::SetValues() { // Write Double min/max values stats_[0] - .set_min(std::string(reinterpret_cast<const char*>(&values_[0]), sizeof(T))) - .set_max(std::string(reinterpret_cast<const char*>(&values_[9]), sizeof(T))); + .set_min(std::string(reinterpret_cast<const char*>(&values_[0]), sizeof(c_type))) + .set_max(std::string(reinterpret_cast<const char*>(&values_[9]), sizeof(c_type))); } // TYPE::ByteArray diff --git a/cpp/src/parquet/test_util.h b/cpp/src/parquet/test_util.h index 737ecc2..e8bfd61 100644 --- a/cpp/src/parquet/test_util.h +++ b/cpp/src/parquet/test_util.h @@ -204,7 +204,7 @@ class MockPageReader : public PageReader { template <typename Type> class DataPageBuilder { public: - typedef typename Type::c_type T; + using c_type = typename Type::c_type; // This class writes data and metadata to the passed inputs explicit DataPageBuilder(ArrowOutputStream* sink) @@ -235,7 +235,7 @@ class DataPageBuilder { have_rep_levels_ = true; } - void AppendValues(const ColumnDescriptor* d, const std::vector<T>& values, + void AppendValues(const ColumnDescriptor* d, const std::vector<c_type>& values, Encoding::type encoding = Encoding::PLAIN) { std::shared_ptr<Buffer> values_sink = EncodeValues<Type>( encoding, false, values.data(), static_cast<int>(values.size()), d); @@ -610,7 +610,7 @@ inline std::string TestColumnName(int i) { template <typename TestType> class PrimitiveTypedTest : public ::testing::Test { public: - typedef typename TestType::c_type T; + using c_type = typename TestType::c_type; void SetUpSchema(Repetition::type repetition, int num_columns = 1) { std::vector<schema::NodePtr> fields; @@ -633,19 +633,19 @@ class PrimitiveTypedTest : public ::testing::Test { SchemaDescriptor schema_; // Input buffers - std::vector<T> values_; + std::vector<c_type> values_; std::vector<int16_t> def_levels_; std::vector<uint8_t> buffer_; // Pointer to the values, needed as we cannot use std::vector<bool>::data() - T* values_ptr_; + c_type* values_ptr_; std::vector<uint8_t> bool_buffer_; // Output buffers - std::vector<T> values_out_; + std::vector<c_type> values_out_; std::vector<uint8_t> bool_buffer_out_; - T* values_out_ptr_; + c_type* values_out_ptr_; }; template <typename TestType> @@ -654,7 +654,7 @@ inline void PrimitiveTypedTest<TestType>::SyncValuesOut() {} template <> inline void PrimitiveTypedTest<BooleanType>::SyncValuesOut() { std::vector<uint8_t>::const_iterator source_iterator = bool_buffer_out_.begin(); - std::vector<T>::iterator destination_iterator = values_out_.begin(); + std::vector<c_type>::iterator destination_iterator = values_out_.begin(); while (source_iterator != bool_buffer_out_.end()) { *destination_iterator++ = *source_iterator++ != 0; } @@ -685,7 +685,7 @@ inline void PrimitiveTypedTest<TestType>::GenerateData(int64_t num_values) { def_levels_.resize(num_values); values_.resize(num_values); - InitValues<T>(static_cast<int>(num_values), values_, buffer_); + InitValues<c_type>(static_cast<int>(num_values), values_, buffer_); values_ptr_ = values_.data(); std::fill(def_levels_.begin(), def_levels_.end(), 1); @@ -696,7 +696,7 @@ inline void PrimitiveTypedTest<BooleanType>::GenerateData(int64_t num_values) { def_levels_.resize(num_values); values_.resize(num_values); - InitValues<T>(static_cast<int>(num_values), values_, buffer_); + InitValues<c_type>(static_cast<int>(num_values), values_, buffer_); bool_buffer_.resize(num_values); std::copy(values_.begin(), values_.end(), bool_buffer_.begin()); values_ptr_ = reinterpret_cast<bool*>(bool_buffer_.data()); diff --git a/cpp/thirdparty/versions.txt b/cpp/thirdparty/versions.txt index 969de58..3a682e5 100644 --- a/cpp/thirdparty/versions.txt +++ b/cpp/thirdparty/versions.txt @@ -36,7 +36,7 @@ ARROW_GBENCHMARK_BUILD_VERSION=v1.5.2 ARROW_GFLAGS_BUILD_VERSION=v2.2.2 ARROW_GLOG_BUILD_VERSION=v0.4.0 ARROW_GRPC_BUILD_VERSION=v1.29.1 -ARROW_GTEST_BUILD_VERSION=1.8.1 +ARROW_GTEST_BUILD_VERSION=1.10.0 ARROW_JEMALLOC_BUILD_VERSION=5.2.1 ARROW_LZ4_BUILD_VERSION=v1.9.2 ARROW_MIMALLOC_BUILD_VERSION=v1.6.4