ARROW-3988: [C++] Do not build unit tests by default, fix building Gandiva unit tests when ARROW_BUILD_TESTS=OFF

wesm · wesm · commit a3ba1a2b54af · 2018-12-12T09:19:41.000-06:00
I found while working on this that disabling `ARROW_GANDIVA_BUILD_TESTS` would break the build -- I think this was caused by some other changes I made. We should remove that option and instead use the new modular build targets and invoke unit tests using labels. So we would write ``` ninja gandiva # this will build all libraries and unit tests when ARROW_BUILD_TESTS=ON ctest -L gandiva ``` Author: Wes McKinney <wesm+git@apache.org> Closes apache#3156 from wesm/ARROW-3988 and squashes the following commits: 0420f9e <Wes McKinney> Remove arrow::PrimitiveBuilder from builder.rst for now because of Sphinx warning f8a33a5 <Wes McKinney> Fix gandiva test flag c489353 <Wes McKinney> Add ARROW_BUILD_TESTS to appveyor-cpp-test-cmake-script.bat 5c6a332 <Wes McKinney> Do not build unit tests by default, fix building Gandiva unit tests when ARROW_BUILD_TESTS=OFF
diff --git a/ci/appveyor-cpp-build.bat b/ci/appveyor-cpp-build.bat
@@ -34,6 +34,7 @@ if "%JOB%" == "Static_Crt_Build" (
         -DARROW_USE_STATIC_CRT=ON ^
         -DARROW_BOOST_USE_SHARED=OFF ^
         -DARROW_BUILD_SHARED=OFF ^
+        -DARROW_BUILD_TESTS=ON ^
         -DCMAKE_BUILD_TYPE=Debug ^
         -DARROW_TEST_LINKAGE=static ^
         -DARROW_CXXFLAGS="/MP" ^
@@ -51,6 +52,7 @@ if "%JOB%" == "Static_Crt_Build" (
         -DARROW_USE_STATIC_CRT=ON ^
         -DARROW_BOOST_USE_SHARED=OFF ^
         -DARROW_BUILD_SHARED=OFF ^
+        -DARROW_BUILD_TESTS=ON ^
         -DCMAKE_BUILD_TYPE=Release ^
         -DARROW_TEST_LINKAGE=static ^
         -DCMAKE_CXX_FLAGS_RELEASE="/MT %CMAKE_CXX_FLAGS_RELEASE%" ^
@@ -76,6 +78,7 @@ if "%JOB%" == "Build_Debug" (
   cmake -G "%GENERATOR%" ^
         -DARROW_VERBOSE_THIRDPARTY_BUILD=OFF ^
         -DARROW_BOOST_USE_SHARED=OFF ^
+        -DARROW_BUILD_TESTS=ON ^
         -DCMAKE_BUILD_TYPE=%CONFIGURATION% ^
         -DARROW_BUILD_STATIC=OFF ^
         -DARROW_CXXFLAGS="/MP" ^
diff --git a/ci/appveyor-cpp-test-cmake-script.bat b/ci/appveyor-cpp-test-cmake-script.bat
@@ -32,6 +32,7 @@ set FLATBUFFERS_HOME=WrongPath
 
 cmake -G "%GENERATOR%" ^
       -DARROW_BOOST_USE_SHARED=OFF ^
+      -DARROW_BUILD_TESTS=ON ^
       -DCMAKE_BUILD_TYPE=%CONFIGURATION% ^
       -DARROW_CXXFLAGS="/MP" ^
       .. >nul 2>error.txt
@@ -49,6 +50,7 @@ set GFLAGS_HOME=WrongPath
 
 cmake -G "%GENERATOR%" ^
       -DARROW_BOOST_USE_SHARED=OFF ^
+      -DARROW_BUILD_TESTS=ON ^
       -DCMAKE_BUILD_TYPE=%CONFIGURATION% ^
       -DARROW_CXXFLAGS="/MP" ^
       .. >nul 2>error.txt
@@ -66,6 +68,7 @@ set SNAPPY_HOME=WrongPath
 
 cmake -G "%GENERATOR%" ^
       -DARROW_BOOST_USE_SHARED=OFF ^
+      -DARROW_BUILD_TESTS=ON ^
       -DCMAKE_BUILD_TYPE=%CONFIGURATION% ^
       -DARROW_CXXFLAGS="/MP" ^
       .. >nul 2>error.txt
@@ -83,6 +86,7 @@ set ZLIB_HOME=WrongPath
 
 cmake -G "%GENERATOR%" ^
       -DARROW_BOOST_USE_SHARED=OFF ^
+      -DARROW_BUILD_TESTS=ON ^
       -DCMAKE_BUILD_TYPE=%CONFIGURATION% ^
       -DARROW_CXXFLAGS="/MP" ^
       .. >nul 2>error.txt
@@ -100,6 +104,7 @@ set BROTLI_HOME=WrongPath
 
 cmake -G "%GENERATOR%" ^
       -DARROW_BOOST_USE_SHARED=OFF ^
+      -DARROW_BUILD_TESTS=ON ^
       -DCMAKE_BUILD_TYPE=%CONFIGURATION% ^
       -DARROW_CXXFLAGS="/MP" ^
       .. >nul 2>error.txt
@@ -117,6 +122,7 @@ set LZ4_HOME=WrongPath
 
 cmake -G "%GENERATOR%" ^
       -DARROW_BOOST_USE_SHARED=OFF ^
+      -DARROW_BUILD_TESTS=ON ^
       -DCMAKE_BUILD_TYPE=%CONFIGURATION% ^
       -DARROW_CXXFLAGS="/MP" ^
       .. >nul 2>error.txt
@@ -134,6 +140,7 @@ set ZSTD_HOME=WrongPath
 
 cmake -G "%GENERATOR%" ^
       -DARROW_BOOST_USE_SHARED=OFF ^
+      -DARROW_BUILD_TESTS=ON ^
       -DCMAKE_BUILD_TYPE=%CONFIGURATION% ^
       -DARROW_CXXFLAGS="/MP" ^
       .. >nul 2>error.txt
@@ -158,6 +165,7 @@ pushd %BUILD_DIR%
 set ARROW_BUILD_TOOLCHAIN=%CONDA_PREFIX%\Library
 cmake -G "%GENERATOR%" ^
       -DARROW_BOOST_USE_SHARED=OFF ^
+      -DARROW_BUILD_TESTS=ON ^
       -DCMAKE_BUILD_TYPE=%CONFIGURATION% ^
       -DARROW_CXXFLAGS="/MP" ^
       .. 2>output.txt
diff --git a/ci/cpp-msvc-build-main.bat b/ci/cpp-msvc-build-main.bat
@@ -48,6 +48,7 @@ cmake -G "%GENERATOR%" %CMAKE_ARGS% ^
       -DARROW_BOOST_USE_SHARED=OFF ^
       -DCMAKE_BUILD_TYPE=%CONFIGURATION% ^
       -DARROW_BUILD_STATIC=OFF ^
+      -DARROW_BUILD_TESTS=ON ^
       -DARROW_CXXFLAGS="%ARROW_CXXFLAGS%" ^
       -DCMAKE_CXX_FLAGS_RELEASE="/MD %CMAKE_CXX_FLAGS_RELEASE%" ^
       -DARROW_PARQUET=ON ^
diff --git a/ci/travis_before_script_cpp.sh b/ci/travis_before_script_cpp.sh
@@ -41,7 +41,6 @@ if [ "$only_library_mode" == "no" ]; then
 fi
 
 CMAKE_COMMON_FLAGS="\
--DARROW_BUILD_BENCHMARKS=ON \
 -DCMAKE_INSTALL_PREFIX=$ARROW_CPP_INSTALL \
 -DARROW_NO_DEPRECATED_API=ON \
 -DARROW_EXTRA_ERROR_CONTEXT=ON"
@@ -61,7 +60,13 @@ pushd $ARROW_CPP_BUILD_DIR
 if [ $only_library_mode == "yes" ]; then
   CMAKE_COMMON_FLAGS="\
 $CMAKE_COMMON_FLAGS \
--DARROW_BUILD_TESTS=OFF \
+-DARROW_BUILD_UTILITIES=OFF \
+-DARROW_INSTALL_NAME_RPATH=OFF"
+else
+  CMAKE_COMMON_FLAGS="\
+$CMAKE_COMMON_FLAGS \
+-DARROW_BUILD_BENCHMARKS=ON \
+-DARROW_BUILD_TESTS=ON \
 -DARROW_BUILD_UTILITIES=OFF \
 -DARROW_INSTALL_NAME_RPATH=OFF"
 fi
@@ -92,6 +97,9 @@ fi
 
 if [ $ARROW_TRAVIS_GANDIVA == "1" ]; then
   CMAKE_COMMON_FLAGS="$CMAKE_COMMON_FLAGS -DARROW_GANDIVA=ON"
+  if [ $only_library_mode == "no" ]; then
+    CMAKE_COMMON_FLAGS="$CMAKE_COMMON_FLAGS -DARROW_GANDIVA_BUILD_TESTS=ON"
+  fi
 fi
 
 if [ $ARROW_TRAVIS_VALGRIND == "1" ]; then
diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt
@@ -115,8 +115,12 @@ if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
     OFF)
 
   option(ARROW_BUILD_TESTS
-    "Build the Arrow googletest unit tests"
-    ON)
+    "Build the Arrow googletest unit tests, default OFF"
+    OFF)
+
+  option(ARROW_BUILD_BENCHMARKS
+    "Build the Arrow micro benchmarks, default OFF"
+    OFF)
 
   set(ARROW_TEST_LINKAGE "shared" CACHE STRING
     "Linkage of Arrow libraries with unit tests executables. \
@@ -126,10 +130,6 @@ static|shared (default shared)")
     "Only build unit tests having the indicated label or labels. \
 Pass multiple labels by dividing with semicolons")
 
-  option(ARROW_BUILD_BENCHMARKS
-    "Build the Arrow micro benchmarks"
-    OFF)
-
   option(ARROW_NO_DEPRECATED_API
     "Exclude deprecated APIs from build"
     OFF)
@@ -322,7 +322,7 @@ Always OFF if building binaries"
 
   option(ARROW_GANDIVA_BUILD_TESTS
     "Build the Gandiva googletest unit tests"
-    ON)
+    OFF)
 
 endif()
 
diff --git a/cpp/README.md b/cpp/README.md
@@ -64,7 +64,7 @@ Simple debug build:
     cd arrow/cpp
     mkdir debug
     cd debug
-    cmake ..
+    cmake -DARROW_BUILD_TESTS=ON ..
     make unittest
 
 Simple release build:
@@ -73,10 +73,14 @@ Simple release build:
     cd arrow/cpp
     mkdir release
     cd release
-    cmake .. -DCMAKE_BUILD_TYPE=Release
+    cmake -DARROW_BUILD_TESTS=ON -DCMAKE_BUILD_TYPE=Release ..
     make unittest
 
-Detailed unit test logs will be placed in the build directory under `build/test-logs`.
+If you do not need to build the test suite, you can omit the
+`ARROW_BUILD_TESTS` option (the default is not to build the unit tests).
+
+Detailed unit test logs will be placed in the build directory under
+`build/test-logs`.
 
 On some Linux distributions, running the test suite might require setting an
 explicit locale. If you see any locale-related errors, try setting the
@@ -132,7 +136,7 @@ not use the macro.
 Follow the directions for simple build except run cmake
 with the `--ARROW_BUILD_BENCHMARKS` parameter set correctly:
 
-    cmake -DARROW_BUILD_BENCHMARKS=ON ..
+    cmake -DARROW_BUILD_TESTS=ON -DARROW_BUILD_BENCHMARKS=ON ..
 
 and instead of make unittest run either `make; ctest` to run both unit tests
 and benchmarks or `make benchmark` to run only the benchmark tests.
@@ -265,7 +269,7 @@ The optional `gandiva` libraries and tests can be built by passing
 `-DARROW_GANDIVA=on`.
 
 ```shell
-cmake .. -DARROW_GANDIVA=on
+cmake .. -DARROW_GANDIVA=ON -DARROW_GANDIVA_BUILD_TESTS=ON
 make
 ctest -L gandiva
 ```
diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -525,9 +525,11 @@ message(STATUS "double-conversion static library: ${DOUBLE_CONVERSION_STATIC_LIB
 # ----------------------------------------------------------------------
 # Google gtest & gflags
 
-if(ARROW_BUILD_TESTS OR ARROW_BUILD_BENCHMARKS)
-  add_custom_target(unittest ctest -L unittest)
+add_custom_target(unittest ctest -L unittest)
+add_custom_target(benchmark ctest -L benchmark)
 
+if(ARROW_BUILD_TESTS OR ARROW_GANDIVA_BUILD_TESTS
+   OR ARROW_BUILD_BENCHMARKS)
   if("${GTEST_HOME}" STREQUAL "")
     if(APPLE)
       set(GTEST_CMAKE_CXX_FLAGS "-fPIC -DGTEST_USE_OWN_TR1_TUPLE=1 -Wno-unused-value -Wno-ignored-attributes")
@@ -627,7 +629,6 @@ if(ARROW_BUILD_TESTS OR ARROW_BUILD_BENCHMARKS)
 endif()
 
 if(ARROW_BUILD_BENCHMARKS)
-  add_custom_target(benchmark ctest -L benchmark)
 
   if("$ENV{GBENCHMARK_HOME}" STREQUAL "")
     if(NOT MSVC)
diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt
@@ -213,7 +213,8 @@ if (ARROW_BUILD_STATIC AND WIN32)
   target_compile_definitions(arrow_static PUBLIC ARROW_STATIC)
 endif()
 
-if (ARROW_BUILD_TESTS OR ARROW_BUILD_BENCHMARKS)
+if (ARROW_BUILD_TESTS OR ARROW_GANDIVA_BUILD_TESTS
+    OR ARROW_BUILD_BENCHMARKS)
   # that depend on gtest
   ADD_ARROW_LIB(arrow_testing
     SOURCES test-util.cc
diff --git a/docs/source/cpp/api/builder.rst b/docs/source/cpp/api/builder.rst
@@ -31,9 +31,6 @@ Concrete builder subclasses
 .. doxygenclass:: arrow::BooleanBuilder
    :members:
 
-.. doxygenclass:: arrow::PrimitiveBuilder
-   :members:
-
 .. doxygenclass:: arrow::NumericBuilder
    :members:
 
