Decouple the building of benchmarks and tests (#491)

Summary: X-link: pytorch/torcharrow#491 Tests and benchmarks targets are now de-coupled. That means they can be built independently. Shared functionality is moved to a common utility library. Resolves #1704 Pull Request resolved: #2439 Reviewed By: Yuhta Differential Revision: D39484543 Pulled By: kgpai fbshipit-source-id: 5ac888c81a6bbfbc5a1a1c4cfd41fa2c86199bc4
facebookincubator · Sep 15, 2022 · 7b518c3 · 7b518c3
1 parent 3dd1f01
commit 7b518c3
Show file tree

Hide file tree

Showing 192 changed files with 272 additions and 221 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -91,6 +91,7 @@ endif()
 if(${VELOX_ENABLE_BENCHMARKS} OR ${VELOX_ENABLE_BENCHMARKS_BASIC})
   set(VELOX_ENABLE_DUCKDB ON)
   set(VELOX_ENABLE_PARSE ON)
+  set(VELOX_BUILD_TEST_UTILS ON)
 endif()
 
 if(VELOX_ENABLE_S3)
@@ -313,12 +314,8 @@ if(VELOX_BUILD_TESTING AND NOT VELOX_ENABLE_DUCKDB)
   )
 endif()
 
-# Benchmarks and tests at some places are coupled which is not great. See
-# velox/vector/CMakeLists.txt. TODO: Decouple.
 set(VELOX_DISABLE_GOOGLETEST OFF)
-if(NOT VELOX_BUILD_TEST_UTILS
-   AND NOT VELOX_BUILD_TESTING
-   AND NOT VELOX_ENABLE_BENCHMARKS_BASIC)
+if(NOT VELOX_BUILD_TEST_UTILS AND NOT VELOX_BUILD_TESTING)
   set(VELOX_DISABLE_GOOGLETEST ON)
   add_definitions(-DVELOX_DISABLE_GOOGLETEST)
 endif()

diff --git a/velox/benchmarks/CMakeLists.txt b/velox/benchmarks/CMakeLists.txt
@@ -11,8 +11,8 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-
 add_subdirectory(basic)
+
 if(${VELOX_ENABLE_BENCHMARKS})
   add_subdirectory(tpch)
 endif()
diff --git a/velox/benchmarks/basic/Preproc.cpp b/velox/benchmarks/basic/Preproc.cpp
@@ -22,7 +22,7 @@
 #include "velox/functions/lib/benchmarks/FunctionBenchmarkBase.h"
 #include "velox/functions/prestosql/Comparisons.h"
 #include "velox/functions/prestosql/registration/RegistrationFunctions.h"
-#include "velox/vector/tests/VectorTestBase.h"
+#include "velox/vector/tests/utils/VectorTestBase.h"
 
 using namespace facebook::velox;
 using namespace facebook::velox::exec;

diff --git a/velox/benchmarks/tpch/CMakeLists.txt b/velox/benchmarks/tpch/CMakeLists.txt
@@ -18,7 +18,7 @@ target_link_libraries(
   velox_tpch_benchmark
   velox_aggregates
   velox_exec
-  velox_exec_test_util
+  velox_exec_test_lib
   velox_dwio_common
   velox_dwio_common_exception
   velox_dwio_parquet_reader

diff --git a/velox/common/file/benchmark/CMakeLists.txt b/velox/common/file/benchmark/CMakeLists.txt
@@ -15,7 +15,7 @@
 add_library(velox_read_benchmark_lib ReadBenchmark.cpp)
 
 target_link_libraries(velox_read_benchmark_lib velox_file velox_exception
-                      velox_exec_test_util ${FMT} ${FOLLY_WITH_DEPENDENCIES})
+                      velox_exec_test_lib ${FMT} ${FOLLY_WITH_DEPENDENCIES})
 
 add_executable(velox_read_benchmark ReadBenchmarkMain.cpp)
 

diff --git a/velox/common/file/tests/CMakeLists.txt b/velox/common/file/tests/CMakeLists.txt
@@ -18,7 +18,7 @@ target_link_libraries(
   velox_file_test
   velox_file
   velox_exception
-  velox_exec_test_util
+  velox_exec_test_lib
   ${FMT}
   ${FOLLY_WITH_DEPENDENCIES}
   gtest

diff --git a/velox/connectors/hive/CMakeLists.txt b/velox/connectors/hive/CMakeLists.txt
@@ -25,5 +25,8 @@ add_subdirectory(storage_adapters)
 
 if(${VELOX_BUILD_TESTING})
   add_subdirectory(tests)
+endif()
+
+if(${VELOX_ENABLE_BENCHMARKS})
   add_subdirectory(benchmarks)
 endif()
diff --git a/velox/connectors/hive/benchmarks/CMakeLists.txt b/velox/connectors/hive/benchmarks/CMakeLists.txt
@@ -25,7 +25,7 @@ target_link_libraries(
   velox_parse_utils
   velox_parse_expression
   velox_exec
-  velox_exec_test_util
+  velox_exec_test_lib
   velox_hive_connector
   velox_hive_partition_function
   velox_memory

diff --git a/velox/connectors/hive/benchmarks/HivePartitionFunctionBenchmark.cpp b/velox/connectors/hive/benchmarks/HivePartitionFunctionBenchmark.cpp
@@ -20,7 +20,7 @@
 #include "velox/type/Type.h"
 #include "velox/vector/ComplexVector.h"
 #include "velox/vector/fuzzer/VectorFuzzer.h"
-#include "velox/vector/tests/VectorMaker.h"
+#include "velox/vector/tests/utils/VectorMaker.h"
 
 DEFINE_int64(fuzzer_seed, 99887766, "Seed for random input dataset generator");
 

diff --git a/velox/connectors/hive/storage_adapters/hdfs/tests/CMakeLists.txt b/velox/connectors/hive/storage_adapters/hdfs/tests/CMakeLists.txt
@@ -19,7 +19,7 @@ target_link_libraries(
   velox_file
   velox_hdfs
   velox_core
-  velox_exec_test_util
+  velox_exec_test_lib
   velox_hive_connector
   velox_dwio_common_exception
   velox_exec

diff --git a/velox/connectors/hive/storage_adapters/s3fs/benchmark/CMakeLists.txt b/velox/connectors/hive/storage_adapters/s3fs/benchmark/CMakeLists.txt
@@ -20,6 +20,6 @@ target_link_libraries(
   velox_read_benchmark_lib
   velox_s3fs
   velox_exception
-  velox_exec_test_util
+  velox_exec_test_lib
   ${FMT}
   ${FOLLY_WITH_DEPENDENCIES})
diff --git a/velox/connectors/hive/storage_adapters/s3fs/tests/CMakeLists.txt b/velox/connectors/hive/storage_adapters/s3fs/tests/CMakeLists.txt
@@ -19,7 +19,7 @@ target_link_libraries(
   velox_file
   velox_s3fs
   velox_core
-  velox_exec_test_util
+  velox_exec_test_lib
   velox_hive_connector
   velox_dwio_common_exception
   velox_exec

diff --git a/velox/connectors/hive/tests/CMakeLists.txt b/velox/connectors/hive/tests/CMakeLists.txt
@@ -22,6 +22,6 @@ target_link_libraries(
   velox_dwio_common_exception
   velox_vector_test_lib
   velox_exec
-  velox_exec_test_util
+  velox_exec_test_lib
   gtest
   gtest_main)
diff --git a/velox/connectors/hive/tests/HivePartitionFunctionTest.cpp b/velox/connectors/hive/tests/HivePartitionFunctionTest.cpp
@@ -15,7 +15,7 @@
  */
 #include "velox/connectors/hive/HivePartitionFunction.h"
 #include "gtest/gtest.h"
-#include "velox/vector/tests/VectorMaker.h"
+#include "velox/vector/tests/utils/VectorMaker.h"
 
 using namespace facebook::velox;
 

diff --git a/velox/connectors/tpch/tests/CMakeLists.txt b/velox/connectors/tpch/tests/CMakeLists.txt
@@ -19,12 +19,12 @@ target_link_libraries(
   velox_tpch_connector_test
   velox_tpch_connector
   velox_vector_test_lib
-  velox_exec_test_util
+  velox_exec_test_lib
   velox_aggregates
   gtest
   gtest_main)
 
 add_executable(velox_tpch_speed_test SpeedTest.cpp)
 
 target_link_libraries(velox_tpch_speed_test velox_tpch_connector velox_exec
-                      velox_exec_test_util velox_memory ${FMT})
+                      velox_exec_test_lib velox_memory ${FMT})
diff --git a/velox/docs/develop/scalar-functions.rst b/velox/docs/develop/scalar-functions.rst
@@ -1255,7 +1255,7 @@ Testing
 -------
 
 Add a test using FunctionBaseTest from
-velox/functions/prestosql/tests/FunctionBaseTest.h as a base class. Name your test
+velox/functions/prestosql/tests/utils/FunctionBaseTest.h as a base class. Name your test
 and the .cpp file <function-name>Test, e.g. CardinalityTest in
 CardinalityTest.cpp or IsNullTest in IsNullTest.cpp.
 

diff --git a/velox/duckdb/conversion/tests/DuckWrapperTest.cpp b/velox/duckdb/conversion/tests/DuckWrapperTest.cpp
@@ -15,7 +15,7 @@
  */
 #include "velox/duckdb/conversion/DuckWrapper.h"
 #include "velox/external/duckdb/duckdb.hpp"
-#include "velox/vector/tests/VectorMaker.h"
+#include "velox/vector/tests/utils/VectorMaker.h"
 
 #include <gtest/gtest.h>
 

diff --git a/velox/duckdb/functions/tests/CMakeLists.txt b/velox/duckdb/functions/tests/CMakeLists.txt
@@ -23,7 +23,7 @@ target_link_libraries(
   velox_functions_prestosql
   velox_functions_lib
   velox_functions_test_lib
-  velox_exec_test_util
+  velox_exec_test_lib
   gtest
   gtest_main
   ${gflags_LIBRARIES})
diff --git a/velox/duckdb/functions/tests/DuckFunctionTest.cpp b/velox/duckdb/functions/tests/DuckFunctionTest.cpp
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 #include "velox/duckdb/functions/DuckFunctions.h"
-#include "velox/functions/prestosql/tests/FunctionBaseTest.h"
+#include "velox/functions/prestosql/tests/utils/FunctionBaseTest.h"
 
 using namespace facebook::velox;
 

diff --git a/velox/dwio/dwrf/test/ColumnWriterTests.cpp b/velox/dwio/dwrf/test/ColumnWriterTests.cpp
@@ -32,7 +32,7 @@
 #include "velox/dwio/dwrf/writer/Writer.h"
 #include "velox/type/Type.h"
 #include "velox/vector/DictionaryVector.h"
-#include "velox/vector/tests/VectorMaker.h"
+#include "velox/vector/tests/utils/VectorMaker.h"
 
 using namespace ::testing;
 using namespace facebook::velox::dwio::common;

diff --git a/velox/dwio/dwrf/test/E2EWriterTests.cpp b/velox/dwio/dwrf/test/E2EWriterTests.cpp
@@ -28,7 +28,7 @@
 #include "velox/dwio/type/fbhive/HiveTypeParser.h"
 #include "velox/vector/FlatVector.h"
 #include "velox/vector/fuzzer/VectorFuzzer.h"
-#include "velox/vector/tests/VectorMaker.h"
+#include "velox/vector/tests/utils/VectorMaker.h"
 
 using namespace ::testing;
 using namespace facebook::velox::dwio::common;

diff --git a/velox/dwio/parquet/tests/ParquetReaderTestBase.h b/velox/dwio/parquet/tests/ParquetReaderTestBase.h
@@ -17,7 +17,7 @@
 #pragma once
 
 #include "velox/dwio/common/tests/utils/DataFiles.h"
-#include "velox/vector/tests/VectorMaker.h"
+#include "velox/vector/tests/utils/VectorMaker.h"
 
 #include <gtest/gtest.h>
 

diff --git a/velox/dwio/parquet/tests/duckdb_reader/CMakeLists.txt b/velox/dwio/parquet/tests/duckdb_reader/CMakeLists.txt
@@ -32,7 +32,7 @@ add_test(
 target_link_libraries(
   velox_dwio_duckdb_parquet_table_scan_test
   velox_dwio_parquet_reader
-  velox_exec_test_util
+  velox_exec_test_lib
   velox_exec
   velox_hive_connector
   velox_aggregates
@@ -48,7 +48,7 @@ add_test(
 target_link_libraries(
   velox_dwio_duckdb_parquet_tpch_test
   velox_dwio_parquet_reader
-  velox_exec_test_util
+  velox_exec_test_lib
   velox_exec
   velox_hive_connector
   velox_aggregates

diff --git a/velox/dwio/parquet/tests/reader/CMakeLists.txt b/velox/dwio/parquet/tests/reader/CMakeLists.txt
@@ -43,7 +43,7 @@ add_test(
 target_link_libraries(
   velox_dwio_parquet_tpch_test
   velox_dwio_parquet_reader
-  velox_exec_test_util
+  velox_exec_test_lib
   velox_exec
   velox_hive_connector
   velox_aggregates

diff --git a/velox/examples/CMakeLists.txt b/velox/examples/CMakeLists.txt
@@ -35,7 +35,7 @@ target_link_libraries(
   velox_type
   velox_vector
   velox_exec
-  velox_exec_test_util
+  velox_exec_test_lib
   velox_hive_connector
   velox_memory)
 
@@ -46,6 +46,6 @@ target_link_libraries(
   velox_type
   velox_vector
   velox_exec
-  velox_exec_test_util
+  velox_exec_test_lib
   velox_hive_connector
   velox_memory)
diff --git a/velox/exec/CMakeLists.txt b/velox/exec/CMakeLists.txt
@@ -73,7 +73,10 @@ target_link_libraries(
 
 if(${VELOX_BUILD_TESTING})
   add_subdirectory(tests)
-  add_subdirectory(benchmarks)
 elseif(${VELOX_BUILD_TEST_UTILS})
   add_subdirectory(tests/utils)
 endif()
+
+if(${VELOX_ENABLE_BENCHMARKS})
+  add_subdirectory(benchmarks)
+endif()
diff --git a/velox/exec/benchmarks/VectorHasherBenchmark.cpp b/velox/exec/benchmarks/VectorHasherBenchmark.cpp
@@ -16,7 +16,7 @@
 #include <folly/Benchmark.h>
 #include <folly/init/Init.h>
 #include "velox/exec/VectorHasher.h"
-#include "velox/vector/tests/VectorMaker.h"
+#include "velox/vector/tests/utils/VectorMaker.h"
 
 using namespace facebook::velox;
 using namespace facebook::velox::exec;

diff --git a/velox/exec/tests/CMakeLists.txt b/velox/exec/tests/CMakeLists.txt
@@ -79,7 +79,7 @@ target_link_libraries(
   velox_test_util
   velox_type
   velox_serialization
-  velox_exec_test_util
+  velox_exec_test_lib
   velox_functions_json
   velox_vector
   velox_vector_fuzzer
@@ -112,6 +112,6 @@ target_link_libraries(
   velox_vector
   velox_vector_test_lib
   velox_exec
-  velox_exec_test_util
+  velox_exec_test_lib
   velox_tpch_connector
   velox_memory)
diff --git a/velox/exec/tests/FunctionResolutionTest.cpp b/velox/exec/tests/FunctionResolutionTest.cpp
@@ -16,7 +16,7 @@
 
 #include "velox/functions/Udf.h"
 #include "velox/functions/prestosql/registration/RegistrationFunctions.h"
-#include "velox/functions/prestosql/tests/FunctionBaseTest.h"
+#include "velox/functions/prestosql/tests/utils/FunctionBaseTest.h"
 
 namespace {
 using namespace facebook::velox;

diff --git a/velox/exec/tests/FunctionSignatureBuilderTest.cpp b/velox/exec/tests/FunctionSignatureBuilderTest.cpp
@@ -15,7 +15,7 @@
  */
 #include <gtest/gtest.h>
 #include "velox/expression/FunctionSignature.h"
-#include "velox/functions/prestosql/tests/FunctionBaseTest.h"
+#include "velox/functions/prestosql/tests/utils/FunctionBaseTest.h"
 
 using namespace facebook::velox;
 using namespace facebook::velox::exec;

diff --git a/velox/exec/tests/HashBitRangeTest.cpp b/velox/exec/tests/HashBitRangeTest.cpp
@@ -15,7 +15,7 @@
  */
 
 #include "velox/exec/HashBitRange.h"
-#include "velox/vector/tests/VectorTestBase.h"
+#include "velox/vector/tests/utils/VectorTestBase.h"
 
 using namespace facebook::velox;
 using namespace facebook::velox::exec;

diff --git a/velox/exec/tests/HashPartitionFunctionTest.cpp b/velox/exec/tests/HashPartitionFunctionTest.cpp
@@ -15,7 +15,7 @@
  */
 
 #include "velox/exec/HashPartitionFunction.h"
-#include "velox/vector/tests/VectorTestBase.h"
+#include "velox/vector/tests/utils/VectorTestBase.h"
 
 using namespace facebook::velox;
 using namespace facebook::velox::exec;

diff --git a/velox/exec/tests/HashTableTest.cpp b/velox/exec/tests/HashTableTest.cpp
@@ -17,7 +17,7 @@
 #include "velox/exec/HashTable.h"
 #include "velox/common/base/SelectivityInfo.h"
 #include "velox/exec/VectorHasher.h"
-#include "velox/vector/tests/VectorMaker.h"
+#include "velox/vector/tests/utils/VectorMaker.h"
 
 #include <gtest/gtest.h>
 #include <memory>

diff --git a/velox/exec/tests/OrderByTest.cpp b/velox/exec/tests/OrderByTest.cpp
@@ -22,7 +22,7 @@
 #include "velox/exec/tests/utils/PlanBuilder.h"
 #include "velox/exec/tests/utils/QueryAssertions.h"
 #include "velox/exec/tests/utils/TempDirectoryPath.h"
-#include "velox/vector/tests/VectorMaker.h"
+#include "velox/vector/tests/utils/VectorMaker.h"
 
 using namespace facebook::velox;
 using namespace facebook::velox::exec;

diff --git a/velox/exec/tests/PlanBuilderTest.cpp b/velox/exec/tests/PlanBuilderTest.cpp
@@ -18,7 +18,7 @@
 #include "velox/exec/WindowFunction.h"
 #include "velox/functions/prestosql/registration/RegistrationFunctions.h"
 #include "velox/parse/TypeResolver.h"
-#include "velox/vector/tests/VectorTestBase.h"
+#include "velox/vector/tests/utils/VectorTestBase.h"
 
 namespace facebook::velox::exec::test {
 

diff --git a/velox/exec/tests/PlanNodeToStringTest.cpp b/velox/exec/tests/PlanNodeToStringTest.cpp
@@ -19,7 +19,7 @@
 #include "velox/exec/tests/utils/PlanBuilder.h"
 #include "velox/functions/prestosql/registration/RegistrationFunctions.h"
 #include "velox/parse/TypeResolver.h"
-#include "velox/vector/tests/VectorTestBase.h"
+#include "velox/vector/tests/utils/VectorTestBase.h"
 
 #include <gtest/gtest.h>
 

diff --git a/velox/exec/tests/RoundRobinPartitionFunctionTest.cpp b/velox/exec/tests/RoundRobinPartitionFunctionTest.cpp
@@ -15,7 +15,7 @@
  */
 #include "velox/exec/RoundRobinPartitionFunction.h"
 #include <gtest/gtest.h>
-#include "velox/vector/tests/VectorMaker.h"
+#include "velox/vector/tests/utils/VectorMaker.h"
 
 using namespace facebook::velox;
 

diff --git a/velox/exec/tests/SpillTest.cpp b/velox/exec/tests/SpillTest.cpp
@@ -20,7 +20,7 @@
 #include "velox/common/file/FileSystems.h"
 #include "velox/exec/tests/utils/TempDirectoryPath.h"
 #include "velox/serializers/PrestoSerializer.h"
-#include "velox/vector/tests/VectorTestBase.h"
+#include "velox/vector/tests/utils/VectorTestBase.h"
 
 using namespace facebook::velox;
 using namespace facebook::velox::exec;

diff --git a/velox/exec/tests/UnorderedStreamReaderTest.cpp b/velox/exec/tests/UnorderedStreamReaderTest.cpp
@@ -16,7 +16,7 @@
 
 #include "velox/exec/UnorderedStreamReader.h"
 #include "velox/vector/fuzzer/VectorFuzzer.h"
-#include "velox/vector/tests/VectorTestBase.h"
+#include "velox/vector/tests/utils/VectorTestBase.h"
 
 using namespace facebook::velox;