Make sure the test server waits for incoming connection

The socket state is local, so it's possible to report 'connected', resume to coroutine finish it and destroy the socket before the server has time to actually get the incoming connection. This results in the test server being stopped by the test before it actually starts waiting for incoming connection, causing the server to incorrectly report the test as time out. The fix is for the test to wait at the end for the server to confirm it has gotten the incoming connection.
danvratil · May 2, 2022 · afa8824 · afa8824
1 parent f9aa28e
commit afa8824
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 0 deletions.
diff --git a/tests/qcoroabstractsocket.cpp b/tests/qcoroabstractsocket.cpp
@@ -24,6 +24,9 @@ class QCoroAbstractSocketTest : public QCoro::TestObject<QCoroAbstractSocketTest
         co_await qCoro(socket).waitForConnected();
 
         QCORO_COMPARE(socket.state(), QAbstractSocket::ConnectedState);
+
+        // Make sure the server gets the connection as well
+        QCORO_VERIFY(mServer.waitForConnection());
     }
 
     void testThenWaitForConnectedTriggers_coro(TestLoop &el) {
@@ -37,6 +40,7 @@ class QCoroAbstractSocketTest : public QCoro::TestObject<QCoroAbstractSocketTest
         });
         el.exec();
         QVERIFY(called);
+        QVERIFY(mServer.waitForConnection());
     }
 
     QCoro::Task<> testWaitForDisconnectedTriggers_coro(QCoro::TestContext) {
@@ -49,6 +53,8 @@ class QCoroAbstractSocketTest : public QCoro::TestObject<QCoroAbstractSocketTest
         co_await qCoro(socket).waitForDisconnected();
 
         QCORO_COMPARE(socket.state(), QAbstractSocket::UnconnectedState);
+
+        QCORO_VERIFY(mServer.waitForConnection());
     }
 
     void testThenWaitForDisconnectedTriggers_coro(TestLoop &el) {
@@ -72,6 +78,8 @@ class QCoroAbstractSocketTest : public QCoro::TestObject<QCoroAbstractSocketTest
         el.exec();
 
         QVERIFY(called);
+
+        QVERIFY(mServer.waitForConnection());
     }
 
     QCoro::Task<> testDoesntCoAwaitConnectedSocket_coro(QCoro::TestContext context) {
@@ -82,6 +90,8 @@ class QCoroAbstractSocketTest : public QCoro::TestObject<QCoroAbstractSocketTest
 
         context.setShouldNotSuspend();
         co_await qCoro(socket).waitForConnected();
+
+        QCORO_VERIFY(mServer.waitForConnection());
     }
 
     void testThenDoesntCoAwaitConnectedSocket_coro(TestLoop &el) {
@@ -103,6 +113,8 @@ class QCoroAbstractSocketTest : public QCoro::TestObject<QCoroAbstractSocketTest
         el.exec();
 
         QVERIFY(called);
+
+        QVERIFY(mServer.waitForConnection());
     }
 
     QCoro::Task<> testDoesntCoAwaitDisconnectedSocket_coro(QCoro::TestContext context) {
@@ -138,6 +150,7 @@ class QCoroAbstractSocketTest : public QCoro::TestObject<QCoroAbstractSocketTest
         co_await qCoro(socket).connectToHost(QHostAddress::LocalHost, mServer.port());
 
         QCORO_COMPARE(socket.state(), QAbstractSocket::ConnectedState);
+        QCORO_VERIFY(mServer.waitForConnection());
     }
 
     void testThenConnectToServerWithArgs_coro(TestLoop &el) {
@@ -151,6 +164,7 @@ class QCoroAbstractSocketTest : public QCoro::TestObject<QCoroAbstractSocketTest
         });
         el.exec();
         QVERIFY(called);
+        QVERIFY(mServer.waitForConnection());
     }
 
     QCoro::Task<> testWaitForConnectedTimeout_coro(QCoro::TestContext) {
@@ -186,6 +200,8 @@ class QCoroAbstractSocketTest : public QCoro::TestObject<QCoroAbstractSocketTest
         QCORO_COMPARE(socket.state(), QAbstractSocket::ConnectedState);
 
         QCORO_TEST_TIMEOUT(co_await qCoro(socket).waitForDisconnected(10ms));
+
+        QCORO_VERIFY(mServer.waitForConnection());
     }
 
     void testThenWaitForDisconnectedTimeout_coro(TestLoop &el) {
@@ -208,6 +224,7 @@ class QCoroAbstractSocketTest : public QCoro::TestObject<QCoroAbstractSocketTest
             });
         });
         el.exec();
+        QVERIFY(mServer.waitForConnection());
     }
 
     QCoro::Task<> testReadAllTriggers_coro(QCoro::TestContext) {
@@ -218,6 +235,7 @@ class QCoroAbstractSocketTest : public QCoro::TestObject<QCoroAbstractSocketTest
         socket.write("GET /stream HTTP/1.1\r\n");
 
         QCORO_TEST_IODEVICE_READALL(socket);
+        QCORO_VERIFY(mServer.waitForConnection());
     }
 
     void testThenReadAllTriggers_coro(TestLoop &el) {
@@ -242,6 +260,7 @@ class QCoroAbstractSocketTest : public QCoro::TestObject<QCoroAbstractSocketTest
         el.exec();
 
         QVERIFY(called);
+        QVERIFY(mServer.waitForConnection());
     }
 
     QCoro::Task<> testReadTriggers_coro(QCoro::TestContext) {
@@ -252,6 +271,7 @@ class QCoroAbstractSocketTest : public QCoro::TestObject<QCoroAbstractSocketTest
         socket.write("GET /stream HTTP/1.1\r\n");
 
         QCORO_TEST_IODEVICE_READ(socket);
+        QCORO_VERIFY(mServer.waitForConnection());
     }
 
      void testThenReadTriggers_coro(TestLoop &el) {
@@ -276,6 +296,7 @@ class QCoroAbstractSocketTest : public QCoro::TestObject<QCoroAbstractSocketTest
         el.exec();
 
         QVERIFY(called);
+        QVERIFY(mServer.waitForConnection());
     }
 
     QCoro::Task<> testReadLineTriggers_coro(QCoro::TestContext) {
@@ -287,6 +308,7 @@ class QCoroAbstractSocketTest : public QCoro::TestObject<QCoroAbstractSocketTest
 
         QCORO_TEST_IODEVICE_READLINE(socket);
         QCORO_COMPARE(lines.size(), 14);
+        QCORO_VERIFY(mServer.waitForConnection());
     }
 
      void testThenReadLineTriggers_coro(TestLoop &el) {
@@ -311,6 +333,7 @@ class QCoroAbstractSocketTest : public QCoro::TestObject<QCoroAbstractSocketTest
         el.exec();
 
         QVERIFY(called);
+        QVERIFY(mServer.waitForConnection());
     }
 
 private Q_SLOTS:

diff --git a/tests/qcorolocalsocket.cpp b/tests/qcorolocalsocket.cpp
@@ -25,6 +25,7 @@ class QCoroLocalSocketTest : public QCoro::TestObject<QCoroLocalSocketTest> {
         co_await qCoro(socket).waitForConnected();
 
         QCORO_COMPARE(socket.state(), QLocalSocket::ConnectedState);
+        QCORO_VERIFY(mServer.waitForConnection());
     }
 
     void testThenWaitForConnectedTriggers_coro(TestLoop &el) {
@@ -38,6 +39,7 @@ class QCoroLocalSocketTest : public QCoro::TestObject<QCoroLocalSocketTest> {
         });
         el.exec();
         QVERIFY(called);
+        QVERIFY(mServer.waitForConnection());
     }
 
     QCoro::Task<> testWaitForDisconnectedTriggers_coro(QCoro::TestContext) {
@@ -50,6 +52,7 @@ class QCoroLocalSocketTest : public QCoro::TestObject<QCoroLocalSocketTest> {
         co_await qCoro(socket).waitForDisconnected();
 
         QCORO_COMPARE(socket.state(), QLocalSocket::UnconnectedState);
+        QCORO_VERIFY(mServer.waitForConnection());
     }
 
     void testThenWaitForDisconnectedTriggers_coro(TestLoop &el) {
@@ -66,6 +69,7 @@ class QCoroLocalSocketTest : public QCoro::TestObject<QCoroLocalSocketTest> {
         });
         el.exec();
         QVERIFY(called);
+        QVERIFY(mServer.waitForConnection());
     }
 
     // On Linux at least, QLocalSocket connects immediately and synchronously
@@ -78,6 +82,7 @@ class QCoroLocalSocketTest : public QCoro::TestObject<QCoroLocalSocketTest> {
         QCORO_COMPARE(socket.state(), QLocalSocket::ConnectedState);
 
         co_await qCoro(socket).waitForConnected();
+        QCORO_VERIFY(mServer.waitForConnection());
     }
 
     void testThenDoesntCoAwaitConnectedSocket_coro(TestLoop &el) {
@@ -93,6 +98,7 @@ class QCoroLocalSocketTest : public QCoro::TestObject<QCoroLocalSocketTest> {
         });
         el.exec();
         QVERIFY(called);
+        QVERIFY(mServer.waitForConnection());
     }
 
     QCoro::Task<> testDoesntCoAwaitDisconnectedSocket_coro(QCoro::TestContext context) {
@@ -128,6 +134,7 @@ class QCoroLocalSocketTest : public QCoro::TestObject<QCoroLocalSocketTest> {
         co_await qCoro(socket).connectToServer(QCoroLocalSocketTest::getSocketName());
 
         QCORO_COMPARE(socket.state(), QLocalSocket::ConnectedState);
+        QCORO_VERIFY(mServer.waitForConnection());
     }
 
     void testThenConnectToServerWithArgs_coro(TestLoop &el) {
@@ -140,6 +147,7 @@ class QCoroLocalSocketTest : public QCoro::TestObject<QCoroLocalSocketTest> {
         });
         el.exec();
         QVERIFY(called);
+        QVERIFY(mServer.waitForConnection());
     }
 
     QCoro::Task<> testConnectToServer_coro(QCoro::TestContext context) {
@@ -151,6 +159,7 @@ class QCoroLocalSocketTest : public QCoro::TestObject<QCoroLocalSocketTest> {
         co_await qCoro(socket).connectToServer();
 
         QCORO_COMPARE(socket.state(), QLocalSocket::ConnectedState);
+        QCORO_VERIFY(mServer.waitForConnection());
     }
 
     void testThenConnectToServer_coro(TestLoop &el) {
@@ -164,6 +173,7 @@ class QCoroLocalSocketTest : public QCoro::TestObject<QCoroLocalSocketTest> {
         });
         el.exec();
         QVERIFY(called);
+        QVERIFY(mServer.waitForConnection());
     }
 
     QCoro::Task<> testWaitForConnectedTimeout_coro(QCoro::TestContext) {
@@ -198,6 +208,7 @@ class QCoroLocalSocketTest : public QCoro::TestObject<QCoroLocalSocketTest> {
         QCORO_COMPARE(socket.state(), QLocalSocket::ConnectedState);
 
         QCORO_TEST_TIMEOUT(co_await qCoro(socket).waitForDisconnected(10ms));
+        QCORO_VERIFY(mServer.waitForConnection());
     }
 
     void testThenWaitForDisconnectedTimeout_coro(TestLoop &el) {
@@ -215,6 +226,7 @@ class QCoroLocalSocketTest : public QCoro::TestObject<QCoroLocalSocketTest> {
         const auto end = std::chrono::steady_clock::now();
         QVERIFY(end - start < 500ms);
         QVERIFY(called);
+        QVERIFY(mServer.waitForConnection());
     }
 
     QCoro::Task<> testReadAllTriggers_coro(QCoro::TestContext) {
@@ -225,6 +237,7 @@ class QCoroLocalSocketTest : public QCoro::TestObject<QCoroLocalSocketTest> {
         socket.write("GET /stream HTTP/1.1\r\n");
 
         QCORO_TEST_IODEVICE_READALL(socket);
+        QCORO_VERIFY(mServer.waitForConnection());
     }
 
     void testThenReadAllTriggers_coro(TestLoop &el) {
@@ -241,6 +254,7 @@ class QCoroLocalSocketTest : public QCoro::TestObject<QCoroLocalSocketTest> {
         el.exec();
 
         QVERIFY(called);
+        QVERIFY(mServer.waitForConnection());
     }
 
     QCoro::Task<> testReadTriggers_coro(QCoro::TestContext) {
@@ -251,6 +265,7 @@ class QCoroLocalSocketTest : public QCoro::TestObject<QCoroLocalSocketTest> {
         socket.write("GET /stream HTTP/1.1\r\n");
 
         QCORO_TEST_IODEVICE_READ(socket);
+        QCORO_VERIFY(mServer.waitForConnection());
     }
 
     void testThenReadTriggers_coro(TestLoop &el) {
@@ -266,6 +281,7 @@ class QCoroLocalSocketTest : public QCoro::TestObject<QCoroLocalSocketTest> {
         socket.write("GET /block HTTP/1.1\r\n");
         el.exec();
         QVERIFY(called);
+        QVERIFY(mServer.waitForConnection());
     }
 
     QCoro::Task<> testReadLineTriggers_coro(QCoro::TestContext) {
@@ -277,6 +293,7 @@ class QCoroLocalSocketTest : public QCoro::TestObject<QCoroLocalSocketTest> {
 
         QCORO_TEST_IODEVICE_READLINE(socket);
         QCORO_COMPARE(lines.size(), 14);
+        QCORO_VERIFY(mServer.waitForConnection());
     }
 
     void testThenReadLineTriggers_coro(TestLoop &el) {
@@ -293,6 +310,7 @@ class QCoroLocalSocketTest : public QCoro::TestObject<QCoroLocalSocketTest> {
         el.exec();
 
         QVERIFY(called);
+        QVERIFY(mServer.waitForConnection());
     }
 
 private Q_SLOTS:

diff --git a/tests/testhttpserver.h b/tests/testhttpserver.h
@@ -52,6 +52,8 @@ class TestHttpServer {
 public:
     template<typename T>
     void start(const T &name) {
+        mPort = 0;
+        mHasConnection = false;
         mStop = false;
         mExpectTimeout = false;
         // Can't use QThread::create, it's only available when Qt is built with C++17,
@@ -69,6 +71,7 @@ class TestHttpServer {
         }
         mThread.reset();
         mPort = 0;
+        mHasConnection = false;
     }
 
     uint16_t port() const {
@@ -79,6 +82,11 @@ class TestHttpServer {
         mExpectTimeout = expectTimeout;
     }
 
+    bool waitForConnection() {
+        std::unique_lock lock(mReadyMutex);
+        return mServerReady.wait_for(lock, std::chrono::seconds(5), [this]() { return mHasConnection; });
+    }
+
 private:
     template<typename T>
     void run(const T &name) {
@@ -117,6 +125,9 @@ class TestHttpServer {
             return;
         }
 
+        mHasConnection = true;
+        mServerReady.notify_all();
+
         if (conn->waitForReadyRead(1000)) {
             const auto request = conn->readLine();
             qDebug() << request;
@@ -171,6 +182,7 @@ class TestHttpServer {
     std::mutex mReadyMutex;
     std::condition_variable mServerReady;
     uint16_t mPort = 0;
+    bool mHasConnection = false;
     std::atomic_bool mStop = false;
     std::atomic_bool mExpectTimeout = false;
 };