[Libreoffice-commits] online.git: 3 commits - common/Session.cpp test/countloolkits.hpp test/httpcrashtest.cpp test/Makefile.am test/test.cpp test/test.hpp test/UnitClient.cpp wsd/LOOLWSD.cpp wsd/LOOLWSD.hpp

Michael Meeks michael.meeks at collabora.com
Tue Sep 19 20:30:29 UTC 2017


 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.meeks at 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.meeks at 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.meeks at 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;
         }
 


More information about the Libreoffice-commits mailing list