From 816f0af86ab6bebe5caf6c904a9beb6d23ac11c0 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 13 Jan 2023 22:09:13 +0900 Subject: [PATCH] Remove expectLinkPropertiesThat expect() is simply better Bug: 157405399 Test: ConnectivityServiceTest NetworkStaticLibTests Change-Id: I5bf109ccf34f1fa5ce2f4ed78300d5a126778e30 --- .../src/android/net/cts/NetworkAgentTest.kt | 4 +- .../ConnectivityServiceIntegrationTest.kt | 7 +- .../server/ConnectivityServiceTest.java | 125 ++++++++++-------- 3 files changed, 71 insertions(+), 65 deletions(-) diff --git a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt index 7ae46881ba..f578ff3014 100644 --- a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt +++ b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt @@ -506,9 +506,7 @@ class NetworkAgentTest { val lp = LinkProperties(agent.lp) lp.setInterfaceName(ifaceName) agent.sendLinkProperties(lp) - callback.expectLinkPropertiesThat(agent.network!!) { - it.getInterfaceName() == ifaceName - } + callback.expect(agent.network!!) { it.lp.interfaceName == ifaceName } val nc = NetworkCapabilities(agent.nc) nc.addCapability(NET_CAPABILITY_NOT_METERED) agent.sendNetworkCapabilities(nc) diff --git a/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt b/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt index cf3f3758de..3c1340d06b 100644 --- a/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt +++ b/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt @@ -53,6 +53,7 @@ import com.android.server.TestNetIdManager import com.android.server.connectivity.MockableSystemProperties import com.android.server.connectivity.MultinetworkPolicyTracker import com.android.server.connectivity.ProxyTracker +import com.android.testutils.RecorderCallback.CallbackEntry.LinkPropertiesChanged import com.android.testutils.TestableNetworkCallback import org.junit.After import org.junit.Before @@ -73,7 +74,6 @@ import org.mockito.Mockito.spy import org.mockito.MockitoAnnotations import org.mockito.Spy import kotlin.test.assertEquals -import kotlin.test.assertNotNull import kotlin.test.assertTrue import kotlin.test.fail @@ -288,10 +288,9 @@ class ConnectivityServiceIntegrationTest { testCb.expectAvailableCallbacks(na.network, validated = false, tmt = TEST_TIMEOUT_MS) - val capportData = testCb.expectLinkPropertiesThat(na, TEST_TIMEOUT_MS) { - it.captivePortalData != null + val capportData = testCb.expect(na, TEST_TIMEOUT_MS) { + it.lp.captivePortalData != null }.lp.captivePortalData - assertNotNull(capportData) assertTrue(capportData.isCaptive) assertEquals(Uri.parse("https://login.capport.android.com"), capportData.userPortalUrl) assertEquals(Uri.parse("https://venueinfo.capport.android.com"), capportData.venueInfoUrl) diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index a1186a3bc4..668d4cd914 100755 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -3224,17 +3224,17 @@ public class ConnectivityServiceTest { final Uri expectedCapportUrl = sanitized ? null : capportUrl; newLp.setCaptivePortalApiUrl(capportUrl); mWiFiAgent.sendLinkProperties(newLp); - callback.expectLinkPropertiesThat(mWiFiAgent, lp -> - Objects.equals(expectedCapportUrl, lp.getCaptivePortalApiUrl())); - defaultCallback.expectLinkPropertiesThat(mWiFiAgent, lp -> - Objects.equals(expectedCapportUrl, lp.getCaptivePortalApiUrl())); + callback.expect(LINK_PROPERTIES_CHANGED, mWiFiAgent, cb -> + Objects.equals(expectedCapportUrl, cb.getLp().getCaptivePortalApiUrl())); + defaultCallback.expect(LINK_PROPERTIES_CHANGED, mWiFiAgent, cb -> + Objects.equals(expectedCapportUrl, cb.getLp().getCaptivePortalApiUrl())); final CaptivePortalData expectedCapportData = sanitized ? null : capportData; mWiFiAgent.notifyCapportApiDataChanged(capportData); - callback.expectLinkPropertiesThat(mWiFiAgent, lp -> - Objects.equals(expectedCapportData, lp.getCaptivePortalData())); - defaultCallback.expectLinkPropertiesThat(mWiFiAgent, lp -> - Objects.equals(expectedCapportData, lp.getCaptivePortalData())); + callback.expect(LINK_PROPERTIES_CHANGED, mWiFiAgent, cb -> + Objects.equals(expectedCapportData, cb.getLp().getCaptivePortalData())); + defaultCallback.expect(LINK_PROPERTIES_CHANGED, mWiFiAgent, cb -> + Objects.equals(expectedCapportData, cb.getLp().getCaptivePortalData())); final LinkProperties lp = mCm.getLinkProperties(mWiFiAgent.getNetwork()); assertEquals(expectedCapportUrl, lp.getCaptivePortalApiUrl()); @@ -4642,15 +4642,16 @@ public class ConnectivityServiceTest { mWiFiAgent.notifyCapportApiDataChanged(testData); - captivePortalCallback.expectLinkPropertiesThat(mWiFiAgent, - lp -> testData.equals(lp.getCaptivePortalData())); + captivePortalCallback.expect(LINK_PROPERTIES_CHANGED, mWiFiAgent, + cb -> testData.equals(cb.getLp().getCaptivePortalData())); final LinkProperties newLps = new LinkProperties(); newLps.setMtu(1234); mWiFiAgent.sendLinkProperties(newLps); // CaptivePortalData is not lost and unchanged when LPs are received from the NetworkAgent - captivePortalCallback.expectLinkPropertiesThat(mWiFiAgent, - lp -> testData.equals(lp.getCaptivePortalData()) && lp.getMtu() == 1234); + captivePortalCallback.expect(LINK_PROPERTIES_CHANGED, mWiFiAgent, + cb -> testData.equals(cb.getLp().getCaptivePortalData()) + && cb.getLp().getMtu() == 1234); } private TestNetworkCallback setupNetworkCallbackAndConnectToWifi() throws Exception { @@ -4744,8 +4745,8 @@ public class ConnectivityServiceTest { // Baseline capport data mWiFiAgent.notifyCapportApiDataChanged(captivePortalTestData.mCapportData); - captivePortalCallback.expectLinkPropertiesThat(mWiFiAgent, - lp -> captivePortalTestData.mCapportData.equals(lp.getCaptivePortalData())); + captivePortalCallback.expect(LINK_PROPERTIES_CHANGED, mWiFiAgent, + cb -> captivePortalTestData.mCapportData.equals(cb.getLp().getCaptivePortalData())); // Venue URL, T&C URL and friendly name from Network agent with Passpoint source, confirm // that API data gets precedence on the bytes remaining. @@ -4754,9 +4755,9 @@ public class ConnectivityServiceTest { mWiFiAgent.sendLinkProperties(linkProperties); // Make sure that the capport data is merged - captivePortalCallback.expectLinkPropertiesThat(mWiFiAgent, - lp -> captivePortalTestData.mExpectedMergedPasspointData - .equals(lp.getCaptivePortalData())); + captivePortalCallback.expect(LINK_PROPERTIES_CHANGED, mWiFiAgent, + cb -> captivePortalTestData.mExpectedMergedPasspointData.equals( + cb.getLp().getCaptivePortalData())); // Now send this information from non-Passpoint source, confirm that Capport data takes // precedence @@ -4764,9 +4765,9 @@ public class ConnectivityServiceTest { mWiFiAgent.sendLinkProperties(linkProperties); // Make sure that the capport data is merged - captivePortalCallback.expectLinkPropertiesThat(mWiFiAgent, - lp -> captivePortalTestData.mExpectedMergedOtherData - .equals(lp.getCaptivePortalData())); + captivePortalCallback.expect(LINK_PROPERTIES_CHANGED, mWiFiAgent, + cb -> captivePortalTestData.mExpectedMergedOtherData.equals( + cb.getLp().getCaptivePortalData())); // Create a new LP with no Network agent capport data final LinkProperties newLps = new LinkProperties(); @@ -4774,21 +4775,22 @@ public class ConnectivityServiceTest { mWiFiAgent.sendLinkProperties(newLps); // CaptivePortalData is not lost and has the original values when LPs are received from the // NetworkAgent - captivePortalCallback.expectLinkPropertiesThat(mWiFiAgent, - lp -> captivePortalTestData.mCapportData.equals(lp.getCaptivePortalData()) - && lp.getMtu() == 1234); + captivePortalCallback.expect(LINK_PROPERTIES_CHANGED, mWiFiAgent, + cb -> captivePortalTestData.mCapportData.equals(cb.getLp().getCaptivePortalData()) + && cb.getLp().getMtu() == 1234); // Now send capport data only from the Network agent mWiFiAgent.notifyCapportApiDataChanged(null); - captivePortalCallback.expectLinkPropertiesThat(mWiFiAgent, - lp -> lp.getCaptivePortalData() == null); + captivePortalCallback.expect(LINK_PROPERTIES_CHANGED, mWiFiAgent, + cb -> cb.getLp().getCaptivePortalData() == null); newLps.setCaptivePortalData(captivePortalTestData.mNaPasspointData); mWiFiAgent.sendLinkProperties(newLps); // Make sure that only the network agent capport data is available - captivePortalCallback.expectLinkPropertiesThat(mWiFiAgent, - lp -> captivePortalTestData.mNaPasspointData.equals(lp.getCaptivePortalData())); + captivePortalCallback.expect(LINK_PROPERTIES_CHANGED, mWiFiAgent, + cb -> captivePortalTestData.mNaPasspointData.equals( + cb.getLp().getCaptivePortalData())); } @Test @@ -4803,25 +4805,26 @@ public class ConnectivityServiceTest { mWiFiAgent.sendLinkProperties(linkProperties); // Make sure that the data is saved correctly - captivePortalCallback.expectLinkPropertiesThat(mWiFiAgent, - lp -> captivePortalTestData.mNaPasspointData.equals(lp.getCaptivePortalData())); + captivePortalCallback.expect(LINK_PROPERTIES_CHANGED, mWiFiAgent, + cb -> captivePortalTestData.mNaPasspointData.equals( + cb.getLp().getCaptivePortalData())); // Expected merged data: Network agent data is preferred, and values that are not used by // it are merged from capport data mWiFiAgent.notifyCapportApiDataChanged(captivePortalTestData.mCapportData); // Make sure that the Capport data is merged correctly - captivePortalCallback.expectLinkPropertiesThat(mWiFiAgent, - lp -> captivePortalTestData.mExpectedMergedPasspointData.equals( - lp.getCaptivePortalData())); + captivePortalCallback.expect(LINK_PROPERTIES_CHANGED, mWiFiAgent, + cb -> captivePortalTestData.mExpectedMergedPasspointData.equals( + cb.getLp().getCaptivePortalData())); // Now set the naData to null linkProperties.setCaptivePortalData(null); mWiFiAgent.sendLinkProperties(linkProperties); // Make sure that the Capport data is retained correctly - captivePortalCallback.expectLinkPropertiesThat(mWiFiAgent, - lp -> captivePortalTestData.mCapportData.equals(lp.getCaptivePortalData())); + captivePortalCallback.expect(LINK_PROPERTIES_CHANGED, mWiFiAgent, + cb -> captivePortalTestData.mCapportData.equals(cb.getLp().getCaptivePortalData())); } @Test @@ -4837,17 +4840,17 @@ public class ConnectivityServiceTest { mWiFiAgent.sendLinkProperties(linkProperties); // Make sure that the data is saved correctly - captivePortalCallback.expectLinkPropertiesThat(mWiFiAgent, - lp -> captivePortalTestData.mNaOtherData.equals(lp.getCaptivePortalData())); + captivePortalCallback.expect(LINK_PROPERTIES_CHANGED, mWiFiAgent, + cb -> captivePortalTestData.mNaOtherData.equals(cb.getLp().getCaptivePortalData())); // Expected merged data: Network agent data is preferred, and values that are not used by // it are merged from capport data mWiFiAgent.notifyCapportApiDataChanged(captivePortalTestData.mCapportData); // Make sure that the Capport data is merged correctly - captivePortalCallback.expectLinkPropertiesThat(mWiFiAgent, - lp -> captivePortalTestData.mExpectedMergedOtherData.equals( - lp.getCaptivePortalData())); + captivePortalCallback.expect(LINK_PROPERTIES_CHANGED, mWiFiAgent, + cb -> captivePortalTestData.mExpectedMergedOtherData.equals( + cb.getLp().getCaptivePortalData())); } private NetworkRequest.Builder newWifiRequestBuilder() { @@ -10335,19 +10338,19 @@ public class ConnectivityServiceTest { // Expect clatd to be stopped and started with the new prefix. mService.mResolverUnsolEventCallback.onNat64PrefixEvent(makeNat64PrefixEvent( cellNetId, PREFIX_OPERATION_ADDED, kOtherNat64PrefixString, 96)); - networkCallback.expectLinkPropertiesThat(mCellAgent, - (lp) -> lp.getStackedLinks().size() == 0); + networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent, + cb -> cb.getLp().getStackedLinks().size() == 0); verifyClatdStop(null /* inOrder */, MOBILE_IFNAME); assertRoutesRemoved(cellNetId, stackedDefault); verify(mMockNetd, times(1)).networkRemoveInterface(cellNetId, CLAT_MOBILE_IFNAME); verifyClatdStart(null /* inOrder */, MOBILE_IFNAME, cellNetId, kOtherNat64Prefix.toString()); - networkCallback.expectLinkPropertiesThat(mCellAgent, - (lp) -> lp.getNat64Prefix().equals(kOtherNat64Prefix)); + networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent, + cb -> cb.getLp().getNat64Prefix().equals(kOtherNat64Prefix)); clat.interfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true); - networkCallback.expectLinkPropertiesThat(mCellAgent, - (lp) -> lp.getStackedLinks().size() == 1); + networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent, + cb -> cb.getLp().getStackedLinks().size() == 1); assertRoutesAdded(cellNetId, stackedDefault); verify(mMockNetd, times(1)).networkAddInterface(cellNetId, CLAT_MOBILE_IFNAME); reset(mMockNetd); @@ -10388,7 +10391,8 @@ public class ConnectivityServiceTest { // Stopping prefix discovery causes netd to tell us that the NAT64 prefix is gone. mService.mResolverUnsolEventCallback.onNat64PrefixEvent(makeNat64PrefixEvent( cellNetId, PREFIX_OPERATION_REMOVED, kOtherNat64PrefixString, 96)); - networkCallback.expectLinkPropertiesThat(mCellAgent, lp -> lp.getNat64Prefix() == null); + networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent, + cb -> cb.getLp().getNat64Prefix() == null); // Remove IPv4 address and expect prefix discovery and clatd to be started again. cellLp.removeLinkAddress(myIpv4); @@ -10405,22 +10409,24 @@ public class ConnectivityServiceTest { // Clat iface comes up. Expect stacked link to be added. clat.interfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true); - networkCallback.expectLinkPropertiesThat(mCellAgent, - lp -> lp.getStackedLinks().size() == 1 && lp.getNat64Prefix() != null); + networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent, + cb -> cb.getLp().getStackedLinks().size() == 1 + && cb.getLp().getNat64Prefix() != null); assertRoutesAdded(cellNetId, stackedDefault); verify(mMockNetd, times(1)).networkAddInterface(cellNetId, CLAT_MOBILE_IFNAME); // NAT64 prefix is removed. Expect that clat is stopped. mService.mResolverUnsolEventCallback.onNat64PrefixEvent(makeNat64PrefixEvent( cellNetId, PREFIX_OPERATION_REMOVED, kNat64PrefixString, 96)); - networkCallback.expectLinkPropertiesThat(mCellAgent, - lp -> lp.getStackedLinks().size() == 0 && lp.getNat64Prefix() == null); + networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent, + cb -> cb.getLp().getStackedLinks().size() == 0 + && cb.getLp().getNat64Prefix() == null); assertRoutesRemoved(cellNetId, ipv4Subnet, stackedDefault); // Stop has no effect because clat is already stopped. verifyClatdStop(null /* inOrder */, MOBILE_IFNAME); - networkCallback.expectLinkPropertiesThat(mCellAgent, - lp -> lp.getStackedLinks().size() == 0); + networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent, + cb -> cb.getLp().getStackedLinks().size() == 0); verify(mMockNetd, times(1)).networkRemoveInterface(cellNetId, CLAT_MOBILE_IFNAME); verify(mMockNetd, times(1)).interfaceGetCfg(CLAT_MOBILE_IFNAME); // Clean up. @@ -10454,9 +10460,9 @@ public class ConnectivityServiceTest { verifyClatdStart(null /* inOrder */, MOBILE_IFNAME, cellNetId, kNat64Prefix.toString()); clat = getNat464Xlat(mCellAgent); clat.interfaceLinkStateChanged(CLAT_MOBILE_IFNAME, true /* up */); - networkCallback.expectLinkPropertiesThat(mCellAgent, - lp -> lp.getStackedLinks().size() == 1 - && lp.getNat64Prefix().equals(kNat64Prefix)); + networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent, + cb -> cb.getLp().getStackedLinks().size() == 1 + && cb.getLp().getNat64Prefix().equals(kNat64Prefix)); verify(mMockNetd).networkAddInterface(cellNetId, CLAT_MOBILE_IFNAME); // assertRoutesAdded sees all calls since last mMockNetd reset, so expect IPv6 routes again. assertRoutesAdded(cellNetId, ipv6Subnet, ipv6Default, stackedDefault); @@ -10480,7 +10486,8 @@ public class ConnectivityServiceTest { private void expectNat64PrefixChange(TestNetworkCallback callback, TestNetworkAgentWrapper agent, IpPrefix prefix) { - callback.expectLinkPropertiesThat(agent, x -> Objects.equals(x.getNat64Prefix(), prefix)); + callback.expect(LINK_PROPERTIES_CHANGED, agent, + x -> Objects.equals(x.getLp().getNat64Prefix(), prefix)); } @Test @@ -12119,7 +12126,8 @@ public class ConnectivityServiceTest { lp.addRoute(rio1); lp.addRoute(defaultRoute); mCellAgent.sendLinkProperties(lp); - networkCallback.expectLinkPropertiesThat(mCellAgent, x -> x.getRoutes().size() == 3); + networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent, + x -> x.getLp().getRoutes().size() == 3); assertRoutesAdded(netId, direct, rio1, defaultRoute); reset(mMockNetd); @@ -12134,7 +12142,8 @@ public class ConnectivityServiceTest { assertTrue(lp.getRoutes().contains(defaultWithMtu)); mCellAgent.sendLinkProperties(lp); - networkCallback.expectLinkPropertiesThat(mCellAgent, x -> x.getRoutes().contains(rio2)); + networkCallback.expect(LINK_PROPERTIES_CHANGED, mCellAgent, + x -> x.getLp().getRoutes().contains(rio2)); assertRoutesRemoved(netId, rio1); assertRoutesAdded(netId, rio2);