test: dung out redundant LOOL_TEST_CLIENT_PORT.

And cleanup other related oddities.

Change-Id: I2d179a2ece6a8553e10e406ad4e5da08a2ff58e5

10 files changed, 47 insertions, 84 deletions

diff --git a/kit/ForKit.cpp b/kit/ForKit.cpp
index 164489589..ac0191e33 100644
--- a/kit/ForKit.cpp
+++ b/kit/ForKit.cpp
@@ -396,15 +396,6 @@ int main(int argc, char** argv)
     std::string sysTemplate;
     std::string loTemplate;
 
-#if ENABLE_DEBUG
-    static const char* clientPort = std::getenv("LOOL_TEST_CLIENT_PORT");
-    if (clientPort)
-        ClientPortNumber = std::stoi(clientPort);
-    static const char* masterPort = std::getenv("LOOL_TEST_MASTER_PORT");
-    if (masterPort)
-        MasterLocation = masterPort;
-#endif
-
     for (int i = 0; i < argc; ++i)
     {
         char *cmd = argv[i];
diff --git a/test/Makefile.am b/test/Makefile.am
index 2b369465a..e10cb7d8c 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -2,12 +2,10 @@
 export MAX_CONCURRENCY=4
 AUTOMAKE_OPTION = serial-tests
 
-# unittest: tests that do not need loolwsd running, and that are run during a
-# normal build
-# test: tests that need loolwsd running, and that are run via 'make check'
-check_PROGRAMS = test fakesockettest
+# unittest: tests that run a captive loolwsd as part of themselves.
+check_PROGRAMS = fakesockettest
 
-noinst_PROGRAMS = test fakesockettest unittest
+noinst_PROGRAMS = fakesockettest unittest
 
 AM_CXXFLAGS = $(CPPUNIT_CFLAGS) -DTDOC=\"$(abs_top_srcdir)/test/data\" \
 	-I${top_srcdir}/common -I${top_srcdir}/net -I${top_srcdir}/wsd -I${top_srcdir}/kit
@@ -87,10 +85,6 @@ unittest_CPPFLAGS = -I$(top_srcdir) -DBUILDING_TESTS
 unittest_SOURCES = $(test_base_source) test.cpp
 unittest_LDADD = $(CPPUNIT_LIBS)
 
-test_CPPFLAGS = -I$(top_srcdir) -DBUILDING_TESTS
-test_SOURCES = $(test_all_source) test.cpp
-test_LDADD = $(CPPUNIT_LIBS)
-
 fakesockettest_SOURCES = fakesockettest.cpp ../net/FakeSocket.cpp
 fakesockettest_LDADD = $(CPPUNIT_LIBS)
 
diff --git a/test/TileCacheTests.cpp b/test/TileCacheTests.cpp
index 7dc7d1500..a69099bd5 100644
--- a/test/TileCacheTests.cpp
+++ b/test/TileCacheTests.cpp
@@ -432,6 +432,8 @@ void TileCacheTests::testUnresponsiveClient()
 {
     const std::string testname = "unresponsiveClient-";
 
+    TST_LOG("testUnresponsiveClient.");
+
     std::string documentPath, documentURL;
     getDocumentPathAndURL("hello.odt", documentPath, documentURL, testname);
 
diff --git a/test/helpers.hpp b/test/helpers.hpp
index dd638d191..19bd8804d 100644
--- a/test/helpers.hpp
+++ b/test/helpers.hpp
@@ -187,12 +187,15 @@ Poco::Net::HTTPClientSession* createSession(const Poco::URI& uri)
     return new Poco::Net::HTTPClientSession(uri.getHost(), uri.getPort());
 }
 
+#ifndef UNIT_CLIENT_TESTS
+
 inline int getClientPort()
 {
-    static const char* clientPort = std::getenv("LOOL_TEST_CLIENT_PORT");
-    return clientPort? atoi(clientPort) : DEFAULT_CLIENT_PORT_NUMBER;
+    return DEFAULT_CLIENT_PORT_NUMBER;
 }
 
+#endif
+
 inline std::shared_ptr<Poco::Net::StreamSocket> createRawSocket()
 {
     return
diff --git a/test/integration-http-server.cpp b/test/integration-http-server.cpp
index 4db04737a..8c0c87ac2 100644
--- a/test/integration-http-server.cpp
+++ b/test/integration-http-server.cpp
@@ -92,15 +92,13 @@ public:
     // A server URI which was not added to loolwsd.xml as post_allow IP or a wopi storage host
     Poco::URI getNotAllowedTestServerURI()
     {
-        static const char* clientPort = std::getenv("LOOL_TEST_CLIENT_PORT");
-
         static std::string serverURI(
 #if ENABLE_SSL
-        "https://165.227.162.232:"
+            "https://165.227.162.232:9980"
 #else
-        "http://165.227.162.232:"
+            "http://165.227.162.232:9980"
 #endif
-            + (clientPort? std::string(clientPort) : std::to_string(DEFAULT_CLIENT_PORT_NUMBER)));
+            );
 
         return Poco::URI(serverURI);
     }
@@ -236,9 +234,12 @@ void HTTPServerTest::testScriptsAndLinksPost()
 
 void HTTPServerTest::testConvertTo()
 {
+    const char *testname = "testConvertTo";
     const std::string srcPath = FileUtil::getTempFilePath(TDOC, "hello.odt", "convertTo_");
     std::unique_ptr<Poco::Net::HTTPClientSession> session(helpers::createSession(_uri));
-    session->setTimeout(Poco::Timespan(2, 0)); // 2 seconds.
+    session->setTimeout(Poco::Timespan(5, 0)); // 5 seconds.
+
+    TST_LOG("Convert-to odt -> txt");
 
     Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_POST, "/lool/convert-to");
     Poco::Net::HTMLForm form;
@@ -253,7 +254,7 @@ void HTTPServerTest::testConvertTo()
     catch (const std::exception& ex)
     {
         // In case the server is still starting up.
-        sleep(2);
+        sleep(5);
         form.write(session->sendRequest(request));
     }
 
@@ -279,9 +280,12 @@ void HTTPServerTest::testConvertTo()
 
 void HTTPServerTest::testConvertTo2()
 {
+    const char *testname = "testConvertTo2";
     const std::string srcPath = FileUtil::getTempFilePath(TDOC, "convert-to.xlsx", "convertTo_");
     std::unique_ptr<Poco::Net::HTTPClientSession> session(helpers::createSession(_uri));
-    session->setTimeout(Poco::Timespan(5, 0)); // 5 seconds.
+    session->setTimeout(Poco::Timespan(10, 0)); // 10 seconds.
+
+    TST_LOG("Convert-to #2 xlsx -> png");
 
     Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_POST, "/lool/convert-to");
     Poco::Net::HTMLForm form;
@@ -296,7 +300,7 @@ void HTTPServerTest::testConvertTo2()
     catch (const std::exception& ex)
     {
         // In case the server is still starting up.
-        sleep(2);
+        sleep(5);
         form.write(session->sendRequest(request));
     }
 
diff --git a/test/run_unit.sh.in b/test/run_unit.sh.in
index 5fb8a6936..4b893f23c 100755
--- a/test/run_unit.sh.in
+++ b/test/run_unit.sh.in
@@ -70,55 +70,10 @@ fi
 echo "Test output is '$test_output'"
 echo > $test_output
 
-if test "z$tst" == "z"; then
-     # run the test on a dedicated port
-     export LOOL_TEST_CLIENT_PORT=9984
-     export LOOL_TEST_MASTER_PORT=9985
-
-     echo "Executing external tests"
-     ${trace} \
-     ${abs_top_builddir}/loolwsd --o:sys_template_path="$systemplate_path" \
-                                 --o:child_root_path="$jails_path" \
-                                 --o:storage.filesystem[@allow]=true \
-                                 --o:logging.level=trace \
-                                 --o:logging.file[@enable]=false \
-                                 --o:ssl.key_file_path="${abs_top_builddir}/etc/key.pem" \
-                                 --o:ssl.cert_file_path="${abs_top_builddir}/etc/cert.pem" \
-                                 --o:ssl.ca_file_path="${abs_top_builddir}/etc/ca-chain.cert.pem" \
-                                 --o:admin_console.username=admin --o:admin_console.password=admin \
-                                 --o:storage.ssl.enable=false \
-                                 > "$tst_log" 2>&1 &
-     if test "z${SLEEPFORDEBUGGER}${SLEEPKITFORDEBUGGER}" != "z"; then
-	  echo "sleeping for debugger"
-          sleep ${SLEEPFORDEBUGGER:-0}
-          sleep ${SLEEPKITFORDEBUGGER:-0}
-     fi
-
-     echo "  executing test"
-
-     oldpath=`pwd`
-     cd "${abs_top_builddir}/test"
-     if eval ${trace} ./test ${verbose}; then
-	 echo "Test run_test.sh passed."
-	 echo ":test-result: PASS run_test.sh" >> $oldpath/$test_output
-	 retval=0
-     else
-	 echo ":test-result: FAIL run_test.sh" >> $oldpath/$test_output
-	 retval=1
-     fi
-
-     if test "z${SLEEPFORDEBUGGER}${SLEEPKITFORDEBUGGER}" == "z"; then
-         echo "killing $!"
-         kill $!
-     fi
-
-     exit $retval
-
-else # newer unit tests.
-    echo "Running $tst | $tst_log ...";
-    if ${trace} \
+echo "Running $tst | $tst_log ...";
+if ${trace} \
        ${abs_top_builddir}/loolwsd --o:sys_template_path="$systemplate_path" \
-                                   --o:child_root_path="$jails_path" \
+       --o:child_root_path="$jails_path" \
                                    --o:storage.filesystem[@allow]=true \
                                    --o:logging.level=trace \
                                    --o:ssl.key_file_path="${abs_top_builddir}/etc/key.pem" \
@@ -129,7 +84,7 @@ else # newer unit tests.
                                    --unitlib="${abs_top_builddir}/test/.libs/$tst.so" 2> "$tst_log"; then
         echo "Test $tst passed."
         echo ":test-result: PASS $tst" >> $test_output
-    else
+else
 	cat $tst_log
         echo "============================================================="
         echo "Test failed on unit: $tst re-run with:"
@@ -147,7 +102,6 @@ else # newer unit tests.
 	echo "   $ less $tst_log # for detailed failure log files"
         echo "============================================================="
         echo ":test-result: FAIL $tst" >> $test_output
-    fi
 fi
 
 # vim:set shiftwidth=4 expandtab:
diff --git a/test/test.cpp b/test/test.cpp
index 4a5b0f6d0..57a7c8ff9 100644
--- a/test/test.cpp
+++ b/test/test.cpp
@@ -183,8 +183,12 @@ bool runClientTests(bool standalone, bool verbose)
             std::cerr << "  (cd test; CPPUNIT_TEST_NAME=\"" << (*failures.begin())->failedTestName() << "\" gdb --args " << cmd << ")\n\n";
         }
 #else
+        std::string aLib = UnitBase::get().getUnitLibPath();
+        size_t lastSlash = aLib.rfind('/');
+        if (lastSlash != std::string::npos)
+            aLib = aLib.substr(lastSlash + 1, aLib.length() - lastSlash - 4) + ".la";
         std::cerr << "(cd test; CPPUNIT_TEST_NAME=\"" << (*failures.begin())->failedTestName() <<
-            "\" ./run_unit.sh --test-name " << UnitBase::get().getUnitLibPath() << ")\n\n";
+            "\" ./run_unit.sh --test-name " << aLib << ")\n\n";
 #endif
     }
 
@@ -314,6 +318,11 @@ int getLoolKitProcessCount()
     return getKitPids().size();
 }
 
+int getClientPort()
+{
+    return LOOLWSD::getClientPortNumber();
+}
+
 #endif // UNIT_CLIENT_TESTS
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/test/test.hpp b/test/test.hpp
index 29b6e76d3..a5a2b2280 100644
--- a/test/test.hpp
+++ b/test/test.hpp
@@ -31,6 +31,9 @@ std::vector<int> getDocKitPids();
 /// Get the PID of the forkit
 std::vector<int> getForKitPids();
 
+/// Which port should we connect to get to WSD.
+int getClientPort();
+
 /// How many live loolkit processes do we have ?
 int getLoolKitProcessCount();
 
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 4a4a61302..090cde1c0 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1404,10 +1404,6 @@ void LOOLWSD::handleOption(const std::string& optionName,
     else if (optionName == "careerspan")
         careerSpanMs = std::stoi(value) * 1000; // Convert second to ms
 
-    static const char* clientPort = std::getenv("LOOL_TEST_CLIENT_PORT");
-    if (clientPort)
-        ClientPortNumber = std::stoi(clientPort);
-
     static const char* latencyMs = std::getenv("LOOL_DELAY_SOCKET_MS");
     if (latencyMs)
         SimulatedLatencyMs = std::stoi(latencyMs);
@@ -3670,6 +3666,11 @@ std::vector<std::shared_ptr<DocumentBroker>> LOOLWSD::getBrokersTestOnly()
     return result;
 }
 
+int LOOLWSD::getClientPortNumber()
+{
+    return ClientPortNumber;
+}
+
 std::vector<int> LOOLWSD::getKitPids()
 {
     std::vector<int> pids;
diff --git a/wsd/LOOLWSD.hpp b/wsd/LOOLWSD.hpp
index 71ddb2155..35c283288 100644
--- a/wsd/LOOLWSD.hpp
+++ b/wsd/LOOLWSD.hpp
@@ -81,6 +81,8 @@ public:
     static std::chrono::time_point<std::chrono::system_clock> StartTime;
 
     /// For testing only [!]
+    static int getClientPortNumber();
+    /// For testing only [!]
     static std::vector<int> getKitPids();
     /// For testing only [!] DocumentBrokers are mostly single-threaded with their own thread
     static std::vector<std::shared_ptr<DocumentBroker>> getBrokersTestOnly();