You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ko...@apache.org on 2022/05/17 20:13:28 UTC
[arrow] branch master updated: ARROW-16588: [C++][FlightRPC] Don't subclass GTest in test helpers
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 a71e5f0d99 ARROW-16588: [C++][FlightRPC] Don't subclass GTest in test helpers
a71e5f0d99 is described below
commit a71e5f0d99992323639499dbeb8e5c1c6dd0fb0e
Author: David Li <li...@gmail.com>
AuthorDate: Wed May 18 05:10:52 2022 +0900
ARROW-16588: [C++][FlightRPC] Don't subclass GTest in test helpers
Also, don't link every Arrow library to UCX when enabled.
Closes #13169 from lidavidm/arrow-16588
Authored-by: David Li <li...@gmail.com>
Signed-off-by: Sutou Kouhei <ko...@clear-code.com>
---
cpp/CMakeLists.txt | 5 ----
cpp/src/arrow/flight/flight_test.cc | 28 +++++++++++++++-----
cpp/src/arrow/flight/test_definitions.cc | 28 ++++++++++----------
cpp/src/arrow/flight/test_definitions.h | 30 +++++++++++-----------
cpp/src/arrow/flight/transport/ucx/CMakeLists.txt | 2 ++
.../transport/ucx/flight_transport_ucx_test.cc | 28 +++++++++++++++-----
6 files changed, 73 insertions(+), 48 deletions(-)
diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt
index eb97f3ad9d..23e0ba311f 100644
--- a/cpp/CMakeLists.txt
+++ b/cpp/CMakeLists.txt
@@ -827,11 +827,6 @@ if(ARROW_USE_XSIMD)
list(APPEND ARROW_STATIC_LINK_LIBS xsimd)
endif()
-if(ARROW_WITH_UCX)
- list(APPEND ARROW_LINK_LIBS ucx::ucx)
- list(APPEND ARROW_STATIC_LINK_LIBS ucx::ucx)
-endif()
-
add_custom_target(arrow_dependencies)
add_custom_target(arrow_benchmark_dependencies)
add_custom_target(arrow_test_dependencies)
diff --git a/cpp/src/arrow/flight/flight_test.cc b/cpp/src/arrow/flight/flight_test.cc
index 5f86fc3292..a7c79a6dc5 100644
--- a/cpp/src/arrow/flight/flight_test.cc
+++ b/cpp/src/arrow/flight/flight_test.cc
@@ -73,45 +73,59 @@ const char kAuthHeader[] = "authorization";
//------------------------------------------------------------
// Common transport tests
-class GrpcConnectivityTest : public ConnectivityTest {
+class GrpcConnectivityTest : public ConnectivityTest, public ::testing::Test {
protected:
std::string transport() const override { return "grpc"; }
+ void SetUp() override { SetUpTest(); }
+ void TearDown() override { TearDownTest(); }
};
ARROW_FLIGHT_TEST_CONNECTIVITY(GrpcConnectivityTest);
-class GrpcDataTest : public DataTest {
+class GrpcDataTest : public DataTest, public ::testing::Test {
protected:
std::string transport() const override { return "grpc"; }
+ void SetUp() override { SetUpTest(); }
+ void TearDown() override { TearDownTest(); }
};
ARROW_FLIGHT_TEST_DATA(GrpcDataTest);
-class GrpcDoPutTest : public DoPutTest {
+class GrpcDoPutTest : public DoPutTest, public ::testing::Test {
protected:
std::string transport() const override { return "grpc"; }
+ void SetUp() override { SetUpTest(); }
+ void TearDown() override { TearDownTest(); }
};
ARROW_FLIGHT_TEST_DO_PUT(GrpcDoPutTest);
-class GrpcAppMetadataTest : public AppMetadataTest {
+class GrpcAppMetadataTest : public AppMetadataTest, public ::testing::Test {
protected:
std::string transport() const override { return "grpc"; }
+ void SetUp() override { SetUpTest(); }
+ void TearDown() override { TearDownTest(); }
};
ARROW_FLIGHT_TEST_APP_METADATA(GrpcAppMetadataTest);
-class GrpcIpcOptionsTest : public IpcOptionsTest {
+class GrpcIpcOptionsTest : public IpcOptionsTest, public ::testing::Test {
protected:
std::string transport() const override { return "grpc"; }
+ void SetUp() override { SetUpTest(); }
+ void TearDown() override { TearDownTest(); }
};
ARROW_FLIGHT_TEST_IPC_OPTIONS(GrpcIpcOptionsTest);
-class GrpcCudaDataTest : public CudaDataTest {
+class GrpcCudaDataTest : public CudaDataTest, public ::testing::Test {
protected:
std::string transport() const override { return "grpc"; }
+ void SetUp() override { SetUpTest(); }
+ void TearDown() override { TearDownTest(); }
};
ARROW_FLIGHT_TEST_CUDA_DATA(GrpcCudaDataTest);
-class GrpcErrorHandlingTest : public ErrorHandlingTest {
+class GrpcErrorHandlingTest : public ErrorHandlingTest, public ::testing::Test {
protected:
std::string transport() const override { return "grpc"; }
+ void SetUp() override { SetUpTest(); }
+ void TearDown() override { TearDownTest(); }
};
ARROW_FLIGHT_TEST_ERROR_HANDLING(GrpcErrorHandlingTest);
diff --git a/cpp/src/arrow/flight/test_definitions.cc b/cpp/src/arrow/flight/test_definitions.cc
index fbe23b789d..61873ba36d 100644
--- a/cpp/src/arrow/flight/test_definitions.cc
+++ b/cpp/src/arrow/flight/test_definitions.cc
@@ -114,7 +114,7 @@ void ConnectivityTest::TestBrokenConnection() {
//------------------------------------------------------------
// Tests of data plane methods
-void DataTest::SetUp() {
+void DataTest::SetUpTest() {
server_ = ExampleTestServer();
ASSERT_OK_AND_ASSIGN(auto location, Location::ForScheme(transport(), "127.0.0.1", 0));
@@ -123,7 +123,7 @@ void DataTest::SetUp() {
ASSERT_OK(ConnectClient());
}
-void DataTest::TearDown() {
+void DataTest::TearDownTest() {
ASSERT_OK(client_->Close());
ASSERT_OK(server_->Shutdown());
}
@@ -637,14 +637,14 @@ class DoPutTestServer : public FlightServerBase {
friend class DoPutTest;
};
-void DoPutTest::SetUp() {
+void DoPutTest::SetUpTest() {
ASSERT_OK_AND_ASSIGN(auto location, Location::ForScheme(transport(), "127.0.0.1", 0));
ASSERT_OK(MakeServer<DoPutTestServer>(
location, &server_, &client_,
[](FlightServerOptions* options) { return Status::OK(); },
[](FlightClientOptions* options) { return Status::OK(); }));
}
-void DoPutTest::TearDown() {
+void DoPutTest::TearDownTest() {
ASSERT_OK(client_->Close());
ASSERT_OK(server_->Shutdown());
reinterpret_cast<DoPutTestServer*>(server_.get())->batches_.clear();
@@ -865,14 +865,14 @@ Status AppMetadataTestServer::DoPut(const ServerCallContext& context,
return Status::OK();
}
-void AppMetadataTest::SetUp() {
+void AppMetadataTest::SetUpTest() {
ASSERT_OK_AND_ASSIGN(auto location, Location::ForScheme(transport(), "127.0.0.1", 0));
ASSERT_OK(MakeServer<AppMetadataTestServer>(
location, &server_, &client_,
[](FlightServerOptions* options) { return Status::OK(); },
[](FlightClientOptions* options) { return Status::OK(); }));
}
-void AppMetadataTest::TearDown() {
+void AppMetadataTest::TearDownTest() {
ASSERT_OK(client_->Close());
ASSERT_OK(server_->Shutdown());
}
@@ -1044,14 +1044,14 @@ class IpcOptionsTestServer : public FlightServerBase {
}
};
-void IpcOptionsTest::SetUp() {
+void IpcOptionsTest::SetUpTest() {
ASSERT_OK_AND_ASSIGN(auto location, Location::ForScheme(transport(), "127.0.0.1", 0));
ASSERT_OK(MakeServer<IpcOptionsTestServer>(
location, &server_, &client_,
[](FlightServerOptions* options) { return Status::OK(); },
[](FlightClientOptions* options) { return Status::OK(); }));
}
-void IpcOptionsTest::TearDown() {
+void IpcOptionsTest::TearDownTest() {
ASSERT_OK(client_->Close());
ASSERT_OK(server_->Shutdown());
}
@@ -1232,7 +1232,7 @@ class CudaDataTest::Impl {
std::shared_ptr<cuda::CudaContext> context;
};
-void CudaDataTest::SetUp() {
+void CudaDataTest::SetUpTest() {
ASSERT_OK_AND_ASSIGN(auto manager, cuda::CudaDeviceManager::Instance());
ASSERT_OK_AND_ASSIGN(auto device, manager->GetDevice(0));
ASSERT_OK_AND_ASSIGN(auto context, device->GetContext());
@@ -1251,7 +1251,7 @@ void CudaDataTest::SetUp() {
[](FlightClientOptions* options) { return Status::OK(); }, impl_->device,
impl_->context));
}
-void CudaDataTest::TearDown() {
+void CudaDataTest::TearDownTest() {
ASSERT_OK(client_->Close());
ASSERT_OK(server_->Shutdown());
}
@@ -1353,8 +1353,8 @@ void CudaDataTest::TestDoExchange() {
#else
-void CudaDataTest::SetUp() {}
-void CudaDataTest::TearDown() {}
+void CudaDataTest::SetUpTest() {}
+void CudaDataTest::TearDownTest() {}
void CudaDataTest::TestDoGet() { GTEST_SKIP() << "Arrow was built without ARROW_CUDA"; }
void CudaDataTest::TestDoPut() { GTEST_SKIP() << "Arrow was built without ARROW_CUDA"; }
void CudaDataTest::TestDoExchange() {
@@ -1440,14 +1440,14 @@ class ErrorHandlingTestServer : public FlightServerBase {
};
} // namespace
-void ErrorHandlingTest::SetUp() {
+void ErrorHandlingTest::SetUpTest() {
ASSERT_OK_AND_ASSIGN(auto location, Location::ForScheme(transport(), "127.0.0.1", 0));
ASSERT_OK(MakeServer<ErrorHandlingTestServer>(
location, &server_, &client_,
[](FlightServerOptions* options) { return Status::OK(); },
[](FlightClientOptions* options) { return Status::OK(); }));
}
-void ErrorHandlingTest::TearDown() {
+void ErrorHandlingTest::TearDownTest() {
ASSERT_OK(client_->Close());
ASSERT_OK(server_->Shutdown());
}
diff --git a/cpp/src/arrow/flight/test_definitions.h b/cpp/src/arrow/flight/test_definitions.h
index 464631455d..aa0d07857b 100644
--- a/cpp/src/arrow/flight/test_definitions.h
+++ b/cpp/src/arrow/flight/test_definitions.h
@@ -24,8 +24,6 @@
#pragma once
-#include <gtest/gtest.h>
-
#include <functional>
#include <memory>
#include <string>
@@ -39,9 +37,11 @@
namespace arrow {
namespace flight {
-class ARROW_FLIGHT_EXPORT FlightTest : public testing::Test {
+class ARROW_FLIGHT_EXPORT FlightTest {
protected:
virtual std::string transport() const = 0;
+ virtual void SetUpTest() {}
+ virtual void TearDownTest() {}
};
/// Common tests of startup/shutdown
@@ -67,8 +67,8 @@ class ARROW_FLIGHT_EXPORT ConnectivityTest : public FlightTest {
/// Common tests of data plane methods
class ARROW_FLIGHT_EXPORT DataTest : public FlightTest {
public:
- void SetUp();
- void TearDown();
+ void SetUpTest() override;
+ void TearDownTest() override;
Status ConnectClient();
// Test methods
@@ -124,8 +124,8 @@ class ARROW_FLIGHT_EXPORT DataTest : public FlightTest {
/// \brief Specific tests of DoPut.
class ARROW_FLIGHT_EXPORT DoPutTest : public FlightTest {
public:
- void SetUp();
- void TearDown();
+ void SetUpTest() override;
+ void TearDownTest() override;
void CheckBatches(const FlightDescriptor& expected_descriptor,
const RecordBatchVector& expected_batches);
void CheckDoPut(const FlightDescriptor& descr, const std::shared_ptr<Schema>& schema,
@@ -171,8 +171,8 @@ class ARROW_FLIGHT_EXPORT AppMetadataTestServer : public FlightServerBase {
/// \brief Tests of app_metadata in data plane methods.
class ARROW_FLIGHT_EXPORT AppMetadataTest : public FlightTest {
public:
- void SetUp();
- void TearDown();
+ void SetUpTest() override;
+ void TearDownTest() override;
// Test methods
void TestDoGet();
@@ -198,8 +198,8 @@ class ARROW_FLIGHT_EXPORT AppMetadataTest : public FlightTest {
/// \brief Tests of IPC options in data plane methods.
class ARROW_FLIGHT_EXPORT IpcOptionsTest : public FlightTest {
public:
- void SetUp();
- void TearDown();
+ void SetUpTest() override;
+ void TearDownTest() override;
// Test methods
void TestDoGetReadOptions();
@@ -233,8 +233,8 @@ class ARROW_FLIGHT_EXPORT IpcOptionsTest : public FlightTest {
/// If not built with ARROW_CUDA, tests are no-ops.
class ARROW_FLIGHT_EXPORT CudaDataTest : public FlightTest {
public:
- void SetUp() override;
- void TearDown() override;
+ void SetUpTest() override;
+ void TearDownTest() override;
// Test methods
void TestDoGet();
@@ -258,8 +258,8 @@ class ARROW_FLIGHT_EXPORT CudaDataTest : public FlightTest {
/// \brief Tests of error handling.
class ARROW_FLIGHT_EXPORT ErrorHandlingTest : public FlightTest {
public:
- void SetUp() override;
- void TearDown() override;
+ void SetUpTest() override;
+ void TearDownTest() override;
// Test methods
void TestGetFlightInfo();
diff --git a/cpp/src/arrow/flight/transport/ucx/CMakeLists.txt b/cpp/src/arrow/flight/transport/ucx/CMakeLists.txt
index 71392ec0af..d682ead633 100644
--- a/cpp/src/arrow/flight/transport/ucx/CMakeLists.txt
+++ b/cpp/src/arrow/flight/transport/ucx/CMakeLists.txt
@@ -53,6 +53,7 @@ if(ARROW_BUILD_TESTS)
arrow_flight_static
arrow_flight_testing_static
arrow_flight_transport_ucx_static
+ ucx::ucx
${ARROW_TEST_LINK_LIBS})
else()
set(ARROW_FLIGHT_UCX_TEST_LINK_LIBS
@@ -60,6 +61,7 @@ if(ARROW_BUILD_TESTS)
arrow_flight_shared
arrow_flight_testing_shared
arrow_flight_transport_ucx_shared
+ ucx::ucx
${ARROW_TEST_LINK_LIBS})
endif()
add_arrow_test(flight_transport_ucx_test
diff --git a/cpp/src/arrow/flight/transport/ucx/flight_transport_ucx_test.cc b/cpp/src/arrow/flight/transport/ucx/flight_transport_ucx_test.cc
index a29d498d0b..1e2599ff11 100644
--- a/cpp/src/arrow/flight/transport/ucx/flight_transport_ucx_test.cc
+++ b/cpp/src/arrow/flight/transport/ucx/flight_transport_ucx_test.cc
@@ -50,45 +50,59 @@ testing::Environment* const kUcxEnvironment =
//------------------------------------------------------------
// Common transport tests
-class UcxConnectivityTest : public ConnectivityTest {
+class UcxConnectivityTest : public ConnectivityTest, public ::testing::Test {
protected:
std::string transport() const override { return "ucx"; }
+ void SetUp() override { SetUpTest(); }
+ void TearDown() override { TearDownTest(); }
};
ARROW_FLIGHT_TEST_CONNECTIVITY(UcxConnectivityTest);
-class UcxDataTest : public DataTest {
+class UcxDataTest : public DataTest, public ::testing::Test {
protected:
std::string transport() const override { return "ucx"; }
+ void SetUp() override { SetUpTest(); }
+ void TearDown() override { TearDownTest(); }
};
ARROW_FLIGHT_TEST_DATA(UcxDataTest);
-class UcxDoPutTest : public DoPutTest {
+class UcxDoPutTest : public DoPutTest, public ::testing::Test {
protected:
std::string transport() const override { return "ucx"; }
+ void SetUp() override { SetUpTest(); }
+ void TearDown() override { TearDownTest(); }
};
ARROW_FLIGHT_TEST_DO_PUT(UcxDoPutTest);
-class UcxAppMetadataTest : public AppMetadataTest {
+class UcxAppMetadataTest : public AppMetadataTest, public ::testing::Test {
protected:
std::string transport() const override { return "ucx"; }
+ void SetUp() override { SetUpTest(); }
+ void TearDown() override { TearDownTest(); }
};
ARROW_FLIGHT_TEST_APP_METADATA(UcxAppMetadataTest);
-class UcxIpcOptionsTest : public IpcOptionsTest {
+class UcxIpcOptionsTest : public IpcOptionsTest, public ::testing::Test {
protected:
std::string transport() const override { return "ucx"; }
+ void SetUp() override { SetUpTest(); }
+ void TearDown() override { TearDownTest(); }
};
ARROW_FLIGHT_TEST_IPC_OPTIONS(UcxIpcOptionsTest);
-class UcxCudaDataTest : public CudaDataTest {
+class UcxCudaDataTest : public CudaDataTest, public ::testing::Test {
protected:
std::string transport() const override { return "ucx"; }
+ void SetUp() override { SetUpTest(); }
+ void TearDown() override { TearDownTest(); }
};
ARROW_FLIGHT_TEST_CUDA_DATA(UcxCudaDataTest);
-class UcxErrorHandlingTest : public ErrorHandlingTest {
+class UcxErrorHandlingTest : public ErrorHandlingTest, public ::testing::Test {
protected:
std::string transport() const override { return "ucx"; }
+ void SetUp() override { SetUpTest(); }
+ void TearDown() override { TearDownTest(); }
};
ARROW_FLIGHT_TEST_ERROR_HANDLING(UcxErrorHandlingTest);