common/Session.cpp | 3 - test/Makefile.am | 25 ++++++++-- test/UnitClient.cpp | 8 ++- test/countloolkits.hpp | 57 ------------------------ test/httpcrashtest.cpp | 72 ++++++++++-------------------- test/test.cpp | 116 +++++++++++++++++++++++++++++++++++++++++++++++++ test/test.hpp | 13 +++++ wsd/LOOLWSD.cpp | 17 ++++++- wsd/LOOLWSD.hpp | 2 9 files changed, 201 insertions(+), 112 deletions(-)
New commits: commit 2c1508c309f8301e4a28f2d6dddc6557fda807ed Author: Michael Meeks <michael.me...@collabora.com> Date: Tue Sep 19 19:06:46 2017 +0100 Implement more reliable in-process short-cuts. Change-Id: Icdfa71affad147c29df175ae687cbecc3f1f171b diff --git a/test/Makefile.am b/test/Makefile.am index dc4f7ce4..57094967 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -69,7 +69,7 @@ unit_oob_la_SOURCES = UnitOOB.cpp unit_fuzz_la_SOURCES = UnitFuzz.cpp unit_admin_la_SOURCES = UnitAdmin.cpp unit_admin_la_LIBADD = $(CPPUNIT_LIBS) -unit_client_la_SOURCES = UnitClient.cpp ${test_base_source} +unit_client_la_SOURCES = UnitClient.cpp ${test_all_source} unit_client_la_LIBADD = $(CPPUNIT_LIBS) unit_timeout_la_SOURCES = UnitTimeout.cpp unit_prefork_la_SOURCES = UnitPrefork.cpp @@ -91,7 +91,7 @@ check-local: # FIXME 2: unit-oob.la fails with symbol undefined: # UnitWSD::testHandleRequest(UnitWSD::TestRequest, UnitHTTPServerRequest&, UnitHTTPServerResponse&) , TESTS = unit-prefork.la unit-tilecache.la unit-timeout.la unit-oauth.la -# TESTS += unit-client.la +# TESTS = unit-client.la # TESTS += unit-admin.la # TESTS += unit-storage.la else diff --git a/test/UnitClient.cpp b/test/UnitClient.cpp index ec31fb2f..0de67bac 100644 --- a/test/UnitClient.cpp +++ b/test/UnitClient.cpp @@ -57,9 +57,9 @@ public: _worker = std::thread([this]{ if (runClientTests(false, true)) - exitTest (TestResult::Failed); + exitTest(TestResult::Ok); else - exitTest (TestResult::Ok); + exitTest(TestResult::Failed); }); } }; diff --git a/test/test.cpp b/test/test.cpp index 6642a151..4e540478 100644 --- a/test/test.cpp +++ b/test/test.cpp @@ -79,6 +79,7 @@ bool isStandalone() return IsStandalone; } +// returns true on success bool runClientTests(bool standalone, bool verbose) { IsStandalone = standalone; @@ -141,6 +142,9 @@ bool runClientTests(bool standalone, bool verbose) return result.wasSuccessful(); } +// Versions assuming a single user, on a single machine +#ifndef UNIT_CLIENT_TESTS + std::vector<int> getProcPids(const char* exec_filename, bool ignoreZombies = false) { std::vector<int> pids; @@ -222,4 +226,31 @@ std::vector<int> getForKitPids() return pids; } +#else // UNIT_CLIENT_TESTS + +// Here we are compiled inside UnitClient.cpp and we have +// full access to the WSD process internals. + +std::vector<int> getKitPids() +{ + return LOOLWSD::getKitPids(); +} + +/// Get the PID of the forkit +std::vector<int> getForKitPids() +{ + std::vector<int> pids; + if (LOOLWSD::ForKitProcId >= 0) + pids.push_back(LOOLWSD::ForKitProcId); + return pids; +} + +/// How many live lookit processes do we have ? +int getLoolKitProcessCount() +{ + return getKitPids().size(); +} + +#endif // UNIT_CLIENT_TESTS + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/test/test.hpp b/test/test.hpp index 44b7adda..2f85cac2 100644 --- a/test/test.hpp +++ b/test/test.hpp @@ -18,6 +18,8 @@ bool isStandalone(); /// Run the set of client tests we have bool runClientTests(bool standalone, bool verbose); +// ---- Abstraction for standalone vs. WSD ---- + /// Get the list of kit PIDs std::vector<int> getKitPids(); diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index 025bf1cb..1c4088b8 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -2765,7 +2765,6 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/) return returnValue; } - void UnitWSD::testHandleRequest(TestRequest type, UnitHTTPServerRequest& /* request */, UnitHTTPServerResponse& /* response */) { switch (type) @@ -2784,6 +2783,22 @@ void UnitWSD::testHandleRequest(TestRequest type, UnitHTTPServerRequest& /* requ } } +std::vector<int> LOOLWSD::getKitPids() +{ + std::vector<int> pids; + { + std::unique_lock<std::mutex> lock(NewChildrenMutex); + for (const auto &child : NewChildren) + pids.push_back(child->getPid()); + } + { + std::unique_lock<std::mutex> lock(DocBrokersMutex); + for (const auto &it : DocBrokers) + pids.push_back(it.second->getPid()); + } + return pids; +} + #if !defined(BUILDING_TESTS) && !defined(KIT_IN_PROCESS) namespace Util { diff --git a/wsd/LOOLWSD.hpp b/wsd/LOOLWSD.hpp index 86f8dbcb..4c7579f9 100644 --- a/wsd/LOOLWSD.hpp +++ b/wsd/LOOLWSD.hpp @@ -57,6 +57,8 @@ public: static std::unique_ptr<TraceFileWriter> TraceDumper; static std::set<std::string> EditFileExtensions; + static std::vector<int> getKitPids(); + /// Flag to shutdown the server. std::atomic<bool> ShutdownFlag; commit def035037967b0ed667439316efd7dcc75d3d92c Author: Michael Meeks <michael.me...@collabora.com> Date: Tue Sep 19 21:16:44 2017 +0100 Re-factor pid hunting code into test.cpp where we can do better. Prepare the ground for using WSD hooks for this. Change-Id: I5c3e32396b335ad189472ab3a51044372ee304b2 diff --git a/test/Makefile.am b/test/Makefile.am index 005618de..dc4f7ce4 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -43,13 +43,25 @@ wsd_sources = \ ../common/Unit.cpp \ ../net/Socket.cpp +test_base_source = \ + TileQueueTests.cpp \ + WhiteBoxTests.cpp \ + $(wsd_sources) + +test_all_source = \ + $(test_base_source) \ + TileCacheTests.cpp \ + integration-http-server.cpp \ + httpwstest.cpp \ + httpcrashtest.cpp \ + httpwserror.cpp + unittest_CPPFLAGS = -I$(top_srcdir) -DBUILDING_TESTS -unittest_SOURCES = TileQueueTests.cpp WhiteBoxTests.cpp test.cpp $(wsd_sources) +unittest_SOURCES = $(test_base_source) test.cpp unittest_LDADD = $(CPPUNIT_LIBS) test_CPPFLAGS = -I$(top_srcdir) -DBUILDING_TESTS -test_SOURCES = TileCacheTests.cpp integration-http-server.cpp \ - httpwstest.cpp httpcrashtest.cpp httpwserror.cpp $(unittest_SOURCES) +test_SOURCES = $(test_all_source) test.cpp test_LDADD = $(CPPUNIT_LIBS) # unit test modules: @@ -57,7 +69,7 @@ unit_oob_la_SOURCES = UnitOOB.cpp unit_fuzz_la_SOURCES = UnitFuzz.cpp unit_admin_la_SOURCES = UnitAdmin.cpp unit_admin_la_LIBADD = $(CPPUNIT_LIBS) -unit_client_la_SOURCES = UnitClient.cpp ${test_SOURCES} +unit_client_la_SOURCES = UnitClient.cpp ${test_base_source} unit_client_la_LIBADD = $(CPPUNIT_LIBS) unit_timeout_la_SOURCES = UnitTimeout.cpp unit_prefork_la_SOURCES = UnitPrefork.cpp @@ -78,7 +90,10 @@ check-local: ./run_unit.sh --log-file test.log --trs-file test.trs # FIXME 2: unit-oob.la fails with symbol undefined: # UnitWSD::testHandleRequest(UnitWSD::TestRequest, UnitHTTPServerRequest&, UnitHTTPServerResponse&) , -TESTS = unit-prefork.la unit-tilecache.la unit-timeout.la unit-oauth.la # unit-client.la unit-storage.la unit-admin.la # - enable to run unit-tests in wsd ... +TESTS = unit-prefork.la unit-tilecache.la unit-timeout.la unit-oauth.la +# TESTS += unit-client.la +# TESTS += unit-admin.la +# TESTS += unit-storage.la else TESTS = ${top_builddir}/test/test endif diff --git a/test/UnitClient.cpp b/test/UnitClient.cpp index 7461aa28..ec31fb2f 100644 --- a/test/UnitClient.cpp +++ b/test/UnitClient.cpp @@ -69,4 +69,8 @@ UnitBase *unit_create_wsd(void) return new UnitClient(); } +// Allows re-use of UnitClient in test.cpp impls. +#define UNIT_CLIENT_TESTS +#include "test.cpp" + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/test/countloolkits.hpp b/test/countloolkits.hpp index a080ddf5..830246e3 100644 --- a/test/countloolkits.hpp +++ b/test/countloolkits.hpp @@ -21,62 +21,7 @@ #include <Poco/StringTokenizer.h> #include <Common.hpp> - -/// Counts the number of LoolKit process instances without wiating. -static int getLoolKitProcessCount() -{ - int result = 0; - for (auto i = Poco::DirectoryIterator(std::string("/proc")); i != Poco::DirectoryIterator(); ++i) - { - try - { - Poco::Path procEntry = i.path(); - const std::string& fileName = procEntry.getFileName(); - int pid; - std::size_t endPos = 0; - try - { - pid = std::stoi(fileName, &endPos); - } - catch (const std::invalid_argument&) - { - pid = 0; - } - - if (pid > 1 && endPos == fileName.length()) - { - Poco::FileInputStream stat(procEntry.toString() + "/stat"); - std::string statString; - Poco::StreamCopier::copyToString(stat, statString); - Poco::StringTokenizer tokens(statString, " "); - if (tokens.count() > 3 && tokens[1] == "(loolkit)") - { - switch (tokens[2].c_str()[0]) - { - // Dead marker for old and new kernels. - case 'x': - case 'X': - // Don't ignore zombies. - break; - default: - ++result; - break; - } - // std::cout << "Process:" << pid << ", '" << tokens[1] << "'" << " state: " << tokens[2] << std::endl; - } - } - } - catch (const std::exception& ex) - { - // 'File not found' is common here, since there is a race - // between iterating the /proc directory and opening files, - // the process in question might have been gone. - //std::cerr << "Error while iterating processes: " << ex.what() << std::endl; - } - } - - return result; -} +#include "test.hpp" static int countLoolKitProcesses(const int expected) { diff --git a/test/httpcrashtest.cpp b/test/httpcrashtest.cpp index 72720734..0f8b1c3f 100644 --- a/test/httpcrashtest.cpp +++ b/test/httpcrashtest.cpp @@ -15,9 +15,7 @@ #include <cstring> -#include <Poco/DirectoryIterator.h> #include <Poco/Dynamic/Var.h> -#include <Poco/FileStream.h> #include <Poco/JSON/JSON.h> #include <Poco/JSON/Parser.h> #include <Poco/Net/AcceptCertificateHandler.h> @@ -44,6 +42,7 @@ #include <LOOLWebSocket.hpp> #include "helpers.hpp" #include "countloolkits.hpp" +#include "test.hpp" using namespace helpers; @@ -68,7 +67,8 @@ class HTTPCrashTest : public CPPUNIT_NS::TestFixture void testCrashForkit(); static - void killLoKitProcesses(const char* exec_filename); + void killLoKitProcesses(); + void killForkitProcess(); public: HTTPCrashTest() @@ -108,7 +108,7 @@ void HTTPCrashTest::testBarren() const auto testname = "barren "; try { - killLoKitProcesses("(loolkit)"); + killLoKitProcesses(); countLoolKitProcesses(0); std::cerr << "Loading after kill." << std::endl; @@ -134,7 +134,7 @@ void HTTPCrashTest::testCrashKit() std::cerr << "Killing loolkit instances." << std::endl; - killLoKitProcesses("(loolkit)"); + killLoKitProcesses(); countLoolKitProcesses(0); // We expect the client connection to close. @@ -183,7 +183,7 @@ void HTTPCrashTest::testRecoverAfterKitCrash() std::cerr << "Killing loolkit instances." << std::endl; - killLoKitProcesses("(loolkit)"); + killLoKitProcesses(); countLoolKitProcesses(0); // We expect the client connection to close. @@ -207,8 +207,7 @@ void HTTPCrashTest::testCrashForkit() auto socket = loadDocAndGetSocket("empty.odt", _uri, testname); std::cerr << "Killing forkit." << std::endl; - killLoKitProcesses("(loolforkit)"); - killLoKitProcesses("(forkit)"); // on new kernels: prctrl does that. + killForkitProcess(); std::cerr << "Communicating after kill." << std::endl; sendTextFrame(socket, "status", testname); @@ -217,9 +216,8 @@ void HTTPCrashTest::testCrashForkit() // respond close frame socket->shutdown(); - std::cerr << "Killing loolkit." << std::endl; - killLoKitProcesses("(loolkit)"); + killLoKitProcesses(); countLoolKitProcesses(0); std::cerr << "Communicating after kill." << std::endl; loadDocAndGetSocket("empty.odt", _uri, testname); @@ -230,48 +228,28 @@ void HTTPCrashTest::testCrashForkit() } } -void HTTPCrashTest::killLoKitProcesses(const char* exec_filename) +void killPids(const std::vector<int> &pids) { - // Crash all lokit processes. - for (auto it = Poco::DirectoryIterator(std::string("/proc")); it != Poco::DirectoryIterator(); ++it) + std::cout << "kill pids " << pids.size() << "\n"; + // Now kill them + for (int pid : pids) { - try - { - Poco::Path procEntry = it.path(); - const std::string& fileName = procEntry.getFileName(); - int pid; - std::size_t endPos = 0; - try - { - pid = std::stoi(fileName, &endPos); - } - catch (const std::invalid_argument&) - { - pid = 0; - } - - if (pid > 1 && endPos == fileName.length()) - { - Poco::FileInputStream stat(procEntry.toString() + "/stat"); - std::string statString; - Poco::StreamCopier::copyToString(stat, statString); - Poco::StringTokenizer tokens(statString, " "); - if (tokens.count() > 3 && tokens[1] == exec_filename) - { - std::cerr << "Killing " << pid << std::endl; - if (kill(pid, SIGKILL) == -1) - { - std::cerr << "kill(" << pid << ", SIGKILL) failed: " << std::strerror(errno) << std::endl; - } - } - } - } - catch (const Poco::Exception&) - { - } + std::cerr << "Killing " << pid << std::endl; + if (kill(pid, SIGKILL) == -1) + std::cerr << "kill(" << pid << ", SIGKILL) failed: " << std::strerror(errno) << std::endl; } } +void HTTPCrashTest::killLoKitProcesses() +{ + killPids(getKitPids()); +} + +void HTTPCrashTest::killForkitProcess() +{ + killPids(getForKitPids()); +} + CPPUNIT_TEST_SUITE_REGISTRATION(HTTPCrashTest); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/test/test.cpp b/test/test.cpp index e989e3fa..6642a151 100644 --- a/test/test.cpp +++ b/test/test.cpp @@ -23,6 +23,10 @@ #include <cppunit/extensions/TestFactoryRegistry.h> #include <Poco/RegularExpression.h> +#include <Poco/DirectoryIterator.h> +#include <Poco/FileStream.h> +#include <Poco/StreamCopier.h> +#include <Poco/StringTokenizer.h> #include <Log.hpp> @@ -137,4 +141,85 @@ bool runClientTests(bool standalone, bool verbose) return result.wasSuccessful(); } +std::vector<int> getProcPids(const char* exec_filename, bool ignoreZombies = false) +{ + std::vector<int> pids; + + // Crash all lokit processes. + for (auto it = Poco::DirectoryIterator(std::string("/proc")); it != Poco::DirectoryIterator(); ++it) + { + try + { + Poco::Path procEntry = it.path(); + const std::string& fileName = procEntry.getFileName(); + int pid; + std::size_t endPos = 0; + try + { + pid = std::stoi(fileName, &endPos); + } + catch (const std::invalid_argument&) + { + pid = 0; + } + + if (pid > 1 && endPos == fileName.length()) + { + Poco::FileInputStream stat(procEntry.toString() + "/stat"); + std::string statString; + Poco::StreamCopier::copyToString(stat, statString); + Poco::StringTokenizer tokens(statString, " "); + if (tokens.count() > 3 && tokens[1] == exec_filename) + { + if (ignoreZombies) + { + switch (tokens[2].c_str()[0]) + { + // Dead marker for old and new kernels. + case 'x': + case 'X': + // Don't ignore zombies. + break; + default: + pids.push_back(pid); + break; + } + } + else + pids.push_back(pid); + } + } + } + catch (const Poco::Exception&) + { + } + } + return pids; +} + +std::vector<int> getKitPids() +{ + std::vector<int> pids; + + pids = getProcPids("(loolkit)"); + + return pids; +} + +int getLoolKitProcessCount() +{ + return getProcPids("(loolkit)", true).size(); +} + +std::vector<int> getForKitPids() +{ + std::vector<int> pids, pids2; + + pids = getProcPids("(loolforkit)"); + pids2 = getProcPids("(forkit)"); + pids.insert(pids.end(), pids2.begin(), pids2.end()); + + return pids; +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/test/test.hpp b/test/test.hpp index 953cd1af..44b7adda 100644 --- a/test/test.hpp +++ b/test/test.hpp @@ -10,12 +10,23 @@ #ifndef INCLUDED_TEST_HPP #define INCLUDED_TEST_HPP +#include <vector> + /// Are we running inside WSD or by ourselves. bool isStandalone(); /// Run the set of client tests we have bool runClientTests(bool standalone, bool verbose); +/// Get the list of kit PIDs +std::vector<int> getKitPids(); + +/// Get the PID of the forkit +std::vector<int> getForKitPids(); + +/// How many live lookit processes do we have ? +int getLoolKitProcessCount(); + #endif /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ commit be228a160da70a514d2963697c9fb77b4f93b55d Author: Michael Meeks <michael.me...@collabora.com> Date: Sat Sep 16 16:25:54 2017 +0100 Allow unit tests to avoid handleInput. Change-Id: Ib4accd6bbfdc1fc55c45365df275edfa8a68bc59 diff --git a/common/Session.cpp b/common/Session.cpp index c58eb3f0..6a3dc585 100644 --- a/common/Session.cpp +++ b/common/Session.cpp @@ -188,7 +188,8 @@ void Session::handleMessage(bool /*fin*/, WSOpCode /*code*/, std::vector<char> & std::unique_ptr< std::vector<char> > replace; if (UnitBase::get().filterSessionInput(this, &data[0], data.size(), replace)) { - _handleInput(replace->data(), replace->size()); + if (!replace || replace->empty()) + _handleInput(replace->data(), replace->size()); return; } _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits