[GCS] Change GCS Test to cc_test (#6596)

.travis.yml

@@ -143,7 +143,6 @@ script:
   - export PATH="$HOME/miniconda/bin:$PATH"
 
   # raylet integration tests
-  - ./ci/suppress_output bash src/ray/test/run_gcs_tests.sh
   - ./ci/suppress_output bash src/ray/test/run_core_worker_tests.sh
   - ./ci/suppress_output bash src/ray/test/run_object_manager_tests.sh
 

BUILD.bazel

@@ -810,56 +810,81 @@ cc_library(
     ],
 )
 
-# TODO(micafan) Replace cc_binary with cc_test for GCS test.
-cc_binary(
+# TODO(micafan) Support test group in future. Use test group we can run all gcs test once.
+cc_test(
     name = "redis_gcs_client_test",
-    testonly = 1,
     srcs = ["src/ray/gcs/test/redis_gcs_client_test.cc"],
+    args = ["$(location redis-server) $(location redis-cli) $(location libray_redis_module.so)"],
     copts = COPTS,
+    data = [
+        "//:libray_redis_module.so",
+        "//:redis-cli",
+        "//:redis-server",
+    ],
     deps = [
         ":gcs",
         "@com_google_googletest//:gtest_main",
     ],
 )
 
-cc_binary(
+cc_test(
     name = "redis_actor_info_accessor_test",
-    testonly = 1,
     srcs = ["src/ray/gcs/test/redis_actor_info_accessor_test.cc"],
+    args = ["$(location redis-server) $(location redis-cli) $(location libray_redis_module.so)"],
     copts = COPTS,
+    data = [
+        "//:libray_redis_module.so",
+        "//:redis-cli",
+        "//:redis-server",
+    ],
     deps = [
         ":gcs",
         "@com_google_googletest//:gtest_main",
     ],
 )
 
-cc_binary(
+cc_test(
     name = "subscription_executor_test",
-    testonly = 1,
     srcs = ["src/ray/gcs/test/subscription_executor_test.cc"],
+    args = ["$(location redis-server) $(location redis-cli) $(location libray_redis_module.so)"],
     copts = COPTS,
+    data = [
+        "//:libray_redis_module.so",
+        "//:redis-cli",
+        "//:redis-server",
+    ],
     deps = [
         ":gcs",
         "@com_google_googletest//:gtest_main",
     ],
 )
 
-cc_binary(
+cc_test(
     name = "redis_job_info_accessor_test",
-    testonly = 1,
     srcs = ["src/ray/gcs/test/redis_job_info_accessor_test.cc"],
+    args = ["$(location redis-server) $(location redis-cli) $(location libray_redis_module.so)"],
     copts = COPTS,
+    data = [
+        "//:libray_redis_module.so",
+        "//:redis-cli",
+        "//:redis-server",
+    ],
     deps = [
         ":gcs",
         "@com_google_googletest//:gtest_main",
     ],
 )
 
-cc_binary(
+cc_test(
     name = "asio_test",
-    testonly = 1,
     srcs = ["src/ray/gcs/test/asio_test.cc"],
+    args = ["$(location redis-server) $(location redis-cli) $(location libray_redis_module.so)"],
     copts = COPTS,
+    data = [
+        "//:libray_redis_module.so",
+        "//:redis-cli",
+        "//:redis-server",
+    ],
     deps = [
         ":gcs",
         ":ray_util",

doc/source/development.rst

@@ -190,7 +190,6 @@ invoke other test scripts via ``pytest``, ``bazel``-based test or other bash
 scripts. Some of the examples include:
 
 * Raylet integration tests commands:
-    * ``src/ray/test/run_gcs_tests.sh``
     * ``src/ray/test/run_core_worker_tests.sh``
     * ``src/ray/test/run_object_manager_tests.sh``
 

src/ray/gcs/gcs_server/test/gcs_server_rpc_test.cc

@@ -11,25 +11,10 @@ static std::string redis_server_executable;
 static std::string redis_client_executable;
 static std::string libray_redis_module_path;
 
-class GcsServerTest : public ::testing::Test {
+class GcsServerTest : public RedisServiceManagerForTest {
  public:
   using CallFunction = std::function<void(std::promise<bool> &promise)>;
 
-  static void SetUpTestCase() {
-    std::string start_redis_command = redis_server_executable +
-                                      " --loglevel warning --loadmodule " +
-                                      libray_redis_module_path + " --port 6379 &";
-    RAY_LOG(INFO) << "Start redis command is: " << start_redis_command;
-    RAY_CHECK(system(start_redis_command.c_str()) == 0);
-    usleep(200 * 1000);
-  }
-
-  static void TearDownTestCase() {
-    std::string stop_redis_command = redis_client_executable + " -p 6379 shutdown";
-    RAY_LOG(INFO) << "Stop redis command is: " << stop_redis_command;
-    RAY_CHECK(system(stop_redis_command.c_str()) == 0);
-  }
-
   void SetUp() override {
     gcs::GcsServerConfig config;
     config.grpc_server_port = 0;
@@ -37,6 +22,7 @@ class GcsServerTest : public ::testing::Test {
     config.grpc_server_thread_num = 1;
     config.redis_address = "127.0.0.1";
     config.is_test = true;
+    config.redis_port = REDIS_SERVER_PORT;
     gcs_server_.reset(new gcs::GcsServer(config));
 
     thread_io_service_.reset(new std::thread([this] {
@@ -214,8 +200,8 @@ TEST_F(GcsServerTest, TestJobInfo) {
 int main(int argc, char **argv) {
   ::testing::InitGoogleTest(&argc, argv);
   RAY_CHECK(argc == 4);
-  ray::redis_server_executable = argv[1];
-  ray::redis_client_executable = argv[2];
-  ray::libray_redis_module_path = argv[3];
+  ray::REDIS_SERVER_EXEC_PATH = argv[1];
+  ray::REDIS_CLIENT_EXEC_PATH = argv[2];
+  ray::REDIS_MODULE_LIBRARY_PATH = argv[3];
   return RUN_ALL_TESTS();
 }

src/ray/gcs/test/accessor_test_base.h

@@ -16,16 +16,17 @@ namespace ray {
 namespace gcs {
 
 template <typename ID, typename Data>
-class AccessorTestBase : public ::testing::Test {
+class AccessorTestBase : public RedisServiceManagerForTest {
  public:
-  AccessorTestBase() : options_("127.0.0.1", 6379, "", true) {}
+  AccessorTestBase() {}
 
   virtual ~AccessorTestBase() {}
 
   virtual void SetUp() {
     GenTestData();
 
-    gcs_client_.reset(new RedisGcsClient(options_));
+    GcsClientOptions options = GcsClientOptions("127.0.0.1", REDIS_SERVER_PORT, "", true);
+    gcs_client_.reset(new RedisGcsClient(options));
     RAY_CHECK_OK(gcs_client_->Connect(io_service_));
 
     work_thread_.reset(new std::thread([this] {
@@ -63,7 +64,6 @@ class AccessorTestBase : public ::testing::Test {
   }
 
  protected:
-  GcsClientOptions options_;
   std::unique_ptr<RedisGcsClient> gcs_client_;
 
   boost::asio::io_service io_service_;

src/ray/gcs/test/asio_test.cc

@@ -3,12 +3,17 @@
 #include "gtest/gtest.h"
 #include "ray/gcs/asio.h"
 #include "ray/util/logging.h"
+#include "ray/util/test_util.h"
 
 extern "C" {
 #include "hiredis/async.h"
 #include "hiredis/hiredis.h"
 }
 
+namespace ray {
+
+namespace gcs {
+
 boost::asio::io_service io_service;
 
 void ConnectCallback(const redisAsyncContext *c, int status) {
@@ -26,8 +31,10 @@ void GetCallback(redisAsyncContext *c, void *r, void *privdata) {
   io_service.stop();
 }
 
-TEST(RedisAsioTest, TestRedisCommands) {
-  redisAsyncContext *ac = redisAsyncConnect("127.0.0.1", 6379);
+class RedisAsioTest : public RedisServiceManagerForTest {};
+
+TEST_F(RedisAsioTest, TestRedisCommands) {
+  redisAsyncContext *ac = redisAsyncConnect("127.0.0.1", REDIS_SERVER_PORT);
   ASSERT_TRUE(ac->err == 0);
   ray::gcs::RedisAsyncContext redis_async_context(ac);
 
@@ -41,3 +48,16 @@ TEST(RedisAsioTest, TestRedisCommands) {
 
   io_service.run();
 }
+
+}  // namespace gcs
+
+}  // namespace ray
+
+int main(int argc, char **argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  RAY_CHECK(argc == 4);
+  ray::REDIS_SERVER_EXEC_PATH = argv[1];
+  ray::REDIS_CLIENT_EXEC_PATH = argv[2];
+  ray::REDIS_MODULE_LIBRARY_PATH = argv[3];
+  return RUN_ALL_TESTS();
+}

src/ray/gcs/test/redis_actor_info_accessor_test.cc

@@ -102,3 +102,12 @@ TEST_F(ActorInfoAccessorTest, Subscribe) {
 }  // namespace gcs
 
 }  // namespace ray
+
+int main(int argc, char **argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  RAY_CHECK(argc == 4);
+  ray::REDIS_SERVER_EXEC_PATH = argv[1];
+  ray::REDIS_CLIENT_EXEC_PATH = argv[2];
+  ray::REDIS_MODULE_LIBRARY_PATH = argv[3];
+  return RUN_ALL_TESTS();
+}

src/ray/gcs/test/redis_gcs_client_test.cc

@@ -17,7 +17,7 @@ namespace gcs {
 
 /* Flush redis. */
 static inline void flushall_redis(void) {
-  redisContext *context = redisConnect("127.0.0.1", 6379);
+  redisContext *context = redisConnect("127.0.0.1", REDIS_SERVER_PORT);
   freeReplyObject(redisCommand(context, "FLUSHALL"));
   redisFree(context);
 }
@@ -28,11 +28,9 @@ inline JobID NextJobID() {
   return JobID::FromInt(++counter);
 }
 
-class TestGcs : public ::testing::Test {
+class TestGcs : public RedisServiceManagerForTest {
  public:
   TestGcs(CommandType command_type) : num_callbacks_(0), command_type_(command_type) {
-    GcsClientOptions options("127.0.0.1", 6379, "", true);
-    client_ = std::make_shared<gcs::RedisGcsClient>(options, command_type_);
     job_id_ = NextJobID();
   }
 
@@ -63,7 +61,6 @@ class TestGcsWithAsio : public TestGcs {
  public:
   TestGcsWithAsio(CommandType command_type)
       : TestGcs(command_type), io_service_(), work_(io_service_) {
-    RAY_CHECK_OK(client_->Connect(io_service_));
   }
 
   TestGcsWithAsio() : TestGcsWithAsio(CommandType::kRegular) {}
@@ -73,6 +70,13 @@ class TestGcsWithAsio : public TestGcs {
     client_->Disconnect();
     client_.reset();
   }
+
+  void SetUp() override {
+    GcsClientOptions options("127.0.0.1", REDIS_SERVER_PORT, "", true);
+    client_ = std::make_shared<gcs::RedisGcsClient>(options, command_type_);
+    RAY_CHECK_OK(client_->Connect(io_service_));
+  }
+
   void Start() override { io_service_.run(); }
   void Stop() override { io_service_.stop(); }
 
@@ -1442,3 +1446,12 @@ TEST_F(TestGcsWithAsio, TestHashTable) {
 
 }  // namespace gcs
 }  // namespace ray
+
+int main(int argc, char **argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  RAY_CHECK(argc == 4);
+  ray::REDIS_SERVER_EXEC_PATH = argv[1];
+  ray::REDIS_CLIENT_EXEC_PATH = argv[2];
+  ray::REDIS_MODULE_LIBRARY_PATH = argv[3];
+  return RUN_ALL_TESTS();
+}

src/ray/gcs/test/redis_job_info_accessor_test.cc

@@ -72,3 +72,12 @@ TEST_F(RedisJobInfoAccessorTest, AddAndSubscribe) {
 }  // namespace gcs
 
 }  // namespace ray
+
+int main(int argc, char **argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  RAY_CHECK(argc == 4);
+  ray::REDIS_SERVER_EXEC_PATH = argv[1];
+  ray::REDIS_CLIENT_EXEC_PATH = argv[2];
+  ray::REDIS_MODULE_LIBRARY_PATH = argv[3];
+  return RUN_ALL_TESTS();
+}

src/ray/gcs/test/subscription_executor_test.cc

@@ -196,3 +196,12 @@ TEST_F(SubscriptionExecutorTest, UnsubscribeTest) {
 }  // namespace gcs
 
 }  // namespace ray
+
+int main(int argc, char **argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  RAY_CHECK(argc == 4);
+  ray::REDIS_SERVER_EXEC_PATH = argv[1];
+  ray::REDIS_CLIENT_EXEC_PATH = argv[2];
+  ray::REDIS_MODULE_LIBRARY_PATH = argv[3];
+  return RUN_ALL_TESTS();
+}

src/ray/test/run_gcs_tests.sh

@@ -1,26 +0,0 @@
-#!/usr/bin/env bash
-
-# This needs to be run in the root directory
-
-# Cause the script to exit if a single command fails.
-set -e
-set -x
-
-bazel build "//:redis_gcs_client_test" "//:subscription_executor_test" "//:asio_test" "//:libray_redis_module.so"
-bazel build "//:redis_actor_info_accessor_test" "//:redis_job_info_accessor_test"
-
-# Start Redis.
-./bazel-bin/redis-server \
-    --loglevel warning \
-    --loadmodule ./bazel-bin/libray_redis_module.so \
-    --port 6379 &
-sleep 1s
-
-./bazel-bin/redis_gcs_client_test
-./bazel-bin/subscription_executor_test
-./bazel-bin/asio_test
-./bazel-bin/redis_actor_info_accessor_test
-./bazel-bin/redis_job_info_accessor_test
-
-./bazel-bin/redis-cli -p 6379 shutdown
-sleep 1s

src/ray/util/test_util.h

@@ -5,7 +5,9 @@
 
 #include <string>
 
+#include "gtest/gtest.h"
 #include "ray/common/buffer.h"
+#include "ray/common/id.h"
 #include "ray/common/ray_object.h"
 #include "ray/util/util.h"
 
@@ -58,6 +60,43 @@ std::shared_ptr<RayObject> GenerateRandomObject() {
   return std::shared_ptr<RayObject>(new RayObject(GenerateRandomBuffer(), nullptr));
 }
 
+/// Path to redis server executable binary.
+std::string REDIS_SERVER_EXEC_PATH;
+/// Path to redis client executable binary.
+std::string REDIS_CLIENT_EXEC_PATH;
+/// Path to redis module library.
+std::string REDIS_MODULE_LIBRARY_PATH;
+/// Port of redis server.
+int REDIS_SERVER_PORT;
+
+/// Test helper class, it will start redis server before the test runs,
+/// and stop redis server after the test is completed.
+class RedisServiceManagerForTest : public ::testing::Test {
+ public:
+  static void SetUpTestCase() {
+    auto seed = std::chrono::high_resolution_clock::now().time_since_epoch().count();
+    std::mt19937 gen(seed);
+    std::uniform_int_distribution<int> random_gen{2000, 7000};
+    // Use random port to avoid port conflicts between UTs.
+    REDIS_SERVER_PORT = random_gen(gen);
+
+    std::string start_redis_command =
+        REDIS_SERVER_EXEC_PATH + " --loglevel warning --loadmodule " +
+        REDIS_MODULE_LIBRARY_PATH + " --port " + std::to_string(REDIS_SERVER_PORT) + " &";
+    RAY_LOG(INFO) << "Start redis command is: " << start_redis_command;
+    RAY_CHECK(system(start_redis_command.c_str()) == 0);
+    usleep(200 * 1000);
+  }
+
+  static void TearDownTestCase() {
+    std::string stop_redis_command =
+        REDIS_CLIENT_EXEC_PATH + " -p " + std::to_string(REDIS_SERVER_PORT) + " shutdown";
+    RAY_LOG(INFO) << "Stop redis command is: " << stop_redis_command;
+    RAY_CHECK(system(stop_redis_command.c_str()) == 0);
+    usleep(100 * 1000);
+  }
+};
+
 }  // namespace ray
 
 #endif  // RAY_UTIL_TEST_UTIL_H