test: Make Arrow C++ dependency optional (#677)

apache · Nov 19, 2024 · 2930787 · 2930787
1 parent 9945d33
commit 2930787
Show file tree

Hide file tree

Showing 15 changed files with 331 additions and 44 deletions.
diff --git a/.github/workflows/build-and-test-device.yaml b/.github/workflows/build-and-test-device.yaml
@@ -98,8 +98,8 @@ jobs:
           cd build
           export VERBOSE=1
           cmake .. -DCMAKE_BUILD_TYPE=Debug -DNANOARROW_DEVICE=ON \
-            -DNANOARROW_BUILD_TESTS=ON -DCMAKE_PREFIX_PATH="${ARROW_PATH}" \
-            ${{ matrix.config.cmake_args }}
+            -DNANOARROW_BUILD_TESTS=ON -DNANOARROW_BUILD_TESTS_WITH_ARROW=ON \
+            -DCMAKE_PREFIX_PATH="${ARROW_PATH}" ${{ matrix.config.cmake_args }}
 
           cmake --build .
 

diff --git a/.github/workflows/build-and-test-ipc.yaml b/.github/workflows/build-and-test-ipc.yaml
@@ -91,8 +91,8 @@ jobs:
           cd build
 
           cmake .. -DCMAKE_BUILD_TYPE=Debug -DNANOARROW_BUILD_TESTS=ON \
-            -DNANOARROW_IPC=ON -DCMAKE_PREFIX_PATH="${ARROW_PATH}" \
-            ${{ matrix.config.cmake_args }}
+            -DNANOARROW_BUILD_TESTS_WITH_ARROW=ON  -DNANOARROW_IPC=ON \
+            -DCMAKE_PREFIX_PATH="${ARROW_PATH}" ${{ matrix.config.cmake_args }}
 
           cmake --build .
 

diff --git a/.github/workflows/build-and-test.yaml b/.github/workflows/build-and-test.yaml
@@ -82,7 +82,8 @@ jobs:
           cd build
 
           cmake .. -DNANOARROW_BUILD_TESTS=ON -DCMAKE_POSITION_INDEPENDENT_CODE=ON \
-            -DCMAKE_PREFIX_PATH="${ARROW_PATH}" ${{ matrix.config.cmake_args }}
+            -DNANOARROW_BUILD_TESTS_WITH_ARROW=ON -DCMAKE_PREFIX_PATH="${ARROW_PATH}" \
+            ${{ matrix.config.cmake_args }}
 
           cmake --build .
 
@@ -158,3 +159,21 @@ jobs:
       - name: Run meson testing script
         run: |
           PKG_CONFIG_PATH="$(pwd)/arrow/lib/pkgconfig" ci/scripts/build-with-meson.sh
+
+  test-no-arrow:
+    name: test-no-arrow
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Build nanoarrow
+        run: |
+          cmake -S . -B build -DNANOARROW_BUILD_TESTS=ON -DNANOARROW_BUILD_TESTS_WITH_ARROW=OFF \
+          -DCMAKE_POSITION_INDEPENDENT_CODE=ON -DCMAKE_PREFIX_PATH="${ARROW_PATH}"
+
+          cmake --build build
+
+      - name: Run tests
+        run: |
+          cd build
+          ctest -T test --output-on-failure .
diff --git a/.github/workflows/clang-tidy.yaml b/.github/workflows/clang-tidy.yaml
@@ -66,9 +66,9 @@ jobs:
           cd build
 
           cmake .. -DNANOARROW_DEVICE=ON -DNANOARROW_IPC=ON \
-            -DNANOARROW_BUILD_TESTS=ON -DCMAKE_POSITION_INDEPENDENT_CODE=ON \
-            -DCMAKE_BUILD_TYPE=Debug -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
-            -DCMAKE_PREFIX_PATH="${ARROW_PATH}"
+            -DNANOARROW_BUILD_TESTS=ON -DNANOARROW_BUILD_TESTS_WITH_ARROW=ON \
+            -DCMAKE_POSITION_INDEPENDENT_CODE=ON -DCMAKE_BUILD_TYPE=Debug \
+            -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_PREFIX_PATH="${ARROW_PATH}"
 
           cmake --build .
 

diff --git a/.github/workflows/verify.yaml b/.github/workflows/verify.yaml
@@ -101,13 +101,21 @@ jobs:
         if: matrix.config.label == 'windows'
         shell: bash
         run: |
-          echo "NANOARROW_CMAKE_OPTIONS=${NANOARROW_CMAKE_OPTIONS} -DNANOARROW_ARROW_STATIC=ON -DArrow_DIR=$(pwd -W)/arrow/lib/cmake/Arrow -Dgtest_force_shared_crt=ON" >> $GITHUB_ENV
+          cat << EOF | tr '\n' ' ' >> $GITHUB_ENV
+          NANOARROW_CMAKE_OPTIONS=${NANOARROW_CMAKE_OPTIONS}
+          -DNANOARROW_ARROW_STATIC=ON
+          -DNANOARROW_BUILD_TESTS_WITH_ARROW=ON
+          -DArrow_DIR=$(pwd -W)/arrow/lib/cmake/Arrow
+          -Dgtest_force_shared_crt=ON
 
       - name: Set CMake options (POSIX)
         if: matrix.config.label != 'windows'
         shell: bash
         run: |
-          echo "NANOARROW_CMAKE_OPTIONS=${NANOARROW_CMAKE_OPTIONS} -DArrow_DIR=$(pwd)/arrow/lib/cmake/Arrow" >> $GITHUB_ENV
+          cat << EOF | tr '\n' ' ' >> $GITHUB_ENV
+          NANOARROW_CMAKE_OPTIONS=${NANOARROW_CMAKE_OPTIONS}
+          -DNANOARROW_BUILD_TESTS_WITH_ARROW=ON
+          -DArrow_DIR=$(pwd)/arrow/lib/cmake/Arrow
 
       - name: Run dev/release/verify-release-candidate.sh
         shell: bash

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -45,6 +45,7 @@ option(NANOARROW_DEVICE_WITH_CUDA "Build CUDA libraries" OFF)
 # Development options
 option(NANOARROW_BUILD_APPS "Build utility applications" OFF)
 option(NANOARROW_BUILD_TESTS "Build tests" OFF)
+option(NANOARROW_BUILD_TESTS_WITH_ARROW "Build tests that require Arrow" OFF)
 option(NANOARROW_BUILD_BENCHMARKS "Build benchmarks" OFF)
 option(NANOARROW_BUILD_INTEGRATION_TESTS
        "Build cross-implementation Arrow integration tests" OFF)
@@ -422,26 +423,28 @@ if(NANOARROW_BUILD_TESTS)
   )
   include(CTest)
 
-  find_package(Arrow REQUIRED)
-  message(STATUS "Arrow version: ${ARROW_VERSION}")
-  message(STATUS "Arrow SO version: ${ARROW_FULL_SO_VERSION}")
+  if(NANOARROW_BUILD_TESTS_WITH_ARROW)
+    find_package(Arrow REQUIRED)
+    message(STATUS "Arrow version: ${ARROW_VERSION}")
+    message(STATUS "Arrow SO version: ${ARROW_FULL_SO_VERSION}")
 
-  # Give caller the option to link a static version of Arrow C++
-  if(NANOARROW_ARROW_STATIC)
-    set(NANOARROW_ARROW_TARGET arrow_static)
-  else()
-    set(NANOARROW_ARROW_TARGET arrow_shared)
-  endif()
+    # Give caller the option to link a static version of Arrow C++
+    if(NANOARROW_ARROW_STATIC)
+      set(NANOARROW_ARROW_TARGET arrow_static)
+    else()
+      set(NANOARROW_ARROW_TARGET arrow_shared)
+    endif()
 
-  # Arrow >= 10.0.0 requires C++17; GTest requires C++11.
-  # Leave the option open to use an older version of Arrow
-  # to make it easier to test on old Linux (e.g., Centos7)
-  if(${ARROW_VERSION} VERSION_GREATER_EQUAL "10.0.0")
-    set(CMAKE_CXX_STANDARD 17)
-  else()
-    set(CMAKE_CXX_STANDARD 11)
+    # Arrow >= 10.0.0 requires C++17; GTest requires C++11.
+    # Leave the option open to use an older version of Arrow
+    # to make it easier to test on old Linux (e.g., Centos7)
+    if(${ARROW_VERSION} VERSION_GREATER_EQUAL "10.0.0")
+      set(CMAKE_CXX_STANDARD 17)
+    else()
+      set(CMAKE_CXX_STANDARD 11)
+    endif()
+    set(CMAKE_CXX_STANDARD_REQUIRED ON)
   endif()
-  set(CMAKE_CXX_STANDARD_REQUIRED ON)
 
   add_subdirectory("thirdparty/googletest")
 
@@ -517,6 +520,27 @@ if(NANOARROW_BUILD_TESTS)
                         gmock_main
                         nanoarrow_coverage_config)
 
+  list(APPEND
+       NanoarrowTests
+       utils_test
+       buffer_test
+       array_test
+       schema_test
+       array_stream_test
+       nanoarrow_testing_test
+       c_data_integration_test
+       hpp_array_stream
+       hpp_buffer
+       hpp_exception
+       hpp_unique
+       hpp_view)
+  if(Arrow_FOUND)
+    foreach(test_target ${NanoarrowTests})
+      target_compile_definitions(${test_target}
+                                 PRIVATE -DNANOARROW_BUILD_TESTS_WITH_ARROW)
+    endforeach()
+  endif()
+
   include(GoogleTest)
   # Some users have reported a timeout with the default value of 5
   # Building with -DBUILD_SHARED_LIBS=ON may also help reduce the size of test
@@ -569,6 +593,11 @@ if(NANOARROW_BUILD_TESTS)
         target_link_libraries(nanoarrow_ipc_${name}_test flatccrt)
       endif()
 
+      if(Arrow_FOUND)
+        target_compile_definitions(nanoarrow_ipc_${name}_test
+                                   PRIVATE -DNANOARROW_BUILD_TESTS_WITH_ARROW)
+      endif()
+
       gtest_discover_tests(nanoarrow_ipc_${name}_test)
     endforeach()
 
@@ -593,6 +622,13 @@ if(NANOARROW_BUILD_TESTS)
                           gtest_main
                           nanoarrow_coverage_config)
 
+    if(Arrow_FOUND)
+      target_compile_definitions(nanoarrow_device_test
+                                 PRIVATE -DNANOARROW_BUILD_TESTS_WITH_ARROW)
+      target_compile_definitions(nanoarrow_device_hpp_test
+                                 PRIVATE -DNANOARROW_BUILD_TESTS_WITH_ARROW)
+    endif()
+
     include(GoogleTest)
     gtest_discover_tests(nanoarrow_device_test)
     gtest_discover_tests(nanoarrow_device_hpp_test)
@@ -604,6 +640,10 @@ if(NANOARROW_BUILD_TESTS)
                             nanoarrow
                             gtest_main
                             nanoarrow_coverage_config)
+      if(Arrow_FOUND)
+        target_compile_definitions(nanoarrow_device_metal_test
+                                   PRIVATE -DNANOARROW_BUILD_TESTS_WITH_ARROW)
+      endif()
       gtest_discover_tests(nanoarrow_device_metal_test)
     endif()
 
@@ -615,6 +655,10 @@ if(NANOARROW_BUILD_TESTS)
                             CUDA::cuda_driver
                             gtest_main
                             nanoarrow_coverage_config)
+      if(Arrow_FOUND)
+        target_compile_definitions(nanoarrow_device_cuda_test
+                                   PRIVATE -DNANOARROW_BUILD_TESTS_WITH_ARROW)
+      endif()
       gtest_discover_tests(nanoarrow_device_cuda_test)
     endif()
   endif()

diff --git a/README.md b/README.md
@@ -127,16 +127,23 @@ cmake ..
 cmake --build .
 ```
 
-Building nanoarrow with tests currently requires [Arrow C++](https://arrow.apache.org/install/).
-If installed via a system package manager like `apt`, `dnf`, or `brew`, the tests can be
-built with:
+To build nanoarrow along with tests run:
 
 ```sh
 mkdir build && cd build
 cmake .. -DNANOARROW_BUILD_TESTS=ON
 cmake --build .
 ```
 
+If you are able to install [Arrow C++](https://arrow.apache.org/install/) you can enable
+more testing:
+
+```sh
+mkdir build && cd build
+cmake .. -DNANOARROW_BUILD_TESTS=ON -DNANOARROW_BUILD_TESTS_WITH_ARROW=ON
+cmake --build .
+```
+
 Tests can be run with `ctest`.
 
 ### Building with Meson
@@ -155,9 +162,9 @@ After setting up your project, be sure to enable the options you want:
 meson configure -Dtests=true -Dbenchmarks=true
 ```
 
-If Arrow is installed in a non-standard location on your system, you may need to
-pass the `--pkg-config-path <path to directory with arrow.pc>` argument to either
-the setup or configure steps above.
+You can enable better test coverage if Apache Arrow is installed on your system
+with `-Dtest_with_arrow=true`. Depending on how you have installed Apache Arrow,
+you may also need to pass `--pkg-config-path <path to directory with arrow.pc>`.
 
 With the above out of the way, the `compile` command should take care of the rest:
 

diff --git a/ci/scripts/coverage.sh b/ci/scripts/coverage.sh
@@ -74,7 +74,8 @@ function main() {
 
     cmake "${TARGET_NANOARROW_DIR}" \
           -DNANOARROW_DEVICE=ON -DNANOARROW_IPC=ON \
-          -DNANOARROW_BUILD_TESTS=ON -DNANOARROW_CODE_COVERAGE=ON
+          -DNANOARROW_BUILD_TESTS=ON -DNANOARROW_BUILD_TESTS_WITH_ARROW=ON \
+          -DNANOARROW_CODE_COVERAGE=ON
     cmake --build .
     CTEST_OUTPUT_ON_FAILURE=1 ctest .
 

diff --git a/meson.build b/meson.build
@@ -208,8 +208,16 @@ if get_option('tests')
     # Similarly code coverage has a built in option users should use instead
     # https://mesonbuild.com/Unit-tests.html#coverage
 
-    # The system include suppresses compilation warnings from Arrow
-    arrow_dep = dependency('arrow', include_type: 'system')
+    arrow_dep = dependency('arrow', include_type: 'system', required: false)
+    if get_option('tests_with_arrow') and not arrow_dep.found()
+        error('tests_with_arrow=true but could not find Arrow')
+    endif
+
+    test_cpp_args = []
+    if get_option('tests_with_arrow')
+        test_cpp_args += ['-DNANOARROW_BUILD_TESTS_WITH_ARROW']
+    endif
+
     gtest_dep = dependency('gtest_main')
     gmock_dep = dependency('gmock')
 

diff --git a/meson.options b/meson.options
@@ -16,6 +16,7 @@
 # under the License.
 
 option('tests', type: 'boolean', description: 'Build tests', value: false)
+option('tests_with_arrow', type: 'boolean', description: 'Build tests with Arrow', value: false)
 option('benchmarks', type: 'boolean', description: 'Build benchmarks', value: false)
 option('apps', type: 'boolean', description: 'Build utility applications', value: false)
 option('ipc', type: 'boolean', description: 'Build IPC libraries', value: false)