From 0cc8d81cc79a18a50ac4aa8833725c5be048cc70 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 21 Mar 2025 16:58:44 +0900 Subject: [PATCH] Make sure tests know to handle both early creation true and false This is necessary to pass tests whatever the values of the features are. NetworkAgentTest was updated assuming the feature was true, so this patch revives the old code for when it's false. TestableNetworkAgent reads the feature directly from ConnectivityManager when it's not mocked. Test: CtsNetTestCases Change-Id: Ia556fbb1d1b6a38a4995b021981e2693d778bfff --- .../android/testutils/TestableNetworkAgent.kt | 12 +++-- .../src/android/net/cts/NetworkAgentTest.kt | 46 +++++++++++++---- .../server/ConnectivityServiceTest.java | 51 ++++++++++++++----- 3 files changed, 82 insertions(+), 27 deletions(-) diff --git a/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkAgent.kt b/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkAgent.kt index 0413ed408d..f5f2e6913b 100644 --- a/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkAgent.kt +++ b/staticlibs/testutils/devicetests/com/android/testutils/TestableNetworkAgent.kt @@ -17,6 +17,7 @@ package com.android.testutils import android.content.Context +import android.net.ConnectivityManager import android.net.ConnectivityManager.FEATURE_QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER import android.net.InetAddresses.parseNumericAddress import android.net.KeepalivePacketData @@ -77,9 +78,7 @@ private val TEST_NETWORK_SCORE = NetworkScore.Builder().setLegacyInt(70).build() private class Provider(context: Context, looper: Looper) : NetworkProvider(context, looper, "NetworkAgentTest NetworkProvider") -private val enabledFeatures = mutableMapOf( - FEATURE_QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER to true -) +private val enabledFeatures = mutableMapOf() public open class TestableNetworkAgent( context: Context, @@ -92,7 +91,12 @@ public open class TestableNetworkAgent( override fun isFeatureEnabled(context: Context, feature: Long): Boolean { when (val it = enabledFeatures.get(feature)) { - null -> fail("Unmocked feature $feature, see TestableNetworkAgent.enabledFeatures") + null -> { + val cm = context.getSystemService(ConnectivityManager::class.java) + val res = cm.isFeatureEnabled(feature) + enabledFeatures[feature] = res + return res + } else -> return it } } diff --git a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt index 4c3bce0baf..8f2ac7f6ea 100644 --- a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt +++ b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt @@ -1015,6 +1015,12 @@ class NetworkAgentTest { mock(Network::class.java), mock(INetworkAgentRegistry::class.java) ) + doReturn(SHOULD_CREATE_NETWORKS_IMMEDIATELY).`when`(mockCm) + .isFeatureEnabled( + eq(ConnectivityManager.FEATURE_QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER) + ) + doReturn(Context.CONNECTIVITY_SERVICE).`when`(mockContext) + .getSystemServiceName(ConnectivityManager::class.java) doReturn(mockCm).`when`(mockContext).getSystemService(Context.CONNECTIVITY_SERVICE) doReturn(mockedResult).`when`(mockCm).registerNetworkAgent( any(), @@ -1673,14 +1679,22 @@ class NetworkAgentTest { // The fact that the first event seen by matchAllCallback is the connection of network3 // implicitly ensures that no callbacks are sent since network1 was lost. val (agent3, network3) = connectNetwork(lp = lp) - - // As soon as the replacement arrives, network1 is disconnected. - // Check that this happens before the replacement timeout (5 seconds) fires. - matchAllCallback.expectAvailableCallbacks(network3, validated = false) - matchAllCallback.expect(network1, 2_000 /* timeoutMs */) - matchAllCallback.expectCaps(network3) { it.hasCapability(NET_CAPABILITY_VALIDATED) } - sendAndExpectUdpPacket(network3, reader, iface) - testCallback.expectAvailableDoubleValidatedCallbacks(network3) + if (SHOULD_CREATE_NETWORKS_IMMEDIATELY) { + // This is the correct sequence of events. + matchAllCallback.expectAvailableCallbacks(network3, validated = false) + matchAllCallback.expect(network1, 2_000 /* timeoutMs */) + matchAllCallback.expectCaps(network3) { it.hasCapability(NET_CAPABILITY_VALIDATED) } + sendAndExpectUdpPacket(network3, reader, iface) + testCallback.expectAvailableDoubleValidatedCallbacks(network3) + } else { + // This is incorrect and fixed by the "create networks immediately" feature + matchAllCallback.expectAvailableThenValidatedCallbacks(network3) + testCallback.expectAvailableDoubleValidatedCallbacks(network3) + sendAndExpectUdpPacket(network3, reader, iface) + // As soon as the replacement arrives, network1 is disconnected. + // Check that this happens before the replacement timeout (5 seconds) fires. + matchAllCallback.expect(network1, 2_000 /* timeoutMs */) + } agent1.expectCallback() // Test lingering: @@ -1798,9 +1812,19 @@ class NetworkAgentTest { val (newWifiAgent, newWifiNetwork) = connectNetwork(TRANSPORT_WIFI) testCallback.expectAvailableCallbacks(newWifiNetwork, validated = true) - matchAllCallback.expectAvailableCallbacks(newWifiNetwork, validated = false) - matchAllCallback.expect(wifiNetwork) - matchAllCallback.expectCaps(newWifiNetwork) { it.hasCapability(NET_CAPABILITY_VALIDATED) } + if (SHOULD_CREATE_NETWORKS_IMMEDIATELY) { + // This is the correct sequence of events + matchAllCallback.expectAvailableCallbacks(newWifiNetwork, validated = false) + matchAllCallback.expect(wifiNetwork) + matchAllCallback.expectCaps(newWifiNetwork) { + it.hasCapability(NET_CAPABILITY_VALIDATED) + } + } else { + // When networks are not created immediately, the sequence is slightly incorrect + // and instead is as follows + matchAllCallback.expectAvailableThenValidatedCallbacks(newWifiNetwork) + matchAllCallback.expect(wifiNetwork) + } wifiAgent.expectCallback() testCallback.expect(newWifiNetwork) diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 35a7fbd9c8..67cb1a4110 100755 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -586,6 +586,10 @@ public class ConnectivityServiceTest { "https://android.com/user/api/capport/"; private static final String TEST_FRIENDLY_NAME = "Network friendly name"; private static final String TEST_REDIRECT_URL = "http://example.com/firstPath"; + private static final String QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER = + "queue_network_agent_events_in_system_server"; + + private boolean mShouldCreateNetworksImmediately; private MockContext mServiceContext; private HandlerThread mCsHandlerThread; @@ -1935,6 +1939,9 @@ public class ConnectivityServiceTest { mService.mLingerDelayMs = TEST_LINGER_DELAY_MS; mService.mNascentDelayMs = TEST_NASCENT_DELAY_MS; + mShouldCreateNetworksImmediately = mService.isConnectivityServiceFeatureEnabledForTesting( + QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER); + if (mDeps.isAtLeastV()) { verify(mNetworkPolicyManager, never()).registerNetworkPolicyCallback(any(), any()); mPolicyCallback = null; @@ -3092,23 +3099,43 @@ public class ConnectivityServiceTest { generalCb.expectAvailableCallbacksUnvalidated(net2); if (expectLingering) { generalCb.expectLosing(net1); + } + if (mShouldCreateNetworksImmediately) { + if (expectLingering) { + // Make sure cell 1 is unwanted immediately if the radio can't time share, but only + // after some delay if it can. + generalCb.expectCaps(net2, c -> c.hasCapability(NET_CAPABILITY_VALIDATED)); + defaultCb.expectAvailableDoubleValidatedCallbacks(net2); + net1.assertNotDisconnected(TEST_CALLBACK_TIMEOUT_MS); // always incurs the timeout + generalCb.assertNoCallback(); + // assertNotDisconnected waited for TEST_CALLBACK_TIMEOUT_MS, so waiting for the + // linger period gives TEST_CALLBACK_TIMEOUT_MS time for the event to process. + net1.expectDisconnected(UNREASONABLY_LONG_ALARM_WAIT_MS); + generalCb.expect(LOST, net1); + } else { + net1.expectDisconnected(TEST_CALLBACK_TIMEOUT_MS); + net1.disconnect(); + generalCb.expect(LOST, net1); + generalCb.expectCaps(net2, c -> c.hasCapability(NET_CAPABILITY_VALIDATED)); + defaultCb.expectAvailableDoubleValidatedCallbacks(net2); + } + } else { + generalCb.expectCaps(net2, c -> c.hasCapability(NET_CAPABILITY_VALIDATED)); + defaultCb.expectAvailableDoubleValidatedCallbacks(net2); // Make sure cell 1 is unwanted immediately if the radio can't time share, but only // after some delay if it can. - generalCb.expectCaps(net2, c -> c.hasCapability(NET_CAPABILITY_VALIDATED)); - defaultCb.expectAvailableDoubleValidatedCallbacks(net2); - net1.assertNotDisconnected(TEST_CALLBACK_TIMEOUT_MS); // always incurs the timeout - generalCb.assertNoCallback(); - // assertNotDisconnected waited for TEST_CALLBACK_TIMEOUT_MS, so waiting for the - // linger period gives TEST_CALLBACK_TIMEOUT_MS time for the event to process. - net1.expectDisconnected(UNREASONABLY_LONG_ALARM_WAIT_MS); - generalCb.expect(LOST, net1); - } else { - net1.expectDisconnected(TEST_CALLBACK_TIMEOUT_MS); + if (expectLingering) { + net1.assertNotDisconnected(TEST_CALLBACK_TIMEOUT_MS); // always incurs the timeout + generalCb.assertNoCallback(); + // assertNotDisconnected waited for TEST_CALLBACK_TIMEOUT_MS, so waiting for the + // linger period gives TEST_CALLBACK_TIMEOUT_MS time for the event to process. + net1.expectDisconnected(UNREASONABLY_LONG_ALARM_WAIT_MS); + } else { + net1.expectDisconnected(TEST_CALLBACK_TIMEOUT_MS); + } net1.disconnect(); generalCb.expect(LOST, net1); - generalCb.expectCaps(net2, c -> c.hasCapability(NET_CAPABILITY_VALIDATED)); - defaultCb.expectAvailableDoubleValidatedCallbacks(net2); } // Remove primary from net 2