[test] Improved tests to avoid all-breaking asserts and inter-test messup

CMakeLists.txt

@@ -73,7 +73,12 @@ if (NOT MICROSOFT)
 	# that ENABLE_DEBUG is set as it should.
 	if (ENABLE_DEBUG EQUAL 2)
 		set (CMAKE_BUILD_TYPE "RelWithDebInfo")
-		add_definitions(-DNDEBUG)
+		if (ENABLE_ASSERT)
+			# Add _DEBUG macro if explicitly requested, to enable SRT_ASSERT().
+			add_definitions(-D_DEBUG)
+		else()
+			add_definitions(-DNDEBUG)
+		endif()
 	elseif (ENABLE_DEBUG) # 1, ON, YES, TRUE, Y, or any other non-zero number
 		set (CMAKE_BUILD_TYPE "Debug")
 

srtcore/api.cpp

@@ -2662,31 +2662,34 @@ void srt::CUDTUnited::checkBrokenSockets()
 
     for (sockets_t::iterator j = m_ClosedSockets.begin(); j != m_ClosedSockets.end(); ++j)
     {
+        CUDTSocket* ps = j->second;
+        CUDT& u = ps->core();
+
         // HLOGC(smlog.Debug, log << "checking CLOSED socket: " << j->first);
-        if (!is_zero(j->second->core().m_tsLingerExpiration))
+        if (!is_zero(u.m_tsLingerExpiration))
         {
             // asynchronous close:
-            if ((!j->second->core().m_pSndBuffer) || (0 == j->second->core().m_pSndBuffer->getCurrBufSize()) ||
-                (j->second->core().m_tsLingerExpiration <= steady_clock::now()))
+            if ((!u.m_pSndBuffer) || (0 == u.m_pSndBuffer->getCurrBufSize()) ||
+                (u.m_tsLingerExpiration <= steady_clock::now()))
             {
-                HLOGC(smlog.Debug, log << "checkBrokenSockets: marking CLOSED qualified @" << j->second->m_SocketID);
-                j->second->core().m_tsLingerExpiration = steady_clock::time_point();
-                j->second->core().m_bClosing           = true;
-                j->second->m_tsClosureTimeStamp        = steady_clock::now();
+                HLOGC(smlog.Debug, log << "checkBrokenSockets: marking CLOSED qualified @" << ps->m_SocketID);
+                u.m_tsLingerExpiration = steady_clock::time_point();
+                u.m_bClosing           = true;
+                ps->m_tsClosureTimeStamp        = steady_clock::now();
             }
         }
 
         // timeout 1 second to destroy a socket AND it has been removed from
         // RcvUList
         const steady_clock::time_point now        = steady_clock::now();
-        const steady_clock::duration   closed_ago = now - j->second->m_tsClosureTimeStamp;
+        const steady_clock::duration   closed_ago = now - ps->m_tsClosureTimeStamp;
         if (closed_ago > seconds_from(1))
         {
-            CRNode* rnode = j->second->core().m_pRNode;
+            CRNode* rnode = u.m_pRNode;
             if (!rnode || !rnode->m_bOnList)
             {
                 HLOGC(smlog.Debug,
-                      log << "checkBrokenSockets: @" << j->second->m_SocketID << " closed "
+                      log << "checkBrokenSockets: @" << ps->m_SocketID << " closed "
                           << FormatDuration(closed_ago) << " ago and removed from RcvQ - will remove");
 
                 // HLOGC(smlog.Debug, log << "will unref socket: " << j->first);

srtcore/logging.h

@@ -154,7 +154,7 @@ struct SRT_API LogDispatcher
     LogLevel::type level;
     static const size_t MAX_PREFIX_SIZE = 32;
     char prefix[MAX_PREFIX_SIZE+1];
-    bool enabled;
+    srt::sync::atomic<bool> enabled;
     LogConfig* src_config;
 
     bool isset(int flg) { return (src_config->flags & flg) != 0; }

test/test_env.h

@@ -63,18 +63,26 @@ class TestInit
 class UniqueSocket
 {
     int32_t sock;
+    std::string lab, f;
+    int l;
 
 public:
-    UniqueSocket(int32_t s): sock(s)
+    UniqueSocket(int32_t s, const char* label, const char* file, int line): sock(s)
     {
         if (s == -1)
             throw std::invalid_argument("Invalid socket");
+        lab = label;
+        f = file;
+        l = line;
     }
 
-    UniqueSocket(): sock(-1)
+#define MAKE_UNIQUE_SOCK(name, label, expr) srt::UniqueSocket name (expr, label, __FILE__, __LINE__)
+
+    UniqueSocket(): sock(-1), l(0)
     {
     }
 
+    void close();
     ~UniqueSocket();
 
     operator int32_t() const

test/test_epoll.cpp

@@ -66,7 +66,7 @@ TEST(CEPoll, WaitEmptyCall)
 {
     srt::TestInit srtinit;
 
-    srt::UniqueSocket client_sock = srt_create_socket();
+    MAKE_UNIQUE_SOCK(client_sock, "client", srt_create_socket());
     ASSERT_NE(client_sock, SRT_ERROR);
 
     const int no = 0;
@@ -89,7 +89,7 @@ TEST(CEPoll, UWaitEmptyCall)
 {
     srt::TestInit srtinit;
 
-    srt::UniqueSocket client_sock = srt_create_socket();
+    MAKE_UNIQUE_SOCK(client_sock, "client", srt_create_socket());
     ASSERT_NE(client_sock, SRT_ERROR);
 
     const int no = 0;
@@ -112,7 +112,7 @@ TEST(CEPoll, WaitAllSocketsInEpollReleased)
 {
     srt::TestInit srtinit;
 
-    srt::UniqueSocket client_sock = srt_create_socket();
+    MAKE_UNIQUE_SOCK(client_sock, "client", srt_create_socket());
     ASSERT_NE(client_sock, SRT_ERROR);
 
     const int yes = 1;
@@ -146,7 +146,7 @@ TEST(CEPoll, WaitAllSocketsInEpollReleased2)
 {
     srt::TestInit srtinit;
 
-    srt::UniqueSocket client_sock = srt_create_socket();
+    MAKE_UNIQUE_SOCK(client_sock, "client", srt_create_socket());
     ASSERT_NE(client_sock, SRT_ERROR);
 
     const int yes = 1;
@@ -174,7 +174,7 @@ TEST(CEPoll, WrongEpoll_idOnAddUSock)
 {
     srt::TestInit srtinit;
 
-    srt::UniqueSocket client_sock = srt_create_socket();
+    MAKE_UNIQUE_SOCK(client_sock, "client", srt_create_socket());
     ASSERT_NE(client_sock, SRT_ERROR);
 
     const int no  = 0;
@@ -197,7 +197,7 @@ TEST(CEPoll, HandleEpollEvent)
 {
     srt::TestInit srtinit;
 
-    srt::UniqueSocket client_sock = srt_create_socket();
+    MAKE_UNIQUE_SOCK(client_sock, "client", srt_create_socket());
     EXPECT_NE(client_sock, SRT_ERROR);
 
     const int yes = 1;
@@ -258,7 +258,7 @@ TEST(CEPoll, NotifyConnectionBreak)
     srt::TestInit srtinit;
 
     // 1. Prepare client
-    srt::UniqueSocket client_sock = srt_create_socket();
+    MAKE_UNIQUE_SOCK(client_sock, "client", srt_create_socket());
     ASSERT_NE(client_sock, SRT_ERROR);
 
     const int yes SRT_ATR_UNUSED = 1;
@@ -280,7 +280,7 @@ TEST(CEPoll, NotifyConnectionBreak)
     ASSERT_EQ(inet_pton(AF_INET, "127.0.0.1", &sa_client.sin_addr), 1);
 
     // 2. Prepare server
-    srt::UniqueSocket server_sock = srt_create_socket();
+    MAKE_UNIQUE_SOCK(server_sock, "server_sock", srt_create_socket());
     ASSERT_NE(server_sock, SRT_ERROR);
 
     ASSERT_NE(srt_setsockopt(server_sock, 0, SRTO_RCVSYN, &no, sizeof no), SRT_ERROR); // for async connect
@@ -372,7 +372,7 @@ TEST(CEPoll, HandleEpollEvent2)
 {
     srt::TestInit srtinit;
 
-    srt::UniqueSocket client_sock = srt_create_socket();
+    MAKE_UNIQUE_SOCK(client_sock, "client", srt_create_socket());
     EXPECT_NE(client_sock, SRT_ERROR);
 
     const int yes = 1;
@@ -433,7 +433,7 @@ TEST(CEPoll, HandleEpollNoEvent)
 {
     srt::TestInit srtinit;
 
-    srt::UniqueSocket client_sock = srt_create_socket();
+    MAKE_UNIQUE_SOCK(client_sock, "client", srt_create_socket());
     EXPECT_NE(client_sock, SRT_ERROR);
 
     const int yes = 1;
@@ -483,7 +483,7 @@ TEST(CEPoll, ThreadedUpdate)
 {
     srt::TestInit srtinit;
 
-    srt::UniqueSocket client_sock = srt_create_socket();
+    MAKE_UNIQUE_SOCK(client_sock, "client", srt_create_socket());
     EXPECT_NE(client_sock, SRT_ERROR);
 
     const int no  = 0;

test/test_file_transmission.cpp

@@ -25,12 +25,14 @@
 #include <ctime>
 #include <random>
 #include <vector>
+#include <atomic>
 
 //#pragma comment (lib, "ws2_32.lib")
 
 TEST(Transmission, FileUpload)
 {
     srt::TestInit srtinit;
+    srtinit.HandlePerTestOptions();
 
     // Generate the source file
     // We need a file that will contain more data
@@ -93,7 +95,7 @@ TEST(Transmission, FileUpload)
 
     // Start listener-receiver thread
 
-    bool thread_exit = false;
+    std::atomic<bool> thread_exit { false };
 
     auto client = std::thread([&]
     {
@@ -116,12 +118,17 @@ TEST(Transmission, FileUpload)
         for (;;)
         {
             int n = srt_recv(accepted_sock, buf.data(), 1456);
-            ASSERT_NE(n, SRT_ERROR);
+            EXPECT_NE(n, SRT_ERROR) << srt_getlasterror_str();
             if (n == 0)
             {
                 std::cerr << "Received 0 bytes, breaking.\n";
                 break;
             }
+            else if (n == -1)
+            {
+                std::cerr << "READ FAILED, breaking anyway\n";
+                break;
+            }
 
             // Write to file any amount of data received
             copyfile.write(buf.data(), n);
@@ -173,7 +180,7 @@ TEST(Transmission, FileUpload)
     std::cout << "Sockets closed, joining receiver thread\n";
     client.join();
 
-    std::ifstream tarfile("file.target");
+    std::ifstream tarfile("file.target", std::ios::in | std::ios::binary);
     EXPECT_EQ(!!tarfile, true);
 
     tarfile.seekg(0, std::ios::end);
@@ -182,8 +189,14 @@ TEST(Transmission, FileUpload)
 
     std::cout << "Comparing files\n";
     // Compare files
-    tarfile.seekg(0, std::ios::end);
-    ifile.seekg(0, std::ios::beg);
+
+    // Theoretically it should work if you just rewind to 0, but
+    // on Windows this somehow doesn't work. 
+    tarfile.close();
+    tarfile.open("file.target", std::ios::in | std::ios::binary);
+
+    ifile.close();
+    ifile.open("file.source", std::ios::in | std::ios::binary);
 
     for (size_t i = 0; i < tar_size; ++i)
     {

test/test_main.cpp

@@ -113,6 +113,11 @@ void TestInit::HandlePerTestOptions()
     {
         srt_setloglevel(LOG_DEBUG);
     }
+
+    if (TestEnv::me->OptionPresent("lognote"))
+    {
+        srt_setloglevel(LOG_NOTICE);
+    }
 }
 
 // Copied from ../apps/apputil.cpp, can't really link this file here.
@@ -178,7 +183,14 @@ sockaddr_any CreateAddr(const std::string& name, unsigned short port, int pref_f
 
 UniqueSocket::~UniqueSocket()
 {
-    srt_close(sock);
+    // Could be closed explicitly
+    if (sock != -1)
+        close();
+}
+
+void UniqueSocket::close()
+{
+    EXPECT_NE(srt_close(sock), SRT_ERROR) << lab << " CREATED: "<< f << ":" << l;
 }
 
 }

test/test_many_connections.cpp

@@ -135,6 +135,8 @@ TEST_F(TestConnection, Multiple)
 
     cerr << "Opening " << NSOCK << " connections\n";
 
+    bool overall_test = true;
+
     for (size_t i = 0; i < NSOCK; i++)
     {
         m_connections[i] = srt_create_socket();
@@ -145,13 +147,18 @@ TEST_F(TestConnection, Multiple)
         int conntimeo = 60;
         srt_setsockflag(m_connections[i], SRTO_CONNTIMEO, &conntimeo, sizeof conntimeo);
 
+        SRTSOCKET connres = SRT_INVALID_SOCK;
+
         //cerr << "Connecting #" << i << " to " << sockaddr_any(psa).str() << "...\n";
         //cerr << "Connecting to: " << sockaddr_any(psa).str() << endl;
-        ASSERT_NE(srt_connect(m_connections[i], psa, sizeof lsa), SRT_ERROR);
+        connres = srt_connect(m_connections[i], psa, sizeof lsa);
+        EXPECT_NE(connres, SRT_INVALID_SOCK) << "conn #" << i << ": " << srt_getlasterror_str();
+        if (connres == SRT_INVALID_SOCK)
+            overall_test = false;
 
         // Set now async sending so that sending isn't blocked
         int no = 0;
-        ASSERT_NE(srt_setsockflag(m_connections[i], SRTO_SNDSYN, &no, sizeof no), -1);
+        EXPECT_NE(srt_setsockflag(m_connections[i], SRTO_SNDSYN, &no, sizeof no), -1);
     }
 
     for (size_t j = 1; j <= 100; j++)
@@ -170,13 +177,16 @@ TEST_F(TestConnection, Multiple)
 
     EXPECT_FALSE(m_accept_exit) << "AcceptLoop already broken for some reason!";
     // Up to this moment the server sock should survive
+
     cerr << "Closing server socket\n";
     // Close server socket to break the accept loop
     EXPECT_EQ(srt_close(m_server_sock), 0);
 
     cerr << "Synchronize with the accepting thread\n";
     ex.wait();
     cerr << "Synchronization done\n";
+
+    ASSERT_TRUE(overall_test);
 }