From f82915ebcb075e46ed28e9cbc6706e288e5a9ff2 Mon Sep 17 00:00:00 2001 From: Neil Guertin Date: Fri, 8 Mar 2024 13:00:23 -0500 Subject: [PATCH] Move kit pid helper functions to one file Signed-off-by: Neil Guertin Change-Id: If77b8ac3c7493f4d44b510ec54b68292856f89d5 --- Makefile.am | 2 +- test/{countcoolkits.hpp => KitPidHelpers.cpp} | 115 +++++++++++++----- test/KitPidHelpers.hpp | 52 ++++++++ test/Makefile.am | 8 +- test/TileCacheTests.cpp | 7 +- test/helpers.hpp | 48 -------- test/httpcrashtest.cpp | 7 +- test/httpwstest.cpp | 5 +- test/integration-http-server.cpp | 6 +- test/test.cpp | 33 ----- test/test.hpp | 21 ---- 11 files changed, 153 insertions(+), 151 deletions(-) rename test/{countcoolkits.hpp => KitPidHelpers.cpp} (51%) create mode 100644 test/KitPidHelpers.hpp diff --git a/Makefile.am b/Makefile.am index e0842a7a12..0bf63b2399 100644 --- a/Makefile.am +++ b/Makefile.am @@ -378,7 +378,7 @@ noinst_HEADERS = $(wsd_headers) $(shared_headers) $(kit_headers) \ test/UnitHTTP.hpp \ test/HttpTestServer.hpp \ test/WopiTestServer.hpp \ - test/countcoolkits.hpp \ + test/KitPidHelpers.hpp \ test/lokassert.hpp \ test/test.hpp \ test/testlog.hpp \ diff --git a/test/countcoolkits.hpp b/test/KitPidHelpers.cpp similarity index 51% rename from test/countcoolkits.hpp rename to test/KitPidHelpers.cpp index b9c0435308..f000b00934 100644 --- a/test/countcoolkits.hpp +++ b/test/KitPidHelpers.cpp @@ -8,29 +8,47 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +#include "KitPidHelpers.hpp" -#pragma once - +#include #include #include #include -#include - -#include -#include -#include - +#include #include -#include "Util.hpp" -#include "lokassert.hpp" -#include "test.hpp" -#include "testlog.hpp" +#include -static int countCoolKitProcesses(const int expected, - std::chrono::milliseconds timeoutMs - = std::chrono::milliseconds(COMMAND_TIMEOUT_MS * 8)) +#include +#include +#include + +std::string getPidList(std::set pids); + +std::set helpers::getKitPids() { return COOLWSD::getKitPids(); } + +std::set helpers::getSpareKitPids() { return COOLWSD::getSpareKitPids(); } + +std::set helpers::getDocKitPids() { return COOLWSD::getDocKitPids(); } + +/// Get the PID of the forkit +std::set helpers::getForKitPids() { + std::set pids; + if (COOLWSD::ForKitProcId >= 0) + pids.emplace(COOLWSD::ForKitProcId); + return pids; +} + +/// How many live coolkit processes do we have ? +int helpers::getCoolKitProcessCount() +{ + return getKitPids().size(); +} + +int helpers::countCoolKitProcesses(const int expected) +{ + std::chrono::milliseconds timeoutMs = std::chrono::milliseconds(COMMAND_TIMEOUT_MS) * 8; const auto testname = "countCoolKitProcesses "; TST_LOG_BEGIN("Waiting until coolkit processes are exactly " << expected << ". Coolkits: "); @@ -64,6 +82,7 @@ static int countCoolKitProcesses(const int expected, } TST_LOG_END; + LOK_ASSERT(expected == count); if (expected != count) { TST_LOG_BEGIN("Found " << count << " LoKit processes but was expecting " << expected << ": ["); @@ -87,37 +106,69 @@ static int countCoolKitProcesses(const int expected, return count; } -// FIXME: we probably should make this extern -// and reuse it. As it stands now, it is per -// translation unit, which isn't desirable if -// (in the non-ideal event that) it's not 1, -// it will cause testNoExtraCoolKitsLeft to -// wait unnecessarily and fail. -static int InitialCoolKitCount = 1; -static std::chrono::steady_clock::time_point TestStartTime; - -static void testCountHowManyCoolkits() +void helpers::testCountHowManyCoolkits() { const char testname[] = "countHowManyCoolkits "; - TestStartTime = std::chrono::steady_clock::now(); + resetTestStartTime(); - InitialCoolKitCount = countCoolKitProcesses(InitialCoolKitCount); + countCoolKitProcesses(InitialCoolKitCount); TST_LOG("Initial coolkit count is " << InitialCoolKitCount); LOK_ASSERT(InitialCoolKitCount > 0); - TestStartTime = std::chrono::steady_clock::now(); + resetTestStartTime(); } -static void testNoExtraCoolKitsLeft() +void helpers::testNoExtraCoolKitsLeft() { const char testname[] = "noExtraCoolKitsLeft "; const int countNow = countCoolKitProcesses(InitialCoolKitCount); LOK_ASSERT_EQUAL(InitialCoolKitCount, countNow); - const auto duration = (std::chrono::steady_clock::now() - TestStartTime); - const auto durationMs = std::chrono::duration_cast(duration); + const auto durationMs = timeSinceTestStartMs(); TST_LOG(" (" << durationMs << ')'); } -/* vim:set shiftwidth=4 softtabstop=4 expandtab: */ +void helpers::waitForKitProcessToStop( + const pid_t pid, + const std::string& testname, + const std::chrono::milliseconds timeoutMs /* = COMMAND_TIMEOUT_MS * 8 */, + const std::chrono::milliseconds retryMs /* = 10ms */) +{ + TST_LOG("Waiting for kit process " << pid << " to stop."); + + std::set pids = getDocKitPids(); + TST_LOG("Active kit pids are: " << getPidList(pids)); + + int tries = (timeoutMs / retryMs); + while(pids.contains(pid) && tries >= 0) + { + std::this_thread::sleep_for(retryMs); + pids = getDocKitPids(); + tries--; + } + + if (pids.contains(pid)) + { + std::ostringstream oss; + oss << "Timed out waiting for kit process " << pid << " to stop. Active kit pids are: " << getPidList(pids); + LOK_ASSERT_FAIL(oss.str()); + } + else + { + TST_LOG("Finished waiting for kit process " << pid << " to stop."); + TST_LOG("Active kit pids are: " << getPidList(pids)); + } +} + +std::string getPidList(std::set pids) +{ + std::ostringstream oss; + oss << "["; + for (pid_t i : pids) + { + oss << i << ", "; + } + oss << "]"; + return oss.str(); +} diff --git a/test/KitPidHelpers.hpp b/test/KitPidHelpers.hpp new file mode 100644 index 0000000000..41b7920fbd --- /dev/null +++ b/test/KitPidHelpers.hpp @@ -0,0 +1,52 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */ +/* + * Copyright the Collabora Online contributors. + * + * SPDX-License-Identifier: MPL-2.0 + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +#pragma once + +#include + +#include +#include + +#include + +namespace helpers +{ + +const int InitialCoolKitCount = 1; +void testCountHowManyCoolkits(); +void testNoExtraCoolKitsLeft(); + +int countCoolKitProcesses(const int expected);/*, + std::chrono::milliseconds timeoutMs + = std::chrono::milliseconds(COMMAND_TIMEOUT_MS * 8)); + */ + +/// Get the list of all kit PIDs +std::set getKitPids(); +/// Get the list of spare (unused) kit PIDs +std::set getSpareKitPids(); +/// Get the list of doc (loaded) kit PIDs +std::set getDocKitPids(); + +/// Get the PID of the forkit +std::set getForKitPids(); + +/// How many live coolkit processes do we have ? +int getCoolKitProcessCount(); + +void waitForKitProcessToStop( + const pid_t pid, + const std::string& testname, + const std::chrono::milliseconds timeoutMs = std::chrono::milliseconds(COMMAND_TIMEOUT_MS * 8), + const std::chrono::milliseconds retryMs = std::chrono::milliseconds(10)); + +} diff --git a/test/Makefile.am b/test/Makefile.am index 67de124d47..a6ad80ac4a 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -183,13 +183,13 @@ fakesockettest_LDADD = $(CPPUNIT_LIBS) # old-style unit tests - bootstrapped via UnitClient unit_base_la_SOURCES = UnitClient.cpp ${test_base_sources} -unit_tiletest_la_SOURCES = UnitClient.cpp TileCacheTests.cpp +unit_tiletest_la_SOURCES = UnitClient.cpp TileCacheTests.cpp KitPidHelpers.cpp unit_tiletest_la_LIBADD = $(CPPUNIT_LIBS) -unit_integration_la_SOURCES = UnitClient.cpp integration-http-server.cpp +unit_integration_la_SOURCES = UnitClient.cpp integration-http-server.cpp KitPidHelpers.cpp unit_integration_la_LIBADD = $(CPPUNIT_LIBS) -unit_httpws_la_SOURCES = UnitClient.cpp httpwstest.cpp +unit_httpws_la_SOURCES = UnitClient.cpp httpwstest.cpp KitPidHelpers.cpp unit_httpws_la_LIBADD = $(CPPUNIT_LIBS) -unit_crash_la_SOURCES = UnitClient.cpp httpcrashtest.cpp +unit_crash_la_SOURCES = UnitClient.cpp httpcrashtest.cpp KitPidHelpers.cpp unit_crash_la_LIBADD = $(CPPUNIT_LIBS) # unit test modules: diff --git a/test/TileCacheTests.cpp b/test/TileCacheTests.cpp index ba92a86bf8..eda2cef72d 100644 --- a/test/TileCacheTests.cpp +++ b/test/TileCacheTests.cpp @@ -13,6 +13,9 @@ #include "WebSocketSession.hpp" +#include +#include + #include #include #include @@ -28,11 +31,9 @@ #include #include -#include #include #include -#include -#include +#include using namespace helpers; diff --git a/test/helpers.hpp b/test/helpers.hpp index 642a06ea8a..b8ed45b44f 100644 --- a/test/helpers.hpp +++ b/test/helpers.hpp @@ -13,7 +13,6 @@ #include #include -#include #include #include @@ -951,53 +950,6 @@ inline std::string getAllText(const std::shared_ptr& soc return std::string(); } - -inline std::string getPidList(std::set pids) -{ - std::ostringstream oss; - oss << "["; - for (pid_t i : pids) - { - oss << i << ", "; - } - oss << "]"; - return oss.str(); -} - - -inline void waitForKitProcessToStop( - const pid_t pid, - const std::string& testname, - const std::chrono::milliseconds timeoutMs = std::chrono::milliseconds(COMMAND_TIMEOUT_MS * 8), - const std::chrono::milliseconds retryMs = std::chrono::milliseconds(10)) -{ - - TST_LOG("Waiting for kit process " << pid << " to stop."); - - std::set pids = getDocKitPids(); - TST_LOG("Active kit pids are: " << getPidList(pids)); - - int tries = (timeoutMs / retryMs); - while(pids.contains(pid) && tries >= 0) - { - std::this_thread::sleep_for(retryMs); - pids = getDocKitPids(); - tries--; - } - - if (pids.contains(pid)) - { - std::ostringstream oss; - oss << "Timed out waiting for kit process " << pid << " to stop. Active kit pids are: " << getPidList(pids); - LOK_ASSERT_FAIL(oss.str()); - } - else - { - TST_LOG("Finished waiting for kit process " << pid << " to stop."); - TST_LOG("Active kit pids are: " << getPidList(pids)); - } -} - } /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/test/httpcrashtest.cpp b/test/httpcrashtest.cpp index 1b57127491..714c1ca3a7 100644 --- a/test/httpcrashtest.cpp +++ b/test/httpcrashtest.cpp @@ -39,7 +39,7 @@ #include #include #include -#include +#include using namespace helpers; @@ -153,7 +153,7 @@ void HTTPCrashTest::testCrashKit() TST_LOG("Killing coolkit instances."); killLoKitProcesses(); - countCoolKitProcesses(0, std::chrono::seconds(1)); + countCoolKitProcesses(0);//, std::chrono::seconds(1)); TST_LOG("Reading the error code from the socket."); //FIXME: implement in WebSocketSession. @@ -188,7 +188,7 @@ void HTTPCrashTest::testRecoverAfterKitCrash() TST_LOG("Killing coolkit instances."); killLoKitProcesses(); - countCoolKitProcesses(0, std::chrono::seconds(1)); + countCoolKitProcesses(0);//, std::chrono::seconds(1)); // We expect the client connection to close. TST_LOG("Reconnect after kill."); @@ -270,7 +270,6 @@ static void killPids(const std::set &pids, const std::string& testname) void HTTPCrashTest::killLoKitProcesses() { killPids(getKitPids(), "killLoKitProcesses "); - InitialCoolKitCount = 1; // non-intuitive but it will arrive soon. } void HTTPCrashTest::killForkitProcess() diff --git a/test/httpwstest.cpp b/test/httpwstest.cpp index d310d58a56..a388ca5104 100644 --- a/test/httpwstest.cpp +++ b/test/httpwstest.cpp @@ -30,9 +30,9 @@ #include #include -#include "lokassert.hpp" -#include +#include #include +#include using namespace helpers; @@ -126,6 +126,7 @@ void HTTPWSTest::testExoticLang() void HTTPWSTest::testSaveOnDisconnect() { + const std::string testname = "saveOnDisconnect- "; const std::string text = helpers::genRandomString(40); diff --git a/test/integration-http-server.cpp b/test/integration-http-server.cpp index e9aa312388..bcc9560ce6 100644 --- a/test/integration-http-server.cpp +++ b/test/integration-http-server.cpp @@ -18,7 +18,7 @@ #include #include #include -#include +#include #include #include @@ -100,14 +100,14 @@ public: void setUp() { helpers::resetTestStartTime(); - testCountHowManyCoolkits(); + helpers::testCountHowManyCoolkits(); helpers::resetTestStartTime(); } void tearDown() { helpers::resetTestStartTime(); - testNoExtraCoolKitsLeft(); + helpers::testNoExtraCoolKitsLeft(); helpers::resetTestStartTime(); } diff --git a/test/test.cpp b/test/test.cpp index 59fa4fafbc..fe6c6778dd 100644 --- a/test/test.cpp +++ b/test/test.cpp @@ -37,7 +37,6 @@ #include #include -#include #if ENABLE_SSL #include #include @@ -293,36 +292,4 @@ bool runClientTests(const char* cmd, bool standalone, bool verbose) return result.wasSuccessful(); } -// Standalone tests don't really use WSD -#ifndef STANDALONE_CPPUNIT - -std::set getKitPids() -{ - return COOLWSD::getKitPids(); -} -std::set getSpareKitPids() -{ - return COOLWSD::getSpareKitPids(); -} -std::set getDocKitPids() -{ - return COOLWSD::getDocKitPids(); -} - -/// Get the PID of the forkit -std::set getForKitPids() -{ - std::set pids; - if (COOLWSD::ForKitProcId >= 0) - pids.emplace(COOLWSD::ForKitProcId); - return pids; -} - -/// How many live coolkit processes do we have ? -int getCoolKitProcessCount() -{ - return getKitPids().size(); -} -#endif - /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/test/test.hpp b/test/test.hpp index 03e0a81f10..3a0e7598a2 100644 --- a/test/test.hpp +++ b/test/test.hpp @@ -11,31 +11,10 @@ #pragma once -#include -#include - /// Are we running inside WSD or by ourselves. bool isStandalone(); /// Run the set of client tests we have bool runClientTests(const char* cmd, bool standalone, bool verbose); -// ---- Abstraction for standalone vs. WSD ---- - -/// Get the list of all kit PIDs -std::set getKitPids(); -/// Get the list of spare (unused) kit PIDs -std::set getSpareKitPids(); -/// Get the list of doc (loaded) kit PIDs -std::set getDocKitPids(); - -/// Get the PID of the forkit -std::set getForKitPids(); - -/// Which port should we connect to get to WSD. -int getClientPort(); - -/// How many live coolkit processes do we have ? -int getCoolKitProcessCount(); - /* vim:set shiftwidth=4 softtabstop=4 expandtab: */