Merge "[FUI26] Address comments on aosp/1560408" am: 8f03f1e788 am: e5929c073c am: 13bcaf5446

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1618845

Change-Id: I8c62d218953027ee2fb5426ddacae5a6f8bb6c73
diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
index 63501d7..97ff3af 100644
--- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
@@ -128,6 +128,7 @@
 import static com.android.testutils.MiscAsserts.assertEmpty;
 import static com.android.testutils.MiscAsserts.assertLength;
 import static com.android.testutils.MiscAsserts.assertRunsInAtMost;
+import static com.android.testutils.MiscAsserts.assertSameElements;
 import static com.android.testutils.MiscAsserts.assertThrows;
 
 import static org.junit.Assert.assertEquals;
@@ -5885,20 +5886,8 @@
         mCm.unregisterNetworkCallback(networkCallback);
     }
 
-    private <T> void assertSameElementsNoDuplicates(T[] expected, T[] actual) {
-        // Easier to implement than a proper "assertSameElements" method that also correctly deals
-        // with duplicates.
-        final String msg = Arrays.toString(expected) + " != " + Arrays.toString(actual);
-        assertEquals(msg, expected.length, actual.length);
-        Set expectedSet = new ArraySet<>(Arrays.asList(expected));
-        assertEquals("expected contains duplicates", expectedSet.size(), expected.length);
-        // actual cannot have duplicates because it's the same length and has the same elements.
-        Set actualSet = new ArraySet<>(Arrays.asList(actual));
-        assertEquals(expectedSet, actualSet);
-    }
-
-    private void expectNetworkStatus(Network[] networks, String defaultIface,
-            Integer vpnUid, String vpnIfname, String[] underlyingIfaces) throws Exception {
+    private void expectNotifyNetworkStatus(List<Network> networks, String defaultIface,
+            Integer vpnUid, String vpnIfname, List<String> underlyingIfaces) throws Exception {
         ArgumentCaptor<List<Network>> networksCaptor = ArgumentCaptor.forClass(List.class);
         ArgumentCaptor<List<UnderlyingNetworkInfo>> vpnInfosCaptor =
                 ArgumentCaptor.forClass(List.class);
@@ -5906,26 +5895,24 @@
         verify(mStatsManager, atLeastOnce()).notifyNetworkStatus(networksCaptor.capture(),
                 any(List.class), eq(defaultIface), vpnInfosCaptor.capture());
 
-        assertSameElementsNoDuplicates(networksCaptor.getValue().toArray(), networks);
+        assertSameElements(networksCaptor.getValue(), networks);
 
-        UnderlyingNetworkInfo[] infos =
-                vpnInfosCaptor.getValue().toArray(new UnderlyingNetworkInfo[0]);
+        List<UnderlyingNetworkInfo> infos = vpnInfosCaptor.getValue();
         if (vpnUid != null) {
-            assertEquals("Should have exactly one VPN:", 1, infos.length);
-            UnderlyingNetworkInfo info = infos[0];
+            assertEquals("Should have exactly one VPN:", 1, infos.size());
+            UnderlyingNetworkInfo info = infos.get(0);
             assertEquals("Unexpected VPN owner:", (int) vpnUid, info.getOwnerUid());
             assertEquals("Unexpected VPN interface:", vpnIfname, info.getInterface());
-            assertSameElementsNoDuplicates(underlyingIfaces,
-                    info.getUnderlyingInterfaces().toArray(new String[0]));
+            assertSameElements(underlyingIfaces, info.getUnderlyingInterfaces());
         } else {
-            assertEquals(0, infos.length);
+            assertEquals(0, infos.size());
             return;
         }
     }
 
-    private void expectNetworkStatus(
-            Network[] networks, String defaultIface) throws Exception {
-        expectNetworkStatus(networks, defaultIface, null, null, new String[0]);
+    private void expectNotifyNetworkStatus(
+            List<Network> networks, String defaultIface) throws Exception {
+        expectNotifyNetworkStatus(networks, defaultIface, null, null, List.of());
     }
 
     @Test
@@ -5933,8 +5920,8 @@
         mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR);
         mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI);
 
-        final Network[] onlyCell = new Network[] {mCellNetworkAgent.getNetwork()};
-        final Network[] onlyWifi = new Network[] {mWiFiNetworkAgent.getNetwork()};
+        final List<Network> onlyCell = List.of(mCellNetworkAgent.getNetwork());
+        final List<Network> onlyWifi = List.of(mWiFiNetworkAgent.getNetwork());
 
         LinkProperties cellLp = new LinkProperties();
         cellLp.setInterfaceName(MOBILE_IFNAME);
@@ -5945,7 +5932,7 @@
         mCellNetworkAgent.connect(false);
         mCellNetworkAgent.sendLinkProperties(cellLp);
         waitForIdle();
-        expectNetworkStatus(onlyCell, MOBILE_IFNAME);
+        expectNotifyNetworkStatus(onlyCell, MOBILE_IFNAME);
         reset(mStatsManager);
 
         // Default network switch should update ifaces.
@@ -5953,37 +5940,37 @@
         mWiFiNetworkAgent.sendLinkProperties(wifiLp);
         waitForIdle();
         assertEquals(wifiLp, mService.getActiveLinkProperties());
-        expectNetworkStatus(onlyWifi, WIFI_IFNAME);
+        expectNotifyNetworkStatus(onlyWifi, WIFI_IFNAME);
         reset(mStatsManager);
 
         // Disconnect should update ifaces.
         mWiFiNetworkAgent.disconnect();
         waitForIdle();
-        expectNetworkStatus(onlyCell, MOBILE_IFNAME);
+        expectNotifyNetworkStatus(onlyCell, MOBILE_IFNAME);
         reset(mStatsManager);
 
         // Metered change should update ifaces
         mCellNetworkAgent.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED);
         waitForIdle();
-        expectNetworkStatus(onlyCell, MOBILE_IFNAME);
+        expectNotifyNetworkStatus(onlyCell, MOBILE_IFNAME);
         reset(mStatsManager);
 
         mCellNetworkAgent.removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED);
         waitForIdle();
-        expectNetworkStatus(onlyCell, MOBILE_IFNAME);
+        expectNotifyNetworkStatus(onlyCell, MOBILE_IFNAME);
         reset(mStatsManager);
 
         // Temp metered change shouldn't update ifaces
         mCellNetworkAgent.addCapability(NET_CAPABILITY_TEMPORARILY_NOT_METERED);
         waitForIdle();
-        verify(mStatsManager, never()).notifyNetworkStatus(eq(Arrays.asList(onlyCell)),
+        verify(mStatsManager, never()).notifyNetworkStatus(eq(onlyCell),
                 any(List.class), eq(MOBILE_IFNAME), any(List.class));
         reset(mStatsManager);
 
         // Roaming change should update ifaces
         mCellNetworkAgent.addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING);
         waitForIdle();
-        expectNetworkStatus(onlyCell, MOBILE_IFNAME);
+        expectNotifyNetworkStatus(onlyCell, MOBILE_IFNAME);
         reset(mStatsManager);
 
         // Test VPNs.
@@ -5993,29 +5980,29 @@
         mMockVpn.establishForMyUid(lp);
         assertUidRangesUpdatedForMyUid(true);
 
-        final Network[] cellAndVpn = new Network[] {
-                mCellNetworkAgent.getNetwork(), mMockVpn.getNetwork()};
+        final List<Network> cellAndVpn =
+                List.of(mCellNetworkAgent.getNetwork(), mMockVpn.getNetwork());
 
         // A VPN with default (null) underlying networks sets the underlying network's interfaces...
-        expectNetworkStatus(cellAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
-                new String[]{MOBILE_IFNAME});
+        expectNotifyNetworkStatus(cellAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
+                List.of(MOBILE_IFNAME));
 
         // ...and updates them as the default network switches.
         mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI);
         mWiFiNetworkAgent.connect(false);
         mWiFiNetworkAgent.sendLinkProperties(wifiLp);
         final Network[] onlyNull = new Network[]{null};
-        final Network[] wifiAndVpn = new Network[] {
-                mWiFiNetworkAgent.getNetwork(), mMockVpn.getNetwork()};
-        final Network[] cellAndWifi = new Network[] {
-                mCellNetworkAgent.getNetwork(), mWiFiNetworkAgent.getNetwork()};
-        final Network[] cellNullAndWifi = new Network[] {
-                mCellNetworkAgent.getNetwork(), null, mWiFiNetworkAgent.getNetwork()};
+        final List<Network> wifiAndVpn =
+                List.of(mWiFiNetworkAgent.getNetwork(), mMockVpn.getNetwork());
+        final List<Network> cellAndWifi =
+                List.of(mCellNetworkAgent.getNetwork(), mWiFiNetworkAgent.getNetwork());
+        final Network[] cellNullAndWifi =
+                new Network[]{mCellNetworkAgent.getNetwork(), null, mWiFiNetworkAgent.getNetwork()};
 
         waitForIdle();
         assertEquals(wifiLp, mService.getActiveLinkProperties());
-        expectNetworkStatus(wifiAndVpn, WIFI_IFNAME, Process.myUid(), VPN_IFNAME,
-                new String[]{WIFI_IFNAME});
+        expectNotifyNetworkStatus(wifiAndVpn, WIFI_IFNAME, Process.myUid(), VPN_IFNAME,
+                List.of(WIFI_IFNAME));
         reset(mStatsManager);
 
         // A VPN that sets its underlying networks passes the underlying interfaces, and influences
@@ -6024,23 +6011,23 @@
         // MOBILE_IFNAME even though the default network is wifi.
         // TODO: fix this to pass in the actual default network interface. Whether or not the VPN
         // applies to the system server UID should not have any bearing on network stats.
-        mMockVpn.setUnderlyingNetworks(onlyCell);
+        mMockVpn.setUnderlyingNetworks(onlyCell.toArray(new Network[0]));
         waitForIdle();
-        expectNetworkStatus(wifiAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
-                new String[]{MOBILE_IFNAME});
+        expectNotifyNetworkStatus(wifiAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
+                List.of(MOBILE_IFNAME));
         reset(mStatsManager);
 
-        mMockVpn.setUnderlyingNetworks(cellAndWifi);
+        mMockVpn.setUnderlyingNetworks(cellAndWifi.toArray(new Network[0]));
         waitForIdle();
-        expectNetworkStatus(wifiAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
-                new String[]{MOBILE_IFNAME, WIFI_IFNAME});
+        expectNotifyNetworkStatus(wifiAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
+                List.of(MOBILE_IFNAME, WIFI_IFNAME));
         reset(mStatsManager);
 
         // Null underlying networks are ignored.
         mMockVpn.setUnderlyingNetworks(cellNullAndWifi);
         waitForIdle();
-        expectNetworkStatus(wifiAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
-                new String[]{MOBILE_IFNAME, WIFI_IFNAME});
+        expectNotifyNetworkStatus(wifiAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
+                List.of(MOBILE_IFNAME, WIFI_IFNAME));
         reset(mStatsManager);
 
         // If an underlying network disconnects, that interface should no longer be underlying.
@@ -6053,8 +6040,8 @@
         mCellNetworkAgent.disconnect();
         waitForIdle();
         assertNull(mService.getLinkProperties(mCellNetworkAgent.getNetwork()));
-        expectNetworkStatus(wifiAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
-                new String[]{MOBILE_IFNAME, WIFI_IFNAME});
+        expectNotifyNetworkStatus(wifiAndVpn, MOBILE_IFNAME, Process.myUid(), VPN_IFNAME,
+                List.of(MOBILE_IFNAME, WIFI_IFNAME));
 
         // Confirm that we never tell NetworkStatsService that cell is no longer the underlying
         // network for the VPN...
@@ -6089,26 +6076,26 @@
         // Also, for the same reason as above, the active interface passed in is null.
         mMockVpn.setUnderlyingNetworks(new Network[0]);
         waitForIdle();
-        expectNetworkStatus(wifiAndVpn, null);
+        expectNotifyNetworkStatus(wifiAndVpn, null);
         reset(mStatsManager);
 
         // Specifying only a null underlying network is the same as no networks.
         mMockVpn.setUnderlyingNetworks(onlyNull);
         waitForIdle();
-        expectNetworkStatus(wifiAndVpn, null);
+        expectNotifyNetworkStatus(wifiAndVpn, null);
         reset(mStatsManager);
 
         // Specifying networks that are all disconnected is the same as specifying no networks.
-        mMockVpn.setUnderlyingNetworks(onlyCell);
+        mMockVpn.setUnderlyingNetworks(onlyCell.toArray(new Network[0]));
         waitForIdle();
-        expectNetworkStatus(wifiAndVpn, null);
+        expectNotifyNetworkStatus(wifiAndVpn, null);
         reset(mStatsManager);
 
         // Passing in null again means follow the default network again.
         mMockVpn.setUnderlyingNetworks(null);
         waitForIdle();
-        expectNetworkStatus(wifiAndVpn, WIFI_IFNAME, Process.myUid(), VPN_IFNAME,
-                new String[]{WIFI_IFNAME});
+        expectNotifyNetworkStatus(wifiAndVpn, WIFI_IFNAME, Process.myUid(), VPN_IFNAME,
+                List.of(WIFI_IFNAME));
         reset(mStatsManager);
     }