platform/tests: merge minor refactoring of route-tests

4 files changed, 65 insertions, 42 deletions

diff --git a/shared/nm-utils/nm-test-utils.h b/shared/nm-utils/nm-test-utils.h
index d2d96620cc..66118fe9cf 100644
--- a/shared/nm-utils/nm-test-utils.h
+++ b/shared/nm-utils/nm-test-utils.h
@@ -82,7 +82,8 @@
  *   Whether long-running tests are enabled is determined as follows (highest priority first):
  *     - specifying the value in NMTST_DEBUG has highest priority
  *     - respect g_test_quick(), if the command line contains '-mslow', '-mquick', '-mthorough'.
- *     - use compile time default
+ *     - use compile time default (CFLAGS=-DNMTST_TEST_QUICK=TRUE)
+ *     - enable slow tests by default
  *
  * "p=PATH"|"s=PATH": passes the path to g_test_init() as "-p" and "-s", respectively.
  *   Unfortunately, these options conflict with "--tap" which our makefile passes to the
@@ -137,12 +138,13 @@
 #define NMTST_WAIT(max_wait_ms, wait) \
 	({ \
 		gboolean _not_expired = TRUE; \
-		gint64 _nmtst_end, _nmtst_max_wait_us = (max_wait_ms) * 1000L; \
+		const gint64 nmtst_wait_start_us = g_get_monotonic_time (); \
+		const gint64 nmtst_wait_duration_us = (max_wait_ms) * 1000L; \
+		const gint64 nmtst_wait_end_us = nmtst_wait_start_us + nmtst_wait_duration_us; \
 		\
-		_nmtst_end = g_get_monotonic_time () + _nmtst_max_wait_us; \
 		while (TRUE) { \
 			{ wait }; \
-			if (g_get_monotonic_time () > _nmtst_end) { \
+			if (g_get_monotonic_time () > nmtst_wait_end_us) { \
 				_not_expired = FALSE; \
 				break; \
 			} \
diff --git a/src/platform/tests/test-common.c b/src/platform/tests/test-common.c
index 2abf6b2cea..eec92af1fd 100644
--- a/src/platform/tests/test-common.c
+++ b/src/platform/tests/test-common.c
@@ -372,7 +372,7 @@ _wait_for_signal_timeout (gpointer user_data)
 }
 
 guint
-nmtstp_wait_for_signal (NMPlatform *platform, guint timeout_ms)
+nmtstp_wait_for_signal (NMPlatform *platform, gint64 timeout_ms)
 {
 	WaitForSignalData data = { 0 };
 	gulong id_link, id_ip4_address, id_ip6_address, id_ip4_route, id_ip6_route;
@@ -387,8 +387,18 @@ nmtstp_wait_for_signal (NMPlatform *platform, guint timeout_ms)
 	id_ip4_route   = g_signal_connect (platform, NM_PLATFORM_SIGNAL_IP4_ROUTE_CHANGED, G_CALLBACK (_wait_for_signal_cb), &data);
 	id_ip6_route   = g_signal_connect (platform, NM_PLATFORM_SIGNAL_IP6_ROUTE_CHANGED, G_CALLBACK (_wait_for_signal_cb), &data);
 
-	if (timeout_ms != 0)
-		data.id = g_timeout_add (timeout_ms, _wait_for_signal_timeout, &data);
+	/* if timeout_ms is negative, it means the wait-time already expired.
+	 * Maybe, we should do nothing and return right away, without even
+	 * processing events from platform. However, that inconsistency (of not
+	 * processing events from mainloop) is inconvenient.
+	 *
+	 * It's better that on the return of nmtstp_wait_for_signal(), we always
+	 * have no events pending. So, a negative timeout is treated the same as
+	 * a zero timeout: we check whether there are any events pending in platform,
+	 * and quite the mainloop immediately afterwards. But we always check. */
+
+	data.id = g_timeout_add (CLAMP (timeout_ms, 0, G_MAXUINT32),
+	                         _wait_for_signal_timeout, &data);
 
 	g_main_loop_run (data.loop);
 
@@ -417,14 +427,14 @@ nmtstp_wait_for_signal_until (NMPlatform *platform, gint64 until_ms)
 		if (until_ms < now)
 			return 0;
 
-		signal_counts = nmtstp_wait_for_signal (platform, MAX (1, until_ms - now));
+		signal_counts = nmtstp_wait_for_signal (platform, until_ms - now);
 		if (signal_counts)
 			return signal_counts;
 	}
 }
 
 const NMPlatformLink *
-nmtstp_wait_for_link (NMPlatform *platform, const char *ifname, NMLinkType expected_link_type, guint timeout_ms)
+nmtstp_wait_for_link (NMPlatform *platform, const char *ifname, NMLinkType expected_link_type, gint64 timeout_ms)
 {
 	return nmtstp_wait_for_link_until (platform, ifname, expected_link_type, nm_utils_get_monotonic_timestamp_ms () + timeout_ms);
 }
@@ -448,7 +458,7 @@ nmtstp_wait_for_link_until (NMPlatform *platform, const char *ifname, NMLinkType
 		if (until_ms < now)
 			return NULL;
 
-		nmtstp_wait_for_signal (platform, MAX (1, until_ms - now));
+		nmtstp_wait_for_signal (platform, until_ms - now);
 	}
 }
 
diff --git a/src/platform/tests/test-common.h b/src/platform/tests/test-common.h
index 373a1fe0da..b4ed0682ea 100644
--- a/src/platform/tests/test-common.h
+++ b/src/platform/tests/test-common.h
@@ -93,9 +93,9 @@ int nmtstp_run_command (const char *format, ...) _nm_printf (1, 2);
 
 /*****************************************************************************/
 
-guint nmtstp_wait_for_signal (NMPlatform *platform, guint timeout_ms);
+guint nmtstp_wait_for_signal (NMPlatform *platform, gint64 timeout_ms);
 guint nmtstp_wait_for_signal_until (NMPlatform *platform, gint64 until_ms);
-const NMPlatformLink *nmtstp_wait_for_link (NMPlatform *platform, const char *ifname, NMLinkType expected_link_type, guint timeout_ms);
+const NMPlatformLink *nmtstp_wait_for_link (NMPlatform *platform, const char *ifname, NMLinkType expected_link_type, gint64 timeout_ms);
 const NMPlatformLink *nmtstp_wait_for_link_until (NMPlatform *platform, const char *ifname, NMLinkType expected_link_type, gint64 until_ms);
 
 #define nmtstp_assert_wait_for_signal(platform, timeout_ms) \
diff --git a/src/platform/tests/test-route.c b/src/platform/tests/test-route.c
index fc395c84ef..2c709d730a 100644
--- a/src/platform/tests/test-route.c
+++ b/src/platform/tests/test-route.c
@@ -31,6 +31,40 @@
 #define EX             NMTSTP_ENV1_EX
 
 static void
+_wait_for_ipv6_addr_non_tentative (NMPlatform *platform,
+                                   gint64 timeout_ms,
+                                   int ifindex,
+                                   guint addr_n,
+                                   const struct in6_addr *addrs)
+{
+	guint i;
+
+	/* Wait that the addresses become non-tentative.  Dummy interfaces are NOARP
+	 * and thus don't do DAD, but the kernel sets the address as tentative for a
+	 * small amount of time, which prevents the immediate addition of the route
+	 * with RTA_PREFSRC */
+
+	NMTST_WAIT_ASSERT (400, {
+		gboolean should_wait = FALSE;
+		const NMPlatformIP6Address *plt_addr;
+
+		for (i = 0; i < addr_n; i++) {
+			plt_addr = nm_platform_ip6_address_get (NM_PLATFORM_GET, ifindex, addrs[i]);
+			if (   !plt_addr
+			    || NM_FLAGS_HAS (plt_addr->n_ifa_flags, IFA_F_TENTATIVE)) {
+				should_wait = TRUE;
+				break;
+			}
+		}
+		if (!should_wait)
+			return;
+		nmtstp_assert_wait_for_signal (NM_PLATFORM_GET,
+		                               (nmtst_wait_end_us - g_get_monotonic_time ()) / 1000);
+	});
+}
+
+
+static void
 ip4_route_callback (NMPlatform *platform, int obj_type_i, int ifindex, const NMPlatformIP4Route *received, int change_type_i, SignalData *data)
 {
 	const NMPObjectType obj_type = obj_type_i;
@@ -260,19 +294,7 @@ test_ip6_route (void)
 	                                       NM_PLATFORM_LIFETIME_PERMANENT, NM_PLATFORM_LIFETIME_PERMANENT, 0));
 	accept_signals (route_added, 0, 1);
 
-	/* Wait that the address becomes non-tentative.  Dummy interfaces are NOARP
-	 * and thus don't do DAD, but the kernel sets the address as tentative for a
-	 * small amount of time, which prevents the immediate addition of the route
-	 * with RTA_PREFSRC */
-	NMTST_WAIT_ASSERT (200, {
-		const NMPlatformIP6Address *plt_addr;
-
-		nmtstp_wait_for_signal (NM_PLATFORM_GET, 50);
-		nm_platform_process_events (NM_PLATFORM_GET);
-		plt_addr = nm_platform_ip6_address_get (NM_PLATFORM_GET, ifindex, pref_src);
-		if (plt_addr && !NM_FLAGS_HAS (plt_addr->n_ifa_flags, IFA_F_TENTATIVE))
-			break;
-	});
+	_wait_for_ipv6_addr_non_tentative (NM_PLATFORM_GET, 200, ifindex, 1, &pref_src);
 
 	/* Add route to gateway */
 	nmtstp_ip6_route_add (NM_PLATFORM_GET, ifindex, NM_IP_CONFIG_SOURCE_USER, gateway, 128, in6addr_any, in6addr_any, metric, mss);
@@ -370,7 +392,6 @@ test_ip4_zero_gateway (void)
 	nmtstp_run_command_check ("ip route flush dev %s", DEVICE_NAME);
 
 	nmtstp_wait_for_signal (NM_PLATFORM_GET, 50);
-	nm_platform_process_events (NM_PLATFORM_GET);
 }
 
 static void
@@ -438,6 +459,7 @@ test_ip6_route_options (gconstpointer test_data)
 	NMPlatformIP6Route rts_add[RTS_MAX] = { };
 	NMPlatformIP6Route rts_cmp[RTS_MAX] = { };
 	NMPlatformIP6Address addr[1] = { };
+	struct in6_addr addr_in6[G_N_ELEMENTS (addr)] = { };
 	guint rts_n = 0;
 	guint addr_n = 0;
 	guint i;
@@ -484,7 +506,10 @@ test_ip6_route_options (gconstpointer test_data)
 	}
 
 	for (i = 0; i < addr_n; i++) {
-		g_assert (nm_platform_ip6_address_add (NM_PLATFORM_GET, IFINDEX,
+		g_assert (addr[i].ifindex == IFINDEX);
+		addr_in6[i] = addr[i].address;
+		g_assert (nm_platform_ip6_address_add (NM_PLATFORM_GET,
+		                                       IFINDEX,
 		                                       addr[i].address,
 		                                       addr[i].plen,
 		                                       addr[i].peer_address,
@@ -493,21 +518,7 @@ test_ip6_route_options (gconstpointer test_data)
 		                                       addr[i].n_ifa_flags));
 	}
 
-	/* Wait that the addresses become non-tentative.  Dummy interfaces are NOARP
-	 * and thus don't do DAD, but the kernel sets the address as tentative for a
-	 * small amount of time, which prevents the immediate addition of the route
-	 * with RTA_PREFSRC */
-	for (i = 0; i < addr_n; i++) {
-		NMTST_WAIT_ASSERT (200, {
-			const NMPlatformIP6Address *plt_addr;
-
-			nmtstp_wait_for_signal (NM_PLATFORM_GET, 50);
-			nm_platform_process_events (NM_PLATFORM_GET);
-			plt_addr = nm_platform_ip6_address_get (NM_PLATFORM_GET, addr[i].ifindex, addr[i].address);
-			if (plt_addr && !NM_FLAGS_HAS (plt_addr->n_ifa_flags, IFA_F_TENTATIVE))
-				break;
-		});
-	}
+	_wait_for_ipv6_addr_non_tentative (NM_PLATFORM_GET, 400, IFINDEX, addr_n, addr_in6);
 
 	for (i = 0; i < rts_n; i++)
 		g_assert (nm_platform_ip6_route_add (NM_PLATFORM_GET, &rts_add[i]));