From 32634e9da3c0a59b14abf9b113c198147e7bb5cb Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Thu, 20 May 2021 13:48:35 +0000 Subject: [PATCH 1/3] No-op refactoring of NetworkAgentTest This is a no-op refactoring of NetworkAgentTest, which makes the create*NetworkAgent helper functions easier to use, this includes: 1. Rename "name" field to specifier, since it is the only purpose of that field. 2. Make the callback generated with agent dedicated to the agent by adding specifier to support multiple agent cases. 3. Refactor some code flow to for readability and less duplicated code. Test: atest CtsNetTestCases:android.net.cts.NetworkAgentTest \ --rerun-until-failure 100 Test: atest CtsNetTestCasesLatestSdk:android.net.cts.NetworkAgentTest on R device Bug: 188657173 Change-Id: Id7948d218b78ae0abf253ca8925e787362ac463f --- .../src/android/net/cts/NetworkAgentTest.kt | 284 +++++++----------- 1 file changed, 115 insertions(+), 169 deletions(-) diff --git a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt index 9017f1b96a..b67acca61d 100644 --- a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt +++ b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt @@ -269,10 +269,9 @@ class NetworkAgentTest { history.add(OnSignalStrengthThresholdsUpdated(thresholds)) } - fun expectEmptySignalStrengths() { + fun expectSignalStrengths(thresholds: IntArray? = intArrayOf()) { expectCallback().let { - // intArrayOf() without arguments makes an empty array - assertArrayEquals(intArrayOf(), it.thresholds) + assertArrayEquals(thresholds, it.thresholds) } } @@ -292,7 +291,7 @@ class NetworkAgentTest { // a NetworkAgent whose network does not require validation (which test networks do // not, since they lack the INTERNET capability). It always contains the default argument // for the URI. - fun expectNoInternetValidationStatus() = expectCallback().let { + fun expectValidationBypassedStatus() = expectCallback().let { assertEquals(it.status, VALID_NETWORK) // The returned Uri is parsed from the empty string, which means it's an // instance of the (private) Uri.StringUri. There are no real good ways @@ -332,9 +331,21 @@ class NetworkAgentTest { callbacksToCleanUp.add(callback) } + private fun makeTestNetworkRequest(specifier: String? = null): NetworkRequest { + return NetworkRequest.Builder() + .clearCapabilities() + .addTransportType(TRANSPORT_TEST) + .also { + if (specifier != null) { + it.setNetworkSpecifier(CompatUtil.makeEthernetNetworkSpecifier(specifier)) + } + } + .build() + } + private fun createNetworkAgent( context: Context = realContext, - name: String? = null, + specifier: String? = null, initialNc: NetworkCapabilities? = null, initialLp: LinkProperties? = null, initialConfig: NetworkAgentConfig? = null @@ -349,8 +360,8 @@ class NetworkAgentTest { if (SdkLevel.isAtLeastS()) { addCapability(NET_CAPABILITY_NOT_VCN_MANAGED) } - if (null != name) { - setNetworkSpecifier(CompatUtil.makeEthernetNetworkSpecifier(name)) + if (null != specifier) { + setNetworkSpecifier(CompatUtil.makeEthernetNetworkSpecifier(specifier)) } } val lp = initialLp ?: LinkProperties().apply { @@ -365,21 +376,22 @@ class NetworkAgentTest { private fun createConnectedNetworkAgent( context: Context = realContext, - name: String? = null, - initialConfig: NetworkAgentConfig? = null + specifier: String? = UUID.randomUUID().toString(), + initialConfig: NetworkAgentConfig? = null, + expectedInitSignalStrengthThresholds: IntArray? = intArrayOf() ): Pair { - val request: NetworkRequest = NetworkRequest.Builder() - .clearCapabilities() - .addTransportType(TRANSPORT_TEST) - .build() val callback = TestableNetworkCallback(timeoutMs = DEFAULT_TIMEOUT_MS) - requestNetwork(request, callback) - val config = initialConfig ?: NetworkAgentConfig.Builder().build() - val agent = createNetworkAgent(context, name, initialConfig = config) + // Ensure this NetworkAgent is never unneeded by filing a request with its specifier. + requestNetwork(makeTestNetworkRequest(specifier = specifier), callback) + val agent = createNetworkAgent(context, specifier, initialConfig = initialConfig) agent.setTeardownDelayMillis(0) + // Connect the agent and verify initial status callbacks. agent.register() agent.markConnected() agent.expectCallback() + agent.expectSignalStrengths(expectedInitSignalStrengthThresholds) + agent.expectValidationBypassedStatus() + callback.expectAvailableThenValidatedCallbacks(agent.network!!) return agent to callback } @@ -413,7 +425,6 @@ class NetworkAgentTest { .setLegacySubType(subtypeLTE) .setLegacySubTypeName(subtypeNameLTE).build() val (agent, callback) = createConnectedNetworkAgent(initialConfig = config) - callback.expectAvailableThenValidatedCallbacks(agent.network) agent.setLegacySubtype(subtypeUMTS, subtypeNameUMTS) // There is no callback when networkInfo changes, @@ -433,12 +444,8 @@ class NetworkAgentTest { @Test fun testConnectAndUnregister() { val (agent, callback) = createConnectedNetworkAgent() - callback.expectAvailableThenValidatedCallbacks(agent.network) - agent.expectEmptySignalStrengths() - agent.expectNoInternetValidationStatus() - unregister(agent) - callback.expectCallback(agent.network) + callback.expectCallback(agent.network!!) assertFailsWith("Must not be able to register an agent twice") { agent.register() } @@ -446,11 +453,8 @@ class NetworkAgentTest { @Test fun testOnBandwidthUpdateRequested() { - val (agent, callback) = createConnectedNetworkAgent() - callback.expectAvailableThenValidatedCallbacks(agent.network) - agent.expectEmptySignalStrengths() - agent.expectNoInternetValidationStatus() - mCM.requestBandwidthUpdate(agent.network) + val (agent, _) = createConnectedNetworkAgent() + mCM.requestBandwidthUpdate(agent.network!!) agent.expectCallback() unregister(agent) } @@ -468,13 +472,8 @@ class NetworkAgentTest { registerNetworkCallback(request, it) } } - createConnectedNetworkAgent().let { (agent, callback) -> - callback.expectAvailableThenValidatedCallbacks(agent.network) - agent.expectCallback().let { - assertArrayEquals(it.thresholds, thresholds) - } - agent.expectNoInternetValidationStatus() - + createConnectedNetworkAgent(expectedInitSignalStrengthThresholds = thresholds).let { + (agent, callback) -> // Send signal strength and check that the callbacks are called appropriately. val nc = NetworkCapabilities(agent.nc) nc.setSignalStrength(20) @@ -483,21 +482,21 @@ class NetworkAgentTest { nc.setSignalStrength(40) agent.sendNetworkCapabilities(nc) - callbacks[0].expectAvailableCallbacks(agent.network) + callbacks[0].expectAvailableCallbacks(agent.network!!) callbacks[1].assertNoCallback(NO_CALLBACK_TIMEOUT) callbacks[2].assertNoCallback(NO_CALLBACK_TIMEOUT) nc.setSignalStrength(80) agent.sendNetworkCapabilities(nc) - callbacks[0].expectCapabilitiesThat(agent.network) { it.signalStrength == 80 } - callbacks[1].expectAvailableCallbacks(agent.network) - callbacks[2].expectAvailableCallbacks(agent.network) + callbacks[0].expectCapabilitiesThat(agent.network!!) { it.signalStrength == 80 } + callbacks[1].expectAvailableCallbacks(agent.network!!) + callbacks[2].expectAvailableCallbacks(agent.network!!) nc.setSignalStrength(55) agent.sendNetworkCapabilities(nc) - callbacks[0].expectCapabilitiesThat(agent.network) { it.signalStrength == 55 } - callbacks[1].expectCapabilitiesThat(agent.network) { it.signalStrength == 55 } - callbacks[2].expectCallback(agent.network) + callbacks[0].expectCapabilitiesThat(agent.network!!) { it.signalStrength == 55 } + callbacks[1].expectCapabilitiesThat(agent.network!!) { it.signalStrength == 55 } + callbacks[2].expectCallback(agent.network!!) } callbacks.forEach { mCM.unregisterNetworkCallback(it) @@ -546,20 +545,17 @@ class NetworkAgentTest { @Test fun testSendUpdates(): Unit = createConnectedNetworkAgent().let { (agent, callback) -> - callback.expectAvailableThenValidatedCallbacks(agent.network) - agent.expectEmptySignalStrengths() - agent.expectNoInternetValidationStatus() val ifaceName = "adhocIface" val lp = LinkProperties(agent.lp) lp.setInterfaceName(ifaceName) agent.sendLinkProperties(lp) - callback.expectLinkPropertiesThat(agent.network) { + callback.expectLinkPropertiesThat(agent.network!!) { it.getInterfaceName() == ifaceName } val nc = NetworkCapabilities(agent.nc) nc.addCapability(NET_CAPABILITY_NOT_METERED) agent.sendNetworkCapabilities(nc) - callback.expectCapabilitiesThat(agent.network) { + callback.expectCapabilitiesThat(agent.network!!) { it.hasCapability(NET_CAPABILITY_NOT_METERED) } } @@ -568,56 +564,32 @@ class NetworkAgentTest { fun testSendScore() { // This test will create two networks and check that the one with the stronger // score wins out for a request that matches them both. - // First create requests to make sure both networks are kept up, using the - // specifier so they are specific to each network - val name1 = UUID.randomUUID().toString() - val name2 = UUID.randomUUID().toString() - val request1 = NetworkRequest.Builder() - .clearCapabilities() - .addTransportType(TRANSPORT_TEST) - .setNetworkSpecifier(CompatUtil.makeEthernetNetworkSpecifier(name1)) - .build() - val request2 = NetworkRequest.Builder() - .clearCapabilities() - .addTransportType(TRANSPORT_TEST) - .setNetworkSpecifier(CompatUtil.makeEthernetNetworkSpecifier(name2)) - .build() - val callback1 = TestableNetworkCallback(timeoutMs = DEFAULT_TIMEOUT_MS) - val callback2 = TestableNetworkCallback(timeoutMs = DEFAULT_TIMEOUT_MS) - requestNetwork(request1, callback1) - requestNetwork(request2, callback2) - // Then file the interesting request - val request = NetworkRequest.Builder() - .clearCapabilities() - .addTransportType(TRANSPORT_TEST) - .build() + // File the interesting request val callback = TestableNetworkCallback(timeoutMs = DEFAULT_TIMEOUT_MS) - requestNetwork(request, callback) + requestNetwork(makeTestNetworkRequest(), callback) - // Connect the first Network - createConnectedNetworkAgent(name = name1).let { (agent1, _) -> - callback.expectAvailableThenValidatedCallbacks(agent1.network) - // If using the int ranking, agent1 must be upgraded to a better score so that there is - // no ambiguity when agent2 connects that agent1 is still better. If using policy - // ranking, this is not necessary. - agent1.sendNetworkScore(NetworkScore.Builder().setLegacyInt(BETTER_NETWORK_SCORE) - .build()) - // Connect the second agent - createConnectedNetworkAgent(name = name2).let { (agent2, _) -> - agent2.markConnected() - // The callback should not see anything yet. With int ranking, agent1 was upgraded - // to a stronger score beforehand. With policy ranking, agent1 is preferred by - // virtue of already satisfying the request. - callback.assertNoCallback(NO_CALLBACK_TIMEOUT) - // Now downgrade the score and expect the callback now prefers agent2 - agent1.sendNetworkScore(NetworkScore.Builder() - .setLegacyInt(WORSE_NETWORK_SCORE) - .setExiting(true) - .build()) - callback.expectCallback(agent2.network) - } - } + // Connect the first Network, with an unused callback that kept the network up. + val (agent1, _) = createConnectedNetworkAgent() + callback.expectAvailableThenValidatedCallbacks(agent1.network!!) + // If using the int ranking, agent1 must be upgraded to a better score so that there is + // no ambiguity when agent2 connects that agent1 is still better. If using policy + // ranking, this is not necessary. + agent1.sendNetworkScore(NetworkScore.Builder().setLegacyInt(BETTER_NETWORK_SCORE) + .build()) + + // Connect the second agent. + val (agent2, _) = createConnectedNetworkAgent() + // The callback should not see anything yet. With int ranking, agent1 was upgraded + // to a stronger score beforehand. With policy ranking, agent1 is preferred by + // virtue of already satisfying the request. + callback.assertNoCallback(NO_CALLBACK_TIMEOUT) + // Now downgrade the score and expect the callback now prefers agent2 + agent1.sendNetworkScore(NetworkScore.Builder() + .setLegacyInt(WORSE_NETWORK_SCORE) + .setExiting(true) + .build()) + callback.expectCallback(agent2.network!!) // tearDown() will unregister the requests and agents } @@ -658,7 +630,7 @@ class NetworkAgentTest { callback.expectAvailableThenValidatedCallbacks(agent.network!!) // Check that the default network's transport is propagated to the VPN. - var vpnNc = mCM.getNetworkCapabilities(agent.network) + var vpnNc = mCM.getNetworkCapabilities(agent.network!!) assertNotNull(vpnNc) assertEquals(VpnManager.TYPE_VPN_SERVICE, (vpnNc.transportInfo as VpnTransportInfo).type) @@ -690,7 +662,7 @@ class NetworkAgentTest { // This is not very accurate because the test does not control the capabilities of the // underlying networks, and because not congested, not roaming, and not suspended are the // default anyway. It's still useful as an extra check though. - vpnNc = mCM.getNetworkCapabilities(agent.network) + vpnNc = mCM.getNetworkCapabilities(agent.network!!) for (cap in listOf(NET_CAPABILITY_NOT_CONGESTED, NET_CAPABILITY_NOT_ROAMING, NET_CAPABILITY_NOT_SUSPENDED)) { @@ -701,7 +673,7 @@ class NetworkAgentTest { } unregister(agent) - callback.expectCallback(agent.network) + callback.expectCallback(agent.network!!) } private fun unregister(agent: TestableNetworkAgent) { @@ -789,43 +761,24 @@ class NetworkAgentTest { fun testTemporarilyUnmeteredCapability() { // This test will create a networks with/without NET_CAPABILITY_TEMPORARILY_NOT_METERED // and check that the callback reflects the capability changes. - // First create a request to make sure the network is kept up - val request1 = NetworkRequest.Builder() - .clearCapabilities() - .addTransportType(TRANSPORT_TEST) - .build() - val callback1 = TestableNetworkCallback(timeoutMs = DEFAULT_TIMEOUT_MS).also { - registerNetworkCallback(request1, it) - } - requestNetwork(request1, callback1) - - // Then file the interesting request - val request = NetworkRequest.Builder() - .clearCapabilities() - .addTransportType(TRANSPORT_TEST) - .build() - val callback = TestableNetworkCallback(timeoutMs = DEFAULT_TIMEOUT_MS) - requestNetwork(request, callback) // Connect the network - createConnectedNetworkAgent().let { (agent, _) -> - callback.expectAvailableThenValidatedCallbacks(agent.network) + val (agent, callback) = createConnectedNetworkAgent() - // Send TEMP_NOT_METERED and check that the callback is called appropriately. - val nc1 = NetworkCapabilities(agent.nc) - .addCapability(NET_CAPABILITY_TEMPORARILY_NOT_METERED) - agent.sendNetworkCapabilities(nc1) - callback.expectCapabilitiesThat(agent.network) { - it.hasCapability(NET_CAPABILITY_TEMPORARILY_NOT_METERED) - } + // Send TEMP_NOT_METERED and check that the callback is called appropriately. + val nc1 = NetworkCapabilities(agent.nc) + .addCapability(NET_CAPABILITY_TEMPORARILY_NOT_METERED) + agent.sendNetworkCapabilities(nc1) + callback.expectCapabilitiesThat(agent.network!!) { + it.hasCapability(NET_CAPABILITY_TEMPORARILY_NOT_METERED) + } - // Remove TEMP_NOT_METERED and check that the callback is called appropriately. - val nc2 = NetworkCapabilities(agent.nc) - .removeCapability(NET_CAPABILITY_TEMPORARILY_NOT_METERED) - agent.sendNetworkCapabilities(nc2) - callback.expectCapabilitiesThat(agent.network) { - !it.hasCapability(NET_CAPABILITY_TEMPORARILY_NOT_METERED) - } + // Remove TEMP_NOT_METERED and check that the callback is called appropriately. + val nc2 = NetworkCapabilities(agent.nc) + .removeCapability(NET_CAPABILITY_TEMPORARILY_NOT_METERED) + agent.sendNetworkCapabilities(nc2) + callback.expectCapabilitiesThat(agent.network!!) { + !it.hasCapability(NET_CAPABILITY_TEMPORARILY_NOT_METERED) } // tearDown() will unregister the requests and agents @@ -838,85 +791,78 @@ class NetworkAgentTest { // score wins out for a request that matches them both. And the weaker agent will // be disconnected after customized linger duration. - // Connect the first Network - val name1 = UUID.randomUUID().toString() - val name2 = UUID.randomUUID().toString() - val (agent1, callback) = createConnectedNetworkAgent(name = name1) - callback.expectAvailableThenValidatedCallbacks(agent1.network!!) - // Downgrade agent1 to a worse score so that there is no ambiguity when - // agent2 connects. - agent1.sendNetworkScore(NetworkScore.Builder().setLegacyInt(WORSE_NETWORK_SCORE) + // Request the first Network, with a request that could moved to agentStronger in order to + // make agentWeaker linger later. + val specifierWeaker = UUID.randomUUID().toString() + val specifierStronger = UUID.randomUUID().toString() + val commonCallback = TestableNetworkCallback(timeoutMs = DEFAULT_TIMEOUT_MS) + requestNetwork(makeTestNetworkRequest(), commonCallback) + val agentWeaker = createNetworkAgent(specifier = specifierWeaker) + agentWeaker.register() + agentWeaker.markConnected() + commonCallback.expectAvailableThenValidatedCallbacks(agentWeaker.network!!) + // Downgrade agentWeaker to a worse score so that there is no ambiguity when + // agentStronger connects. + agentWeaker.sendNetworkScore(NetworkScore.Builder().setLegacyInt(WORSE_NETWORK_SCORE) .setExiting(true).build()) // Verify invalid linger duration cannot be set. assertFailsWith { - agent1.setLingerDuration(Duration.ofMillis(-1)) + agentWeaker.setLingerDuration(Duration.ofMillis(-1)) } - assertFailsWith { agent1.setLingerDuration(Duration.ZERO) } + assertFailsWith { agentWeaker.setLingerDuration(Duration.ZERO) } assertFailsWith { - agent1.setLingerDuration(Duration.ofMillis(Integer.MIN_VALUE.toLong())) + agentWeaker.setLingerDuration(Duration.ofMillis(Integer.MIN_VALUE.toLong())) } assertFailsWith { - agent1.setLingerDuration(Duration.ofMillis(Integer.MAX_VALUE.toLong() + 1)) + agentWeaker.setLingerDuration(Duration.ofMillis(Integer.MAX_VALUE.toLong() + 1)) } assertFailsWith { - agent1.setLingerDuration(Duration.ofMillis( + agentWeaker.setLingerDuration(Duration.ofMillis( NetworkAgent.MIN_LINGER_TIMER_MS.toLong() - 1)) } // Verify valid linger timer can be set, but it should not take effect since the network // is still needed. - agent1.setLingerDuration(Duration.ofMillis(Integer.MAX_VALUE.toLong())) - callback.assertNoCallback(NO_CALLBACK_TIMEOUT) + agentWeaker.setLingerDuration(Duration.ofMillis(Integer.MAX_VALUE.toLong())) + commonCallback.assertNoCallback(NO_CALLBACK_TIMEOUT) // Set to the value we want to verify the functionality. - agent1.setLingerDuration(Duration.ofMillis(NetworkAgent.MIN_LINGER_TIMER_MS.toLong())) - // Make a listener which can observe agent1 lost later. + agentWeaker.setLingerDuration(Duration.ofMillis(NetworkAgent.MIN_LINGER_TIMER_MS.toLong())) + // Make a listener which can observe agentWeaker lost later. val callbackWeaker = TestableNetworkCallback(timeoutMs = DEFAULT_TIMEOUT_MS) registerNetworkCallback(NetworkRequest.Builder() .clearCapabilities() .addTransportType(TRANSPORT_TEST) - .setNetworkSpecifier(CompatUtil.makeEthernetNetworkSpecifier(name1)) + .setNetworkSpecifier(CompatUtil.makeEthernetNetworkSpecifier(specifierWeaker)) .build(), callbackWeaker) - callbackWeaker.expectAvailableCallbacks(agent1.network!!) + callbackWeaker.expectAvailableCallbacks(agentWeaker.network!!) - // Connect the second agent with a score better than agent1. Verify the callback for - // agent1 sees the linger expiry while the callback for both sees the winner. + // Connect the agentStronger with a score better than agentWeaker. Verify the callback for + // agentWeaker sees the linger expiry while the callback for both sees the winner. // Record linger start timestamp prior to send score to prevent possible race, the actual // timestamp should be slightly late than this since the service handles update // network score asynchronously. val lingerStart = SystemClock.elapsedRealtime() - val agent2 = createNetworkAgent(name = name2) - agent2.register() - agent2.markConnected() - callback.expectAvailableCallbacks(agent2.network!!) - callbackWeaker.expectCallback(agent1.network!!) + val agentStronger = createNetworkAgent(specifier = specifierStronger) + agentStronger.register() + agentStronger.markConnected() + commonCallback.expectAvailableCallbacks(agentStronger.network!!) + callbackWeaker.expectCallback(agentWeaker.network!!) val expectedRemainingLingerDuration = lingerStart + NetworkAgent.MIN_LINGER_TIMER_MS.toLong() - SystemClock.elapsedRealtime() // If the available callback is too late. The remaining duration will be reduced. assertTrue(expectedRemainingLingerDuration > 0, "expected remaining linger duration is $expectedRemainingLingerDuration") callbackWeaker.assertNoCallback(expectedRemainingLingerDuration) - callbackWeaker.expectCallback(agent1.network!!) + callbackWeaker.expectCallback(agentWeaker.network!!) } @Test @IgnoreUpTo(Build.VERSION_CODES.R) fun testSetSubscriberId() { - val name = "TEST-AGENT" val imsi = UUID.randomUUID().toString() val config = NetworkAgentConfig.Builder().setSubscriberId(imsi).build() - val request: NetworkRequest = NetworkRequest.Builder() - .clearCapabilities() - .addTransportType(TRANSPORT_TEST) - .setNetworkSpecifier(CompatUtil.makeEthernetNetworkSpecifier(name)) - .build() - val callback = TestableNetworkCallback(timeoutMs = DEFAULT_TIMEOUT_MS) - requestNetwork(request, callback) - - val agent = createNetworkAgent(name = name, initialConfig = config) - agent.register() - agent.markConnected() - callback.expectAvailableThenValidatedCallbacks(agent.network!!) + val (agent, _) = createConnectedNetworkAgent(initialConfig = config) val snapshots = runWithShellPermissionIdentity(ThrowingSupplier { mCM!!.allNetworkStateSnapshots }, NETWORK_SETTINGS) val testNetworkSnapshot = snapshots.findLast { it.network == agent.network } From eb5451dd3542aeb7456dd02918620ec7ed63fe80 Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Fri, 21 May 2021 02:56:31 +0000 Subject: [PATCH 2/3] Add CTS for registerBestMatchingNetworkCallback Bug: 188657173 Test: atest CtsNetTestCases:android.net.cts.NetworkAgentTest \ CtsNetTestCases:android.net.cts.ConnectivityManagerTest Test: atest CtsNetTestCasesLatestSdk:android.net.cts.NetworkAgentTest \ CtsNetTestCasesLatestSdk:android.net.cts.ConnectivityManagerTest on R device Change-Id: I81abc1742a3aa965b444d0196f4eaa2393dcad1c --- .../net/cts/ConnectivityManagerTest.java | 19 ++++++ .../src/android/net/cts/NetworkAgentTest.kt | 66 +++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java index 0075b54716..582c66363c 100644 --- a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java +++ b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java @@ -648,6 +648,18 @@ public class ConnectivityManagerTest { mCm.getBackgroundDataSetting(); } + private NetworkRequest makeDefaultRequest() { + // Make a request that is similar to the way framework tracks the system + // default network. + return new NetworkRequest.Builder() + .clearCapabilities() + .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED) + .addCapability(NetworkCapabilities.NET_CAPABILITY_TRUSTED) + .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN) + .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) + .build(); + } + private NetworkRequest makeWifiNetworkRequest() { return new NetworkRequest.Builder() .addTransportType(NetworkCapabilities.TRANSPORT_WIFI) @@ -686,12 +698,14 @@ public class ConnectivityManagerTest { final TestNetworkCallback systemDefaultCallback = new TestNetworkCallback(); final TestNetworkCallback perUidCallback = new TestNetworkCallback(); + final TestNetworkCallback bestMatchingCallback = new TestNetworkCallback(); final Handler h = new Handler(Looper.getMainLooper()); if (TestUtils.shouldTestSApis()) { runWithShellPermissionIdentity(() -> { mCmShim.registerSystemDefaultNetworkCallback(systemDefaultCallback, h); mCmShim.registerDefaultNetworkCallbackForUid(Process.myUid(), perUidCallback, h); }, NETWORK_SETTINGS); + mCm.registerBestMatchingNetworkCallback(makeDefaultRequest(), bestMatchingCallback, h); } Network wifiNetwork = null; @@ -717,6 +731,10 @@ public class ConnectivityManagerTest { assertNotNull("Did not receive onAvailable on per-UID default network callback", perUidNetwork); assertEquals(defaultNetwork, perUidNetwork); + final Network bestMatchingNetwork = bestMatchingCallback.waitForAvailable(); + assertNotNull("Did not receive onAvailable on best matching network callback", + bestMatchingNetwork); + assertEquals(defaultNetwork, bestMatchingNetwork); } } catch (InterruptedException e) { @@ -727,6 +745,7 @@ public class ConnectivityManagerTest { if (TestUtils.shouldTestSApis()) { mCm.unregisterNetworkCallback(systemDefaultCallback); mCm.unregisterNetworkCallback(perUidCallback); + mCm.unregisterNetworkCallback(bestMatchingCallback); } } } diff --git a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt index b67acca61d..c505cefef9 100644 --- a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt +++ b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt @@ -66,6 +66,7 @@ import android.net.cts.NetworkAgentTest.TestableNetworkAgent.CallbackEntry.OnSta import android.net.cts.NetworkAgentTest.TestableNetworkAgent.CallbackEntry.OnStopSocketKeepalive import android.net.cts.NetworkAgentTest.TestableNetworkAgent.CallbackEntry.OnValidationStatus import android.os.Build +import android.os.Handler import android.os.HandlerThread import android.os.Looper import android.os.Message @@ -331,6 +332,15 @@ class NetworkAgentTest { callbacksToCleanUp.add(callback) } + private fun registerBestMatchingNetworkCallback( + request: NetworkRequest, + callback: TestableNetworkCallback, + handler: Handler + ) { + mCM!!.registerBestMatchingNetworkCallback(request, callback, handler) + callbacksToCleanUp.add(callback) + } + private fun makeTestNetworkRequest(specifier: String? = null): NetworkRequest { return NetworkRequest.Builder() .clearCapabilities() @@ -868,4 +878,60 @@ class NetworkAgentTest { val testNetworkSnapshot = snapshots.findLast { it.network == agent.network } assertEquals(imsi, testNetworkSnapshot!!.subscriberId) } + + @Test + @IgnoreUpTo(Build.VERSION_CODES.R) + // TODO: Refactor helper functions to util class and move this test case to + // {@link android.net.cts.ConnectivityManagerTest}. + fun testRegisterBestMatchingNetworkCallback() { + // Register best matching network callback with additional condition that will be + // exercised later. This assumes the test network agent has NOT_VCN_MANAGED in it and + // does not have NET_CAPABILITY_TEMPORARILY_NOT_METERED. + val bestMatchingCb = TestableNetworkCallback(timeoutMs = DEFAULT_TIMEOUT_MS) + registerBestMatchingNetworkCallback(NetworkRequest.Builder() + .clearCapabilities() + .addTransportType(TRANSPORT_TEST) + .addCapability(NET_CAPABILITY_NOT_VCN_MANAGED) + .build(), bestMatchingCb, mHandlerThread.threadHandler) + + val (agent1, _) = createConnectedNetworkAgent(specifier = "AGENT-1") + bestMatchingCb.expectAvailableThenValidatedCallbacks(agent1.network!!) + // Make agent1 worse so when agent2 shows up, the callback will see that. + agent1.sendNetworkScore(NetworkScore.Builder().setExiting(true).build()) + bestMatchingCb.assertNoCallback(NO_CALLBACK_TIMEOUT) + + val (agent2, _) = createConnectedNetworkAgent(specifier = "AGENT-2") + bestMatchingCb.expectAvailableDoubleValidatedCallbacks(agent2.network!!) + + // Change something on agent1 to trigger capabilities changed, since the callback + // only cares about the best network, verify it received nothing from agent1. + val ncAgent1 = agent1.nc + ncAgent1.addCapability(NET_CAPABILITY_TEMPORARILY_NOT_METERED) + agent1.sendNetworkCapabilities(ncAgent1) + bestMatchingCb.assertNoCallback(NO_CALLBACK_TIMEOUT) + + // Make agent1 the best network again, verify the callback now tracks agent1. + agent1.sendNetworkScore(NetworkScore.Builder() + .setExiting(false).setTransportPrimary(true).build()) + bestMatchingCb.expectAvailableCallbacks(agent1.network!!) + + // Make agent1 temporary vcn managed, which will not satisfying the request. + // Verify the callback switch from/to the other network accordingly. + ncAgent1.removeCapability(NET_CAPABILITY_NOT_VCN_MANAGED) + agent1.sendNetworkCapabilities(ncAgent1) + bestMatchingCb.expectAvailableCallbacks(agent2.network!!) + ncAgent1.addCapability(NET_CAPABILITY_NOT_VCN_MANAGED) + agent1.sendNetworkCapabilities(ncAgent1) + bestMatchingCb.expectAvailableDoubleValidatedCallbacks(agent1.network!!) + + // Verify the callback doesn't care about agent2 disconnect. + agent2.unregister() + agentsToCleanUp.remove(agent2) + bestMatchingCb.assertNoCallback() + agent1.unregister() + agentsToCleanUp.remove(agent1) + bestMatchingCb.expectCallback(agent1.network!!) + + // tearDown() will unregister the requests and agents + } } From 63e3dedfee29985094883aac40c11979381aa8c8 Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Mon, 24 May 2021 13:04:52 +0000 Subject: [PATCH 3/3] Fix expectCallback does not fail when no callback received This hides 3 bugs where, 1. In Android S, onNetworkRequested is no longer broadcasted by ConnectivityService in any condition. However, the test still passes since assertion does not fail when no callback received. Ignore the test on S+ devices since the behavior changed on S or later devices. 2. Test network agent is not registered, but the test still passes. 3. Test network agent does not fulfill the request that kept the network up, so the test fails since the test network is not needed and be torn down. Test: android.net.NetworkProviderTest on R/S device Bug: 189074532 Change-Id: I627dcd0f57b6ef4197d16e6c1ec0c53e675ab055 --- .../java/android/net/NetworkProviderTest.kt | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/tests/common/java/android/net/NetworkProviderTest.kt b/tests/common/java/android/net/NetworkProviderTest.kt index 7424157bea..97d3c5a802 100644 --- a/tests/common/java/android/net/NetworkProviderTest.kt +++ b/tests/common/java/android/net/NetworkProviderTest.kt @@ -18,6 +18,7 @@ package android.net import android.app.Instrumentation import android.content.Context +import android.net.NetworkCapabilities.NET_CAPABILITY_TRUSTED import android.net.NetworkCapabilities.TRANSPORT_TEST import android.net.NetworkProviderTest.TestNetworkCallback.CallbackEntry.OnUnavailable import android.net.NetworkProviderTest.TestNetworkProvider.CallbackEntry.OnNetworkRequestWithdrawn @@ -25,14 +26,18 @@ import android.net.NetworkProviderTest.TestNetworkProvider.CallbackEntry.OnNetwo import android.os.Build import android.os.HandlerThread import android.os.Looper +import android.util.Log import androidx.test.InstrumentationRegistry import com.android.net.module.util.ArrayTrackRecord import com.android.testutils.CompatUtil +import com.android.testutils.DevSdkIgnoreRule +import com.android.testutils.DevSdkIgnoreRule.IgnoreAfter import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo import com.android.testutils.DevSdkIgnoreRunner import com.android.testutils.isDevSdkInRange import org.junit.After import org.junit.Before +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mockito.doReturn @@ -41,6 +46,7 @@ import org.mockito.Mockito.verifyNoMoreInteractions import java.util.UUID import kotlin.test.assertEquals import kotlin.test.assertNotEquals +import kotlin.test.fail private const val DEFAULT_TIMEOUT_MS = 5000L private val instrumentation: Instrumentation @@ -51,6 +57,8 @@ private val PROVIDER_NAME = "NetworkProviderTest" @RunWith(DevSdkIgnoreRunner::class) @IgnoreUpTo(Build.VERSION_CODES.Q) class NetworkProviderTest { + @Rule @JvmField + val mIgnoreRule = DevSdkIgnoreRule() private val mCm = context.getSystemService(ConnectivityManager::class.java) private val mHandlerThread = HandlerThread("${javaClass.simpleName} handler thread") @@ -68,6 +76,7 @@ class NetworkProviderTest { private class TestNetworkProvider(context: Context, looper: Looper) : NetworkProvider(context, looper, PROVIDER_NAME) { + private val TAG = this::class.simpleName private val seenEvents = ArrayTrackRecord().newReadHead() sealed class CallbackEntry { @@ -80,22 +89,30 @@ class NetworkProviderTest { } override fun onNetworkRequested(request: NetworkRequest, score: Int, id: Int) { + Log.d(TAG, "onNetworkRequested $request, $score, $id") seenEvents.add(OnNetworkRequested(request, score, id)) } override fun onNetworkRequestWithdrawn(request: NetworkRequest) { + Log.d(TAG, "onNetworkRequestWithdrawn $request") seenEvents.add(OnNetworkRequestWithdrawn(request)) } - inline fun expectCallback( + inline fun eventuallyExpectCallbackThat( crossinline predicate: (T) -> Boolean ) = seenEvents.poll(DEFAULT_TIMEOUT_MS) { it is T && predicate(it) } + ?: fail("Did not receive callback after ${DEFAULT_TIMEOUT_MS}ms") } private fun createNetworkProvider(ctx: Context = context): TestNetworkProvider { return TestNetworkProvider(ctx, mHandlerThread.looper) } + // In S+ framework, do not run this test, since the provider will no longer receive + // onNetworkRequested for every request. Instead, provider needs to + // call {@code registerNetworkOffer} with the description of networks they + // might have ability to setup, and expects {@link NetworkOfferCallback#onNetworkNeeded}. + @IgnoreAfter(Build.VERSION_CODES.R) @Test fun testOnNetworkRequested() { val provider = createNetworkProvider() @@ -105,13 +122,15 @@ class NetworkProviderTest { val specifier = CompatUtil.makeTestNetworkSpecifier( UUID.randomUUID().toString()) + // Test network is not allowed to be trusted. val nr: NetworkRequest = NetworkRequest.Builder() .addTransportType(TRANSPORT_TEST) + .removeCapability(NET_CAPABILITY_TRUSTED) .setNetworkSpecifier(specifier) .build() val cb = ConnectivityManager.NetworkCallback() mCm.requestNetwork(nr, cb) - provider.expectCallback() { callback -> + provider.eventuallyExpectCallbackThat() { callback -> callback.request.getNetworkSpecifier() == specifier && callback.request.hasTransport(TRANSPORT_TEST) } @@ -131,22 +150,24 @@ class NetworkProviderTest { val config = NetworkAgentConfig.Builder().build() val agent = object : NetworkAgent(context, mHandlerThread.looper, "TestAgent", nc, lp, initialScore, config, provider) {} + agent.register() + agent.markConnected() - provider.expectCallback() { callback -> + provider.eventuallyExpectCallbackThat() { callback -> callback.request.getNetworkSpecifier() == specifier && callback.score == initialScore && callback.id == agent.providerId } agent.sendNetworkScore(updatedScore) - provider.expectCallback() { callback -> + provider.eventuallyExpectCallbackThat() { callback -> callback.request.getNetworkSpecifier() == specifier && callback.score == updatedScore && callback.id == agent.providerId } mCm.unregisterNetworkCallback(cb) - provider.expectCallback() { callback -> + provider.eventuallyExpectCallbackThat() { callback -> callback.request.getNetworkSpecifier() == specifier && callback.request.hasTransport(TRANSPORT_TEST) }