From c19e8a00cc07f95aefa5f765fdbbb26744e54088 Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Thu, 24 Oct 2024 09:56:09 +0900 Subject: [PATCH] Fix flakes due to cell reconnecting on wifi toggle On some networks, telephony may teardown its network (unregisterAfterReplacement) when wifi is toggled to change the MMS capability. Update testRejectUnvalidated_TearDownNetwork to take that into account: right after toggling wifi to make it unvalidated, wifi may still be default if mobile data is being reconnected. Wait for mobile data to reconnect and validate in that case. Bug: 296403855 Test: atest Change-Id: Ief46bed41ecb23d75c9a1f58e7a272e7f3de7caf --- .../com/android/testutils/MiscAsserts.kt | 23 ++++++++++++++ .../net/cts/ConnectivityManagerTest.java | 31 +++++++++++-------- 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/staticlibs/testutils/hostdevice/com/android/testutils/MiscAsserts.kt b/staticlibs/testutils/hostdevice/com/android/testutils/MiscAsserts.kt index 1883387c62..d1d56493e7 100644 --- a/staticlibs/testutils/hostdevice/com/android/testutils/MiscAsserts.kt +++ b/staticlibs/testutils/hostdevice/com/android/testutils/MiscAsserts.kt @@ -20,11 +20,13 @@ package com.android.testutils import com.android.testutils.FunctionalUtils.ThrowingRunnable import java.lang.reflect.Modifier +import java.util.function.BooleanSupplier import kotlin.system.measureTimeMillis import kotlin.test.assertEquals import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertTrue +import kotlin.test.fail private const val TAG = "Connectivity unit test" @@ -118,4 +120,25 @@ fun assertSameElements(expected: List, actual: List) { val actualSet: HashSet = HashSet(actual) assertEquals(actualSet.size, actual.size, "actual list contains duplicates") assertEquals(expectedSet, actualSet) +} + +@JvmOverloads +fun assertEventuallyTrue( + descr: String, + timeoutMs: Long, + pollIntervalMs: Long = 10L, + fn: BooleanSupplier +) { + // This should use SystemClock.elapsedRealtime() since nanoTime does not include time in deep + // sleep, but this is a host-device library and SystemClock is Android-specific (not available + // on host). When waiting for a condition during tests the device would generally not go into + // deep sleep, and the polling sleep would go over the timeout anyway in that case, so this is + // fine. + val limit = System.nanoTime() + timeoutMs * 1000 + while (!fn.asBoolean) { + if (System.nanoTime() > limit) { + fail(descr) + } + Thread.sleep(pollIntervalMs) + } } \ No newline at end of file diff --git a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java index 0e9ea0c552..3a8252a2e6 100644 --- a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java +++ b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java @@ -113,6 +113,7 @@ import static com.android.networkstack.apishim.ConstantsShim.BLOCKED_REASON_NONE import static com.android.networkstack.apishim.ConstantsShim.RECEIVER_EXPORTED; import static com.android.testutils.Cleanup.testAndCleanup; import static com.android.testutils.DevSdkIgnoreRuleKt.SC_V2; +import static com.android.testutils.MiscAsserts.assertEventuallyTrue; import static com.android.testutils.MiscAsserts.assertThrows; import static com.android.testutils.TestNetworkTrackerKt.initTestNetwork; import static com.android.testutils.TestPermissionUtil.runAsShell; @@ -2934,12 +2935,7 @@ public class ConnectivityManagerTest { mCm.getActiveNetwork(), false /* accept */ , false /* always */)); } - private void ensureCellIsValidatedBeforeMockingValidationUrls() { - // Verify that current supported network is validated so that the mock http server will not - // apply to unexpected networks. Also see aosp/2208680. - // - // This may also apply to wifi in principle, but in practice methods that mock validation - // URL all disconnect wifi forcefully anyway, so don't wait for wifi to validate. + private void ensureCellIsValidated() { if (mPackageManager.hasSystemFeature(FEATURE_TELEPHONY)) { new ConnectUtil(mContext).ensureCellularValidated(); } @@ -3022,9 +3018,13 @@ public class ConnectivityManagerTest { networkCallbackRule.requestCell(); final Network wifiNetwork = prepareUnvalidatedNetwork(); - // Default network should not be wifi ,but checking that wifi is not the default doesn't - // guarantee that it won't become the default in the future. - assertNotEquals(wifiNetwork, mCm.getActiveNetwork()); + // Default network should not be wifi ,but checking that Wi-Fi is not the default + // doesn't guarantee that it won't become the default in the future. + // On U 24Q2+ telephony may teardown (unregisterAfterReplacement) its network when Wi-Fi + // is toggled (as part of prepareUnvalidatedNetwork here). Give some time for Wi-Fi to + // not be default in case telephony is reconnecting. + assertEventuallyTrue("Wifi remained default despite being unvalidated", + WIFI_CONNECT_TIMEOUT_MS, () -> !wifiNetwork.equals(mCm.getActiveNetwork())); final TestableNetworkCallback wifiCb = networkCallbackRule.registerNetworkCallback( makeWifiNetworkRequest()); @@ -3061,7 +3061,7 @@ public class ConnectivityManagerTest { try { final Network cellNetwork = networkCallbackRule.requestCell(); - ensureCellIsValidatedBeforeMockingValidationUrls(); + ensureCellIsValidated(); final Network wifiNetwork = prepareValidatedNetwork(); final TestableNetworkCallback defaultCb = @@ -3157,7 +3157,12 @@ public class ConnectivityManagerTest { } private Network prepareValidatedNetwork() throws Exception { - ensureCellIsValidatedBeforeMockingValidationUrls(); + // Verify that current supported network is validated so that the mock http server will not + // apply to unexpected networks. Also see aosp/2208680. + // + // This may also apply to wifi in principle, but in practice methods that mock validation + // URL all disconnect wifi forcefully anyway, so don't wait for wifi to validate. + ensureCellIsValidated(); prepareHttpServer(); configTestServer(Status.NO_CONTENT, Status.NO_CONTENT); @@ -3169,7 +3174,7 @@ public class ConnectivityManagerTest { } private Network preparePartialConnectivity() throws Exception { - ensureCellIsValidatedBeforeMockingValidationUrls(); + ensureCellIsValidated(); prepareHttpServer(); // Configure response code for partial connectivity @@ -3184,7 +3189,7 @@ public class ConnectivityManagerTest { } private Network prepareUnvalidatedNetwork() throws Exception { - ensureCellIsValidatedBeforeMockingValidationUrls(); + ensureCellIsValidated(); prepareHttpServer(); // Configure response code for unvalidated network