Fix CI: clang-tidy const warning, stabilise timing-sensitive tests

Author: TrentHouliston

src/util/network/sock_t.hpp

@@ -75,7 +75,7 @@ namespace util {
                            < std::forward_as_tuple(ntohl(b.ipv4.sin_addr.s_addr), ntohs(b.ipv4.sin_port));
                 }
                 if (a.sock.sa_family == AF_INET6) {
-                    int cmp = std::memcmp(&a.ipv6.sin6_addr, &b.ipv6.sin6_addr, sizeof(in6_addr));
+                    const int cmp = std::memcmp(&a.ipv6.sin6_addr, &b.ipv6.sin6_addr, sizeof(in6_addr));
                     if (cmp != 0) {
                         return cmp < 0;
                     }

tests/tests/nuclearnet/Discovery.cpp

@@ -156,19 +156,19 @@ SCENARIO("Discovery check_timeouts removes stale peers", "[nuclearnet][discovery
 }
 
 SCENARIO("Discovery touch_peer resets timeout", "[nuclearnet][discovery]") {
-    Discovery disc(std::chrono::milliseconds(50));
+    Discovery disc(std::chrono::milliseconds(200));
 
     sock_t peer_addr = make_addr(0x0A000001, 5000);
 
     auto announce = Discovery::build_announce_packet("peer_a", {});
     disc.process_announce(peer_addr, announce.data(), announce.size());
 
     // Wait part of the timeout, then touch
-    std::this_thread::sleep_for(std::chrono::milliseconds(30));
+    std::this_thread::sleep_for(std::chrono::milliseconds(120));
     disc.touch_peer(peer_addr);
 
     // Wait another partial timeout (total would have expired without touch)
-    std::this_thread::sleep_for(std::chrono::milliseconds(30));
+    std::this_thread::sleep_for(std::chrono::milliseconds(120));
 
     auto removed = disc.check_timeouts();
     REQUIRE(removed.empty());

tests/tests/nuclearnet/Integration.cpp

@@ -25,9 +25,7 @@
 
 #include <catch2/catch_test_macros.hpp>
 #include <algorithm>
-#include <atomic>
 #include <chrono>
-#include <condition_variable>
 #include <cstdint>
 #include <functional>
 #include <mutex>
@@ -78,38 +76,8 @@ bool wait_for(const std::function<bool()>& predicate,
 struct NetworkPair {
     NUClearNet a;
     NUClearNet b;
-    std::thread worker_a;
-    std::thread worker_b;
-    std::atomic<bool> running{false};
-
-    void start() {
-        running = true;
-        worker_a = std::thread([this] {
-            while (running) {
-                a.process();
-                std::this_thread::sleep_for(5ms);
-            }
-        });
-        worker_b = std::thread([this] {
-            while (running) {
-                b.process();
-                std::this_thread::sleep_for(5ms);
-            }
-        });
-    }
-
-    void stop() {
-        running = false;
-        if (worker_a.joinable()) {
-            worker_a.join();
-        }
-        if (worker_b.joinable()) {
-            worker_b.join();
-        }
-    }
 
     ~NetworkPair() {
-        stop();
         a.shutdown();
         b.shutdown();
     }
@@ -140,50 +108,41 @@ SCENARIO("Two NUClearNet instances discover and exchange messages", "[nuclearnet
     net.b.set_subscriptions({HASH_A});
 
     std::mutex mutex;
-    std::condition_variable cv;
     std::vector<std::string> join_events;
     std::vector<std::string> leave_events;
     std::vector<std::pair<std::string, std::vector<uint8_t>>> received;
 
     net.a.set_join_callback([&](const PeerInfo& peer) {
         std::lock_guard<std::mutex> lock(mutex);
         join_events.push_back("a:" + peer.name);
-        cv.notify_all();
     });
     net.b.set_join_callback([&](const PeerInfo& peer) {
         std::lock_guard<std::mutex> lock(mutex);
         join_events.push_back("b:" + peer.name);
-        cv.notify_all();
     });
 
     net.a.set_leave_callback([&](const PeerInfo& peer) {
         std::lock_guard<std::mutex> lock(mutex);
         leave_events.push_back("a:" + peer.name);
-        cv.notify_all();
     });
     net.b.set_leave_callback([&](const PeerInfo& peer) {
         std::lock_guard<std::mutex> lock(mutex);
         leave_events.push_back("b:" + peer.name);
-        cv.notify_all();
     });
 
     net.a.set_packet_callback([&](const sock_t&, const std::string& peer_name, uint64_t hash, bool reliable,
                                   std::vector<uint8_t>&& payload) {
         std::lock_guard<std::mutex> lock(mutex);
         received.emplace_back("a:" + peer_name + ":" + std::to_string(hash) + ":" + (reliable ? "1" : "0"),
                               std::move(payload));
-        cv.notify_all();
     });
     net.b.set_packet_callback([&](const sock_t&, const std::string& peer_name, uint64_t hash, bool reliable,
                                   std::vector<uint8_t>&& payload) {
         std::lock_guard<std::mutex> lock(mutex);
         received.emplace_back("b:" + peer_name + ":" + std::to_string(hash) + ":" + (reliable ? "1" : "0"),
                               std::move(payload));
-        cv.notify_all();
     });
 
-    net.start();
-
     REQUIRE(wait_for([&] {
         std::lock_guard<std::mutex> lock(mutex);
         return std::find(join_events.begin(), join_events.end(), "a:bravo") != join_events.end()
@@ -234,10 +193,7 @@ SCENARIO("Two NUClearNet instances discover and exchange messages", "[nuclearnet
         return std::find(leave_events.begin(), leave_events.end(), "a:bravo") != leave_events.end();
     }, 5s, [&] {
         net.a.process();
-        net.b.process();
     }));
-
-    net.stop();
 }
 
 SCENARIO("NUClearNet handles bidirectional reliable traffic", "[nuclearnet][integration]") {
@@ -276,8 +232,6 @@ SCENARIO("NUClearNet handles bidirectional reliable traffic", "[nuclearnet][inte
         b_received.push_back(std::move(payload));
     });
 
-    net.start();
-
     REQUIRE(wait_for([&] {
         std::lock_guard<std::mutex> lock(mutex);
         return std::find(join_events.begin(), join_events.end(), "a:right") != join_events.end()
@@ -306,6 +260,4 @@ SCENARIO("NUClearNet handles bidirectional reliable traffic", "[nuclearnet][inte
         REQUIRE(a_received[0] == small_payload);
         REQUIRE(b_received[0] == large_payload);
     }
-
-    net.stop();
 }