HDFS-16265. Refactor HDFS tool tests for better reuse (#3536)

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/tools/CMakeLists.txt

@@ -16,7 +16,20 @@
 # limitations under the License.
 #
 
-add_executable(hdfs_tool_test hdfs-allow-snapshot-mock.cc hdfs-cat-mock.cc hdfs-tool-test.cc main.cc)
+add_executable(hdfs_tool_tests
-target_include_directories(hdfs_tool_test PRIVATE ../tools ../../tools ../../tools/hdfs-allow-snapshot ../../tools/hdfs-cat)
+                hdfs-allow-snapshot-mock.cc
-target_link_libraries(hdfs_tool_test PRIVATE gmock_main hdfs_allowSnapshot_lib hdfs_cat_lib)
+                hdfs-cat-mock.cc
-add_test(hdfs_tool_test hdfs_tool_test)
+                hdfs-tool-test-fixtures.cc
+                hdfs-tool-tests.cc
+                main.cc)
+target_include_directories(hdfs_tool_tests PRIVATE
+                            ../tools
+                            ../../tools
+                            ../../tools/hdfs-allow-snapshot
+                            ../../tools/hdfs-delete-snapshot
+                            ../../tools/hdfs-cat)
+target_link_libraries(hdfs_tool_tests PRIVATE
+                       gmock_main
+                       hdfs_allowSnapshot_lib
+                       hdfs_cat_lib)
+add_test(hdfs_tool_tests hdfs_tool_tests)

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/tools/hdfs-allow-snapshot-mock.cc

@@ -17,8 +17,40 @@
   under the License.
 */
 
+#include <functional>
+#include <memory>
+#include <string>
+#include <vector>
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
 #include "hdfs-allow-snapshot-mock.h"
+#include "hdfs-tool-tests.h"
 
 namespace hdfs::tools::test {
-AllowSnapshotMock::~AllowSnapshotMock() {}
+AllowSnapshotMock::~AllowSnapshotMock() = default;
-} // namespace hdfs::tools::test
+
+void AllowSnapshotMock::SetExpectations(
+    std::function<std::unique_ptr<AllowSnapshotMock>()> test_case,
+    const std::vector<std::string> &args) const {
+  // Get the pointer to the function that defines the test case
+  const auto test_case_func =
+      test_case.target<std::unique_ptr<AllowSnapshotMock> (*)()>();
+  ASSERT_NE(test_case_func, nullptr);
+
+  // Set the expected method calls and their corresponding arguments for each
+  // test case
+  if (*test_case_func == &CallHelp<AllowSnapshotMock>) {
+    EXPECT_CALL(*this, HandleHelp()).Times(1).WillOnce(testing::Return(true));
+    return;
+  }
+
+  if (*test_case_func == &PassAPath<AllowSnapshotMock>) {
+    const auto arg1 = args[0];
+    EXPECT_CALL(*this, HandlePath(arg1))
+        .Times(1)
+        .WillOnce(testing::Return(true));
+  }
+}
+} // namespace hdfs::tools::test

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/tools/hdfs-allow-snapshot-mock.h

@@ -19,7 +19,10 @@
 #ifndef LIBHDFSPP_TOOLS_HDFS_ALLOW_SNAPSHOT_MOCK
 #define LIBHDFSPP_TOOLS_HDFS_ALLOW_SNAPSHOT_MOCK
 
+#include <functional>
+#include <memory>
 #include <string>
+#include <vector>
 
 #include <gmock/gmock.h>
 
@@ -44,6 +47,18 @@ public:
   AllowSnapshotMock &operator=(AllowSnapshotMock &&) = delete;
   ~AllowSnapshotMock() override;
 
+  /**
+   * Defines the methods and the corresponding arguments that are expected
+   * to be called on this instance of {@link HdfsTool} for the given test case.
+   *
+   * @param test_case An {@link std::function} object that points to the
+   * function defining the test case
+   * @param args The arguments that are passed to this test case
+   */
+  void
+  SetExpectations(std::function<std::unique_ptr<AllowSnapshotMock>()> test_case,
+                  const std::vector<std::string> &args = {}) const;
+
   MOCK_METHOD(bool, HandleHelp, (), (const, override));
 
   MOCK_METHOD(bool, HandlePath, (const std::string &), (const, override));

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/tools/hdfs-cat-mock.cc

@@ -16,8 +16,40 @@
  * limitations under the License.
  */
 
+#include <functional>
+#include <memory>
+#include <string>
+#include <vector>
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
 #include "hdfs-cat-mock.h"
+#include "hdfs-tool-tests.h"
 
 namespace hdfs::tools::test {
-CatMock::~CatMock() {}
+CatMock::~CatMock() = default;
+
+void CatMock::SetExpectations(
+    std::function<std::unique_ptr<CatMock>()> test_case,
+    const std::vector<std::string> &args) const {
+  // Get the pointer to the function that defines the test case
+  const auto test_case_func =
+      test_case.target<std::unique_ptr<CatMock> (*)()>();
+  ASSERT_NE(test_case_func, nullptr);
+
+  // Set the expected method calls and their corresponding arguments for each
+  // test case
+  if (*test_case_func == &CallHelp<CatMock>) {
+    EXPECT_CALL(*this, HandleHelp()).Times(1).WillOnce(testing::Return(true));
+    return;
+  }
+
+  if (*test_case_func == &PassAPath<CatMock>) {
+    const auto arg1 = args[0];
+    EXPECT_CALL(*this, HandlePath(arg1))
+        .Times(1)
+        .WillOnce(testing::Return(true));
+  }
+}
 } // namespace hdfs::tools::test

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/tools/hdfs-cat-mock.h

@@ -19,7 +19,10 @@
 #ifndef LIBHDFSPP_TOOLS_HDFS_CAT_MOCK
 #define LIBHDFSPP_TOOLS_HDFS_CAT_MOCK
 
+#include <functional>
+#include <memory>
 #include <string>
+#include <vector>
 
 #include <gmock/gmock.h>
 
@@ -44,6 +47,17 @@ public:
   CatMock &operator=(CatMock &&) = delete;
   ~CatMock() override;
 
+  /**
+   * Defines the methods and the corresponding arguments that are expected
+   * to be called on this instance of {@link HdfsTool} for the given test case.
+   *
+   * @param test_case An {@link std::function} object that points to the
+   * function defining the test case
+   * @param args The arguments that are passed to this test case
+   */
+  void SetExpectations(std::function<std::unique_ptr<CatMock>()> test_case,
+                       const std::vector<std::string> &args = {}) const;
+
   MOCK_METHOD(bool, HandleHelp, (), (const, override));
 
   MOCK_METHOD(bool, HandlePath, (const std::string &), (const, override));

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/tools/hdfs-tool-test-fixtures.cc

@@ -0,0 +1,41 @@
+/*
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, 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.
+*/
+
+#include <tuple>
+
+#include <gtest/gtest.h>
+
+#include "hdfs-tool-test-fixtures.h"
+
+/**
+ * Implementing virtual destructors out-of-line to avoid bulky v-table entries.
+ */
+HdfsToolBasicTest::~HdfsToolBasicTest() = default;
+HdfsToolNegativeTestThrows::~HdfsToolNegativeTestThrows() = default;
+HdfsToolNegativeTestNoThrow::~HdfsToolNegativeTestNoThrow() = default;
+
+TEST_P(HdfsToolBasicTest, RunTool) { EXPECT_TRUE(this->hdfs_tool_->Do()); }
+
+TEST_P(HdfsToolNegativeTestNoThrow, RunTool) {
+  EXPECT_FALSE(this->hdfs_tool_->Do());
+}
+
+TEST_P(HdfsToolNegativeTestThrows, RunTool) {
+  EXPECT_ANY_THROW({ std::ignore = this->hdfs_tool_->Do(); });
+}

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/tools/hdfs-tool-test-fixtures.h

@@ -0,0 +1,100 @@
+/*
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, 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.
+*/
+
+#ifndef LIBHDFSPP_TOOLS_HDFS_TOOL_TEST_FIXTURES
+#define LIBHDFSPP_TOOLS_HDFS_TOOL_TEST_FIXTURES
+
+#include <functional>
+#include <memory>
+
+#include <gtest/gtest.h>
+
+#include "hdfs-tool.h"
+
+/**
+ * {@class HdfsToolBasicTest} is a fixture that houses basic tests on {@link
+ * hdfs::tools::HdfsTool} interface. It contains the "Happy path" tests which
+ * covers the scenarios where {@link hdfs::tools::HdfsTool} is expected to
+ * work just fine.
+ *
+ * {@class HdfsToolBasicTest} is parameterized on a lambda returning an instance
+ * of {@link hdfs::tools::HdfsTool} wrapped in a std::unique_ptr. We then run
+ * the tests on this instance. Each test runs in isolation. So, a new instance
+ * is created for each test.
+ */
+class HdfsToolBasicTest
+    : public testing::TestWithParam<
+          std::function<std::unique_ptr<hdfs::tools::HdfsTool>()>> {
+public:
+  // Abiding to the rule of 5
+  HdfsToolBasicTest() = default;
+  HdfsToolBasicTest(const HdfsToolBasicTest &) = delete;
+  HdfsToolBasicTest(HdfsToolBasicTest &&) = delete;
+  HdfsToolBasicTest &operator=(const HdfsToolBasicTest &) = delete;
+  HdfsToolBasicTest &operator=(HdfsToolBasicTest &&) = delete;
+  ~HdfsToolBasicTest() override;
+
+protected:
+  void SetUp() override { hdfs_tool_ = GetParam()(); }
+
+  std::unique_ptr<hdfs::tools::HdfsTool> hdfs_tool_{nullptr};
+};
+
+/**
+ * {@class HdfsToolNegativeTestThrows} is a fixture that houses negative tests
+ * on {@link hdfs::tools::HdfsTool} interface. It covers the tests where
+ * unfavorable inputs are presented to the {@link hdfs::tools::HdfsTool}
+ * instance and is expected to throw exceptions. Regardless, the tool is not
+ * expected to crash and ensures that the thrown exceptions are handled
+ * gracefully.
+ */
+class HdfsToolNegativeTestThrows : public HdfsToolBasicTest {
+public:
+  // Abiding to the rule of 5
+  HdfsToolNegativeTestThrows() = default;
+  HdfsToolNegativeTestThrows(const HdfsToolNegativeTestThrows &) = delete;
+  HdfsToolNegativeTestThrows(HdfsToolNegativeTestThrows &&) = delete;
+  HdfsToolNegativeTestThrows &
+  operator=(const HdfsToolNegativeTestThrows &) = delete;
+  HdfsToolNegativeTestThrows &operator=(HdfsToolNegativeTestThrows &&) = delete;
+  ~HdfsToolNegativeTestThrows() override;
+};
+
+/**
+ * {@class HdfsToolNegativeTestNoThrow} is a fixture that houses negative
+ * tests on {@link hdfs::tools::HdfsTool} interface. It covers the tests where
+ * unfavorable inputs are presented to the {@link hdfs::tools::HdfsTool}
+ * instance and is not expected to throw exceptions and returns false instead.
+ * The tool is not expected to crash and ensures that the unfavorable inputs are
+ * handled gracefully.
+ */
+class HdfsToolNegativeTestNoThrow : public HdfsToolBasicTest {
+public:
+  // Abiding to the rule of 5
+  HdfsToolNegativeTestNoThrow() = default;
+  HdfsToolNegativeTestNoThrow(const HdfsToolNegativeTestNoThrow &) = delete;
+  HdfsToolNegativeTestNoThrow(HdfsToolNegativeTestNoThrow &&) = delete;
+  HdfsToolNegativeTestNoThrow &
+  operator=(const HdfsToolNegativeTestNoThrow &) = delete;
+  HdfsToolNegativeTestNoThrow &
+  operator=(HdfsToolNegativeTestNoThrow &&) = delete;
+  ~HdfsToolNegativeTestNoThrow() override;
+};
+
+#endif

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/tools/hdfs-tool-test.h

@@ -1,123 +0,0 @@
-/*
-  Licensed to the Apache Software Foundation (ASF) under one
-  or more contributor license agreements.  See the NOTICE file
-  distributed with this work for additional information
-  regarding copyright ownership.  The ASF licenses this file
-  to you under the Apache License, Version 2.0 (the
-  "License"); you may not use this file except in compliance
-  with the License.  You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-  Unless required by applicable law or agreed to in writing,
-  software distributed under the License is distributed on an
-  "AS IS" BASIS, 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.
-*/
-
-#ifndef LIBHDFSPP_TOOLS_HDFS_TOOL_TEST
-#define LIBHDFSPP_TOOLS_HDFS_TOOL_TEST
-
-#include <functional>
-#include <memory>
-#include <string>
-#include <tuple>
-
-#include <gmock/gmock.h>
-#include <gtest/gtest.h>
-
-#include "hdfs-tool.h"
-
-/**
- * {@class HdfsToolBasicTest} is a fixture that houses basic tests on {@class
- * hdfs::tools::HdfsTool} interface. It contains the "Happy path" tests which
- * covers the scenarios where {@class hdfs::tools::HdfsTool} is expected to
- * work just fine.
- *
- * {@class HdfsToolBasicTest} is parameterized on a lambda returning an instance
- * of {@class hdfs::tools::HdfsTool} wrapped in a std::unique_ptr. We then run
- * the tests on this instance. Each test runs in isolation. So, a new instance
- * is created for each test.
- */
-class HdfsToolBasicTest
-    : public testing::TestWithParam<
-          std::function<std::unique_ptr<hdfs::tools::HdfsTool>()>> {
-public:
-  // Abiding to the rule of 5
-  HdfsToolBasicTest() = default;
-  HdfsToolBasicTest(const HdfsToolBasicTest &) = delete;
-  HdfsToolBasicTest(HdfsToolBasicTest &&) = delete;
-  HdfsToolBasicTest &operator=(const HdfsToolBasicTest &) = delete;
-  HdfsToolBasicTest &operator=(HdfsToolBasicTest &&) = delete;
-  ~HdfsToolBasicTest() override;
-
-protected:
-  void SetUp() override { hdfs_tool_ = GetParam()(); }
-
-  std::unique_ptr<hdfs::tools::HdfsTool> hdfs_tool_{nullptr};
-};
-
-/**
- * {@class HdfsToolNegativeTest} is a fixture that houses negative tests on
- * {@class hdfs::tools::HdfsTool} interface. It covers the tests where
- * unfavorable inputs are presented to the {@class hdfs::tools::HdfsTool}
- * instance and is expected to not crash and is handled gracefully.
- */
-class HdfsToolNegativeTest : public HdfsToolBasicTest {
-public:
-  // Abiding to the rule of 5
-  HdfsToolNegativeTest() = default;
-  HdfsToolNegativeTest(const HdfsToolNegativeTest &) = delete;
-  HdfsToolNegativeTest(HdfsToolNegativeTest &&) = delete;
-  HdfsToolNegativeTest &operator=(const HdfsToolNegativeTest &) = delete;
-  HdfsToolNegativeTest &operator=(HdfsToolNegativeTest &&) = delete;
-  ~HdfsToolNegativeTest() override;
-};
-
-TEST_P(HdfsToolBasicTest, RunTool) { EXPECT_TRUE(this->hdfs_tool_->Do()); }
-
-TEST_P(HdfsToolNegativeTest, RunTool) {
-  EXPECT_ANY_THROW({ std::ignore = this->hdfs_tool_->Do(); });
-}
-
-template <class T> std::unique_ptr<T> PassAPath() {
-  constexpr auto argc = 2;
-  static std::string exe("hdfs_tool_name");
-  static std::string arg1("a/b/c");
-
-  static char *argv[] = {exe.data(), arg1.data()};
-
-  auto hdfs_tool = std::make_unique<T>(argc, argv);
-  EXPECT_CALL(*hdfs_tool, HandlePath(arg1))
-      .Times(1)
-      .WillOnce(testing::Return(true));
-  return hdfs_tool;
-}
-
-template <class T> std::unique_ptr<T> CallHelp() {
-  constexpr auto argc = 2;
-  static std::string exe("hdfs_tool_name");
-  static std::string arg1("-h");
-
-  static char *argv[] = {exe.data(), arg1.data()};
-
-  auto hdfs_tool = std::make_unique<T>(argc, argv);
-  EXPECT_CALL(*hdfs_tool, HandleHelp())
-      .Times(1)
-      .WillOnce(testing::Return(true));
-  return hdfs_tool;
-}
-
-template <class T> std::unique_ptr<T> Pass2Paths() {
-  constexpr auto argc = 3;
-  static std::string exe("hdfs_tool_name");
-  static std::string arg1("a/b/c");
-  static std::string arg2("d/e/f");
-
-  static char *argv[] = {exe.data(), arg1.data(), arg2.data()};
-  return std::make_unique<T>(argc, argv);
-}
-
-#endif

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/tools/hdfs-tool-tests.cc

@@ -21,10 +21,14 @@
 
 #include "hdfs-allow-snapshot-mock.h"
 #include "hdfs-cat-mock.h"
-#include "hdfs-tool-test.h"
+#include "hdfs-tool-test-fixtures.h"
+#include "hdfs-tool-tests.h"
 
-HdfsToolBasicTest::~HdfsToolBasicTest() {}
+/**
-HdfsToolNegativeTest::~HdfsToolNegativeTest() {}
+ * This file combines the test fixtures defined in {@file
+ * hdfs-tool-test-fixtures.h} and the test cases defined in {@file
+ * hdfs-tool-test.h} to yield the test suite.
+ */
 
 INSTANTIATE_TEST_SUITE_P(
     HdfsAllowSnapshot, HdfsToolBasicTest,
@@ -36,9 +40,9 @@ INSTANTIATE_TEST_SUITE_P(HdfsCat, HdfsToolBasicTest,
                                          CallHelp<hdfs::tools::test::CatMock>));
 
 INSTANTIATE_TEST_SUITE_P(
-    HdfsAllowSnapshot, HdfsToolNegativeTest,
+    HdfsAllowSnapshot, HdfsToolNegativeTestThrows,
     testing::Values(Pass2Paths<hdfs::tools::test::AllowSnapshotMock>));
 
 INSTANTIATE_TEST_SUITE_P(
-    HdfsCat, HdfsToolNegativeTest,
+    HdfsCat, HdfsToolNegativeTestThrows,
     testing::Values(Pass2Paths<hdfs::tools::test::CatMock>));

hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/tools/hdfs-tool-tests.h

@@ -0,0 +1,72 @@
+/*
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, 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.
+*/
+
+#ifndef LIBHDFSPP_TOOLS_HDFS_TOOL_TEST
+#define LIBHDFSPP_TOOLS_HDFS_TOOL_TEST
+
+#include <memory>
+#include <string>
+
+/**
+ * This file contains the generalized test cases to run against  the derivatives
+ * of {@link hdfs::tools::HdfsTool}.
+ *
+ * Each test case passes the arguments to the {@link hdfs::tools::HdfsTool} and
+ * calls the method to set the expectation on the instance of {@link
+ * hdfs::tools::HdfsTool} as defined in its corresponding mock implementation.
+ */
+
+template <class T> std::unique_ptr<T> PassAPath() {
+  constexpr auto argc = 2;
+  static std::string exe("hdfs_tool_name");
+  static std::string arg1("a/b/c");
+
+  static char *argv[] = {exe.data(), arg1.data()};
+
+  auto hdfs_tool = std::make_unique<T>(argc, argv);
+  hdfs_tool->SetExpectations(PassAPath<T>, {arg1});
+  return hdfs_tool;
+}
+
+template <class T> std::unique_ptr<T> CallHelp() {
+  constexpr auto argc = 2;
+  static std::string exe("hdfs_tool_name");
+  static std::string arg1("-h");
+
+  static char *argv[] = {exe.data(), arg1.data()};
+
+  auto hdfs_tool = std::make_unique<T>(argc, argv);
+  hdfs_tool->SetExpectations(CallHelp<T>);
+  return hdfs_tool;
+}
+
+template <class T> std::unique_ptr<T> Pass2Paths() {
+  constexpr auto argc = 3;
+  static std::string exe("hdfs_tool_name");
+  static std::string arg1("a/b/c");
+  static std::string arg2("d/e/f");
+
+  static char *argv[] = {exe.data(), arg1.data(), arg2.data()};
+
+  auto hdfs_tool = std::make_unique<T>(argc, argv);
+  hdfs_tool->SetExpectations(Pass2Paths<T>, {arg1, arg2});
+  return hdfs_tool;
+}
+
+#endif