Move test methods in ClientInvoker to SCPort

- create `harness` libraries to support the tests Re ECFLOW-1957
ecmwf · Oct 21, 2024 · 3eb760f · 3eb760f
1 parent f9cd127
commit 3eb760f
Show file tree

Hide file tree

Showing 33 changed files with 400 additions and 324 deletions.
diff --git a/libs/attribute/CMakeLists.txt b/libs/attribute/CMakeLists.txt
@@ -35,13 +35,11 @@ ecbuild_add_test(
     nightly
   SOURCES
     ${test_srcs}
-  INCLUDES
-    ../core/test
   LIBS
     ecflow_all
+    test_harness.core
   TEST_DEPENDS
     u_core
-    Boost::boost # Boost header-only libraries must be available (namely unit_test_framework)
 )
 target_clangformat(u_attributes
   CONDITION ENABLE_TESTS

diff --git a/libs/base/CMakeLists.txt b/libs/base/CMakeLists.txt
@@ -8,10 +8,9 @@
 # nor does it submit to any jurisdiction.
 #
 
+add_subdirectory(test/harness)
+
 set(test_srcs
-  # Headers
-  test/MockServer.hpp
-  test/TestHelper.hpp
   # Sources
   test/TestAlterCmd.cpp
   test/TestArchiveAndRestoreCmd.cpp
@@ -50,7 +49,7 @@ ecbuild_add_test(
     ecflow_all
     Threads::Threads
     $<$<BOOL:${OPENSSL_FOUND}>:OpenSSL::SSL>
-    test_support
+    test_harness.base
   TEST_DEPENDS
     u_parser
 )

diff --git a/libs/base/test/harness/CMakeLists.txt b/libs/base/test/harness/CMakeLists.txt
@@ -0,0 +1,29 @@
+#
+# Copyright 2009- ECMWF.
+#
+# This software is licensed under the terms of the Apache Licence version 2.0
+# which can be obtained at http://www.apache.org/licenses/LICENSE-2.0.
+# In applying this licence, ECMWF does not waive the privileges and immunities
+# granted to it by virtue of its status as an intergovernmental organisation
+# nor does it submit to any jurisdiction.
+#
+
+ecbuild_add_library(
+  TARGET
+    test_harness.base
+  NOINSTALL
+  TYPE STATIC
+  SOURCES
+    # Headers
+    MockServer.hpp
+    TestHelper.hpp
+    # Sources
+    MockServer.cpp
+  PUBLIC_INCLUDES
+    ./
+  PUBLIC_LIBS
+    test_harness.core
+)
+target_clangformat(test_harness.base
+  CONDITION ENABLE_TESTS
+)
diff --git a/libs/base/test/harness/MockServer.cpp b/libs/base/test/harness/MockServer.cpp
@@ -0,0 +1,17 @@
+/*
+ * Copyright 2009- ECMWF.
+ *
+ * This software is licensed under the terms of the Apache Licence version 2.0
+ * which can be obtained at http://www.apache.org/licenses/LICENSE-2.0.
+ * In applying this licence, ECMWF does not waive the privileges and immunities
+ * granted to it by virtue of its status as an intergovernmental organisation
+ * nor does it submit to any jurisdiction.
+ */
+
+#include "MockServer.hpp"
+
+void MockServer::set_server_state(SState::State ss) {
+    serverState_    = ss;
+    stats().status_ = static_cast<int>(serverState_);
+    defs_->set_server().set_state(serverState_);
+}
diff --git a/libs/base/test/MockServer.hpp → libs/base/test/harness/MockServer.hpp b/libs/base/test/MockServer.hpp → libs/base/test/harness/MockServer.hpp
@@ -37,11 +37,7 @@ class MockServer : public AbstractServer {
     explicit MockServer(defs_ptr defs) : defs_(defs) { Ecf::set_server(true); }
     ~MockServer() override { Ecf::set_server(false); }
 
-    void set_server_state(SState::State ss) {
-        serverState_    = ss;
-        stats().status_ = static_cast<int>(serverState_);
-        defs_->set_server().set_state(serverState_);
-    }
+    void set_server_state(SState::State ss);
 
     // AbstractServer functions
     SState::State state() const override { return serverState_; }

diff --git a/libs/base/test/TestHelper.hpp → libs/base/test/harness/TestHelper.hpp b/libs/base/test/TestHelper.hpp → libs/base/test/harness/TestHelper.hpp
diff --git a/libs/client/CMakeLists.txt b/libs/client/CMakeLists.txt
@@ -32,8 +32,6 @@ SET_TARGET_PROPERTIES(ecflow_client
     INSTALL_RPATH ""
 )
 
-
-
 # use, i.e. don't skip the full RPATH for the build tree
 #SET(CMAKE_SKIP_BUILD_RPATH  FALSE)
 
@@ -48,12 +46,11 @@ SET_TARGET_PROPERTIES(ecflow_client
 # which point to directories outside the build tree to the install RPATH
 #SET(CMAKE_INSTALL_RPATH_USE_LINK_PATH FALSE)
 
+add_subdirectory(test/harness)
 
 # ================================================================================                         
 # TEST
 set(test_srcs
-  # Headers
-  test/InvokeServer.hpp
   # Sources
   test/TestClient_main.cpp # test entry point
   test/TestClientEnvironment.cpp
@@ -68,10 +65,7 @@ set(test_srcs
 
 if (ENABLE_SERVER)
   list(APPEND test_srcs
-    # Headers
-    test/SCPort.hpp
     # Sources
-    test/SCPort.cpp
     test/TestClientTimeout.cpp
     test/TestClientHandleCmd.cpp
     test/TestCheckPtDefsCmd.cpp
@@ -101,10 +95,9 @@ ecbuild_add_test(
     ../node/test
   LIBS
     ecflow_all
-    Boost::boost # Boost header-only libraries must be available (namely unit_test_framework)
     Boost::timer
     $<$<BOOL:${OPENSSL_FOUND}>:OpenSSL::SSL>
-    test_support
+    test_harness.client
   DEPENDS
     ecflow_server # the server is launched to support tests
   TEST_DEPENDS
@@ -128,18 +121,15 @@ if (ENABLE_ALL_TESTS AND ENABLE_SERVER)
       manual # requires manual definition of env var ECF_TEST_DEFS_DIR
     SOURCES
       # Headers
-      test/SCPort.hpp
       # Sources
-      test/SCPort.cpp
       test/TestSinglePerf.cpp
       test/TestSinglePerf_main.cpp # test entry point
     INCLUDES
       ../base/test
     LIBS
       ecflow_all
-      Boost::boost # Boost header-only libraries must be available (namely unit_test_framework)
       $<$<BOOL:${OPENSSL_FOUND}>:OpenSSL::SSL>
-      test_support
+      test_harness.client
     DEPENDS
       ecflow_server # the server is launched to support tests
     TEST_DEPENDS
@@ -159,19 +149,15 @@ if (ENABLE_ALL_TESTS AND ENABLE_SERVER)
       migration
       manual # requires command line argument: m_test_migration <>
     SOURCES
-      # Headers
-      test/SCPort.hpp
       # Sources
-      test/SCPort.cpp
       test/TestMigration.cpp
       test/TestMigration_main.cpp # test entry point
     INCLUDES
       ../base/test
     LIBS
       ecflow_all
-      Boost::boost # Boost header-only libraries must be available (namely unit_test_framework)
       $<$<BOOL:${OPENSSL_FOUND}>:OpenSSL::SSL>
-      test_support
+      test_harness.client
     DEPENDS
       ecflow_server # the server is launched to support tests
     TEST_DEPENDS

diff --git a/libs/client/src/ecflow/client/ClientInvoker.cpp b/libs/client/src/ecflow/client/ClientInvoker.cpp
@@ -1524,112 +1524,6 @@ std::string ClientInvoker::client_env_host_port() const {
     return host_port;
 }
 
-bool ClientInvoker::is_free_port(int port, bool debug) {
-    // Ping failed, We need to distinguish between:
-    //    a/ Server does not exist : <FREE> port
-    //    b/ Address in use        : <BUSY> port on existing server
-    // Using server_version() but then get error messages
-    // ******** Until this is done we can't implement port hopping **********
-
-    if (debug) {
-        cout << "  ClientInvoker::is_free_port: checking port " << port << "\n";
-    }
-
-    ClientInvoker client;
-    client.set_retry_connection_period(1); // avoid long wait
-    client.set_connection_attempts(1);     // avoid long wait
-
-    const auto the_port = ecf::convert_to<std::string>(port);
-    try {
-        if (debug) {
-            cout << "   Trying to connect to server on '" << Str::LOCALHOST() << ":" << the_port << "'\n";
-        }
-        client.set_host_port(Str::LOCALHOST(), the_port);
-        client.pingServer();
-        if (debug) {
-            cout << "   Connected to server on port " << the_port << ". Returning FALSE\n";
-        }
-        return false;
-    }
-    catch (std::runtime_error& e) {
-        std::string msg = e.what();
-        if (debug) {
-            cout << "   " << msg;
-        }
-        if (msg.find("authentication failed") != std::string::npos) {
-            if (debug) {
-                cout << "   Could not connect, due to authentication failure, hence port " << the_port
-                     << " is used. Returning FALSE\n";
-            }
-            return false;
-        }
-        if (msg.find("invalid_argument") != std::string::npos) {
-            if (debug) {
-                cout << "   Mixing 4 and 5 series ?, hence port " << the_port << " is used. Returning FALSE\n";
-            }
-            return false;
-        }
-        else {
-            if (debug) {
-                cout << "   Found free port " << the_port << "\n";
-            }
-        }
-    }
-    return true;
-}
-
-std::string ClientInvoker::find_free_port(int seed_port_number, bool debug) {
-    // Ping failed, We need to distinguish between:
-    //    a/ Server does not exist : <FREE> port
-    //    b/ Address in use        : <BUSY> port on existing server
-    // Using server_version() but then get error messages
-    // ******** Until this is done we can't implement port hopping **********
-
-    if (debug)
-        cout << "  ClientInvoker::find_free_port: starting with port " << seed_port_number << "\n";
-    int the_port = seed_port_number;
-    std::string free_port;
-    ClientInvoker client;
-    client.set_retry_connection_period(1); // avoid long wait
-    client.set_connection_attempts(1);     // avoid long wait
-    while (true) {
-        free_port = ecf::convert_to<std::string>(the_port);
-        try {
-            if (debug)
-                cout << "   Trying to connect to server on '" << Str::LOCALHOST() << ":" << free_port << "'\n";
-            client.set_host_port(Str::LOCALHOST(), free_port);
-            client.pingServer();
-            if (debug)
-                cout << "   Connected to server on port " << free_port << " trying next port\n";
-            the_port++;
-        }
-        catch (std::runtime_error& e) {
-            std::string error_msg = e.what();
-            if (debug)
-                cout << "   " << error_msg;
-            if (error_msg.find("authentication failed") != std::string::npos) {
-                if (debug)
-                    cout << "   Could not connect, due to authentication failure, hence port " << the_port
-                         << " is used, trying next port\n";
-                the_port++;
-                continue;
-            }
-            if (error_msg.find("invalid_argument") != std::string::npos) {
-                if (debug)
-                    cout << "   Mixing 4 and 5 series ?, hence port " << the_port << " is used, trying next port\n";
-                the_port++;
-                continue;
-            }
-            else {
-                if (debug)
-                    cout << "   Found free port " << free_port << "\n";
-                break;
-            }
-        }
-    }
-    return free_port;
-}
-
 bool ClientInvoker::wait_for_server_reply(int time_out) const {
     DurationTimer timer;
     while (true) {

diff --git a/libs/client/src/ecflow/client/ClientInvoker.hpp b/libs/client/src/ecflow/client/ClientInvoker.hpp
@@ -222,20 +222,6 @@ class ClientInvoker {
     int news(defs_ptr& client_defs) const;
     int news_local() const;
 
-    /**
-     * Check if there is *no *ecFlow server running on the given port.
-     * 
-     * The check is performed by connecting to the port (on localhost) and issuing a ping request.
-     * 
-     * \returns false, if: (1) the server answers with a ping response;
-     *                     (2) the request is accepted, but fails due to failed authentication;
-     *                     (3) there is a mismatch communication protocol (4.x vs 5.x)
-     *          true, otherwise
-     */
-    static bool is_free_port(int port, bool debug = false);
-    // find free port on local host. Not 100% accurate, use in test
-    static std::string find_free_port(int seed_port_number, bool debug = false);
-
     bool wait_for_server_reply(int time_out = 60) const; // wait for server reply, returning false means timed out.
     bool wait_for_server_death(
         int time_out = 60) const; // wait for server reply, returning true means server died,false means timed out.