Merge "Cleanup TestableNetworkCallback#assertNoCallback"

tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java

@@ -2391,7 +2391,7 @@ public class ConnectivityManagerTest {
             getHistory().add(new CallbackEntry.BlockedStatusInt(network, blockedReasons));
         }
         private void assertNoBlockedStatusCallback() {
-            super.assertNoCallbackThat(NO_CALLBACK_TIMEOUT_MS,
+            super.assertNoCallback(NO_CALLBACK_TIMEOUT_MS,
                     c -> c instanceof CallbackEntry.BlockedStatus);
         }
     }
@@ -2979,7 +2979,7 @@ public class ConnectivityManagerTest {
             defaultCb.eventuallyExpect(CallbackEntry.AVAILABLE, NETWORK_CALLBACK_TIMEOUT_MS,
                     entry -> cellNetwork.equals(entry.getNetwork()));
             // The network should not validate again.
-            wifiCb.assertNoCallbackThat(NO_CALLBACK_TIMEOUT_MS, c -> isValidatedCaps(c));
+            wifiCb.assertNoCallback(NO_CALLBACK_TIMEOUT_MS, c -> isValidatedCaps(c));
         } finally {
             resetAvoidBadWifi(previousAvoidBadWifi);
             mHttpServer.stop();
@@ -3151,7 +3151,7 @@ public class ConnectivityManagerTest {
      */
     private void assertNoCallbackExceptCapOrLpChange(
             @NonNull final TestableNetworkCallback cb) {
-        cb.assertNoCallbackThat(NO_CALLBACK_TIMEOUT_MS,
+        cb.assertNoCallback(NO_CALLBACK_TIMEOUT_MS,
                 c -> !(c instanceof CallbackEntry.CapabilitiesChanged
                         || c instanceof CallbackEntry.LinkPropertiesChanged));
     }

tests/cts/net/src/android/net/cts/EthernetManagerTest.kt

@@ -57,8 +57,8 @@ import android.os.Build
 import android.os.Handler
 import android.os.Looper
 import android.os.OutcomeReceiver
-import android.os.SystemProperties
 import android.os.Process
+import android.os.SystemProperties
 import android.platform.test.annotations.AppModeFull
 import androidx.test.platform.app.InstrumentationRegistry
 import com.android.net.module.util.ArrayTrackRecord
@@ -77,16 +77,10 @@ import com.android.testutils.TestableNetworkCallback
 import com.android.testutils.assertThrows
 import com.android.testutils.runAsShell
 import com.android.testutils.waitForIdle
-import org.junit.After
-import org.junit.Assume.assumeFalse
-import org.junit.Assume.assumeTrue
-import org.junit.Before
-import org.junit.Test
-import org.junit.runner.RunWith
 import java.io.IOException
 import java.net.Inet6Address
-import java.util.Random
 import java.net.Socket
+import java.util.Random
 import java.util.concurrent.CompletableFuture
 import java.util.concurrent.ExecutionException
 import java.util.concurrent.TimeUnit
@@ -99,6 +93,12 @@ import kotlin.test.assertNotNull
 import kotlin.test.assertNull
 import kotlin.test.assertTrue
 import kotlin.test.fail
+import org.junit.After
+import org.junit.Assume.assumeFalse
+import org.junit.Assume.assumeTrue
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
 
 private const val TAG = "EthernetManagerTest"
 // This timeout does not affect the test duration for passing tests. It needs to be long enough to
@@ -530,12 +530,10 @@ class EthernetManagerTest {
         eventuallyExpect(Lost::class) { n?.equals(it.network) ?: true }
 
     private fun TestableNetworkCallback.assertNeverLost(n: Network? = null) =
-        assertNoCallbackThat() {
+        assertNoCallback { it is Lost && (n?.equals(it.network) ?: true) }
-            it is Lost && (n?.equals(it.network) ?: true)
-        }
 
     private fun TestableNetworkCallback.assertNeverAvailable(n: Network? = null) =
-        assertNoCallbackThat { it is Available && (n?.equals(it.network) ?: true) }
+        assertNoCallback { it is Available && (n?.equals(it.network) ?: true) }
 
     private fun TestableNetworkCallback.expectCapabilitiesWithInterfaceName(name: String) =
         expect<CapabilitiesChanged> { it.caps.networkSpecifier == EthernetNetworkSpecifier(name) }

tests/unit/java/com/android/server/ConnectivityServiceTest.java

@@ -2806,7 +2806,8 @@ public class ConnectivityServiceTest {
             // for any other request.
             generalCb.expectLosing(net2);
             net2.assertNotDisconnected(TEST_CALLBACK_TIMEOUT_MS);
-            generalCb.assertNoCallback();
+            // Timeout 0 because after a while LOST will actually arrive
+            generalCb.assertNoCallback(0 /* timeoutMs */);
             net2.expectDisconnected(UNREASONABLY_LONG_ALARM_WAIT_MS);
         } else {
             net2.expectDisconnected(TEST_CALLBACK_TIMEOUT_MS);
@@ -3006,14 +3007,12 @@ public class ConnectivityServiceTest {
      */
     private class TestNetworkCallback extends TestableNetworkCallback {
         TestNetworkCallback() {
-            super(TEST_CALLBACK_TIMEOUT_MS);
+            // In the context of this test, the testable network callbacks should use waitForIdle
-        }
+            // before calling assertNoCallback in an effort to detect issues where a callback is
-
+            // not yet sent but a message currently in the queue of a handler will cause it to
-        @Override
+            // be sent soon.
-        public void assertNoCallback() {
+            super(TEST_CALLBACK_TIMEOUT_MS, TEST_CALLBACK_TIMEOUT_MS,
-            // TODO: better support this use case in TestableNetworkCallback
+                    ConnectivityServiceTest.this::waitForIdle);
-            waitForIdle();
-            assertNoCallback(0 /* timeout */);
         }
 
         public CallbackEntry.Losing expectLosing(final HasNetwork n, final long timeoutMs) {
@@ -3039,9 +3038,15 @@ public class ConnectivityServiceTest {
 
     // Can't be part of TestNetworkCallback because "cannot be declared static; static methods can
     // only be declared in a static or top level type".
+    static void assertNoCallbacks(final long timeoutMs, TestNetworkCallback ... callbacks) {
+        for (TestNetworkCallback c : callbacks) {
+            c.assertNoCallback(timeoutMs);
+        }
+    }
+
     static void assertNoCallbacks(TestNetworkCallback ... callbacks) {
         for (TestNetworkCallback c : callbacks) {
-            c.assertNoCallback();
+            c.assertNoCallback(); // each callback uses its own timeout
         }
     }
 
@@ -3177,7 +3182,10 @@ public class ConnectivityServiceTest {
         wifiNetworkCallback.expectAvailableThenValidatedCallbacks(mWiFiNetworkAgent);
         cellNetworkCallback.expectLosing(mCellNetworkAgent);
         assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork());
-        assertNoCallbacks(genericNetworkCallback, wifiNetworkCallback, cellNetworkCallback);
+        // Cell will disconnect after the lingering period. Before that elapses check that
+        // there have been no callbacks.
+        assertNoCallbacks(0 /* timeoutMs */,
+                genericNetworkCallback, wifiNetworkCallback, cellNetworkCallback);
 
         mWiFiNetworkAgent.disconnect();
         genericNetworkCallback.expect(CallbackEntry.LOST, mWiFiNetworkAgent);
@@ -3488,7 +3496,7 @@ public class ConnectivityServiceTest {
         callback.expectLosing(mCellNetworkAgent);
 
         // Let linger run its course.
-        callback.assertNoCallback();
+        callback.assertNoCallback(0 /* timeoutMs */);
         final int lingerTimeoutMs = mService.mLingerDelayMs + mService.mLingerDelayMs / 4;
         callback.expect(CallbackEntry.LOST, mCellNetworkAgent, lingerTimeoutMs);
 
@@ -5927,7 +5935,7 @@ public class ConnectivityServiceTest {
                 .clearCapabilities()
                 .addTransportType(TRANSPORT_WIFI)
                 .build();
-        final TestableNetworkCallback wifiCallback = new TestableNetworkCallback();
+        final TestNetworkCallback wifiCallback = new TestNetworkCallback();
         mCm.registerNetworkCallback(wifiRequest, wifiCallback);
 
         // Bring up validated cell and unvalidated wifi.
@@ -10507,7 +10515,7 @@ public class ConnectivityServiceTest {
         mCm.unregisterNetworkCallback(networkCallback);
     }
 
-    private void expectNat64PrefixChange(TestableNetworkCallback callback,
+    private void expectNat64PrefixChange(TestNetworkCallback callback,
             TestNetworkAgentWrapper agent, IpPrefix prefix) {
         callback.expectLinkPropertiesThat(agent, x -> Objects.equals(x.getNat64Prefix(), prefix));
     }
@@ -16409,7 +16417,7 @@ public class ConnectivityServiceTest {
             final NetworkCallback[] callbacks = new NetworkCallback[remainingCount];
             doAsUid(otherAppUid, () -> {
                 for (int i = 0; i < remainingCount; ++i) {
-                    callbacks[i] = new TestableNetworkCallback();
+                    callbacks[i] = new TestNetworkCallback();
                     mCm.registerDefaultNetworkCallback(callbacks[i]);
                 }
             });