From c559512cd22db1bc230250156a7e0349dab8d7fc Mon Sep 17 00:00:00 2001 From: Remi NGUYEN VAN Date: Wed, 13 Apr 2022 19:23:12 +0900 Subject: [PATCH] Fix CtsNetTestCases on Q - Skip NetworkCapabilitiesTest on Q. The test covers code that is very localized in NetworkCapabilities.java, and is only updatable on S+, so does not need to be in MTS before that. - Fix ConnectivityManagerTest to remove external references to CaptivePortalData instances in the lambdas, as this is compiled into a method taking CaptivePortalData as argument, and because the class does not exist on Q, the test runner would crash enumerating method arguments. - Remove call to getSystemService(ETHERNET_SERVICE) on Q from the test thread, as it will crash when called from a thread that does not have a Looper. Test: atest CtsNetTestCases Bug: 227253613 Change-Id: I97c64b24c767e874dfba00aa1f46be2c4972f917 --- .../android/net/NetworkCapabilitiesTest.java | 39 ++++++++++--------- .../net/cts/ConnectivityManagerTest.java | 25 +++++++++--- 2 files changed, 41 insertions(+), 23 deletions(-) diff --git a/tests/common/java/android/net/NetworkCapabilitiesTest.java b/tests/common/java/android/net/NetworkCapabilitiesTest.java index 9ae5fab9d6..c30e1d3830 100644 --- a/tests/common/java/android/net/NetworkCapabilitiesTest.java +++ b/tests/common/java/android/net/NetworkCapabilitiesTest.java @@ -82,12 +82,11 @@ import android.test.suitebuilder.annotation.SmallTest; import android.util.ArraySet; import android.util.Range; -import androidx.test.runner.AndroidJUnit4; - import com.android.testutils.CompatUtil; import com.android.testutils.ConnectivityModuleTest; import com.android.testutils.DevSdkIgnoreRule; import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo; +import com.android.testutils.DevSdkIgnoreRunner; import org.junit.Rule; import org.junit.Test; @@ -99,8 +98,12 @@ import java.util.Arrays; import java.util.List; import java.util.Set; -@RunWith(AndroidJUnit4.class) @SmallTest +@RunWith(DevSdkIgnoreRunner.class) +// NetworkCapabilities is only updatable on S+, and this test covers behavior which implementation +// is self-contained within NetworkCapabilities.java, so it does not need to be run on, or +// compatible with, earlier releases. +@IgnoreUpTo(Build.VERSION_CODES.R) @ConnectivityModuleTest public class NetworkCapabilitiesTest { private static final String TEST_SSID = "TEST_SSID"; @@ -489,7 +492,7 @@ public class NetworkCapabilitiesTest { assertTrue(netCap.hasCapability(NET_CAPABILITY_NOT_RESTRICTED)); } - @Test @IgnoreUpTo(Build.VERSION_CODES.R) + @Test public void testOemPrivate() { NetworkCapabilities nc = new NetworkCapabilities(); // By default OEM_PRIVATE is neither in the required or forbidden lists and the network is @@ -516,7 +519,7 @@ public class NetworkCapabilitiesTest { assertFalse(nr.satisfiedByNetworkCapabilities(new NetworkCapabilities())); } - @Test @IgnoreUpTo(Build.VERSION_CODES.R) + @Test public void testForbiddenCapabilities() { NetworkCapabilities network = new NetworkCapabilities(); @@ -630,7 +633,7 @@ public class NetworkCapabilitiesTest { return new Range(from, to); } - @Test @IgnoreUpTo(Build.VERSION_CODES.Q) + @Test public void testSetAdministratorUids() { NetworkCapabilities nc = new NetworkCapabilities().setAdministratorUids(new int[] {2, 1, 3}); @@ -638,7 +641,7 @@ public class NetworkCapabilitiesTest { assertArrayEquals(new int[] {1, 2, 3}, nc.getAdministratorUids()); } - @Test @IgnoreUpTo(Build.VERSION_CODES.Q) + @Test public void testSetAdministratorUidsWithDuplicates() { try { new NetworkCapabilities().setAdministratorUids(new int[] {1, 1}); @@ -750,7 +753,7 @@ public class NetworkCapabilitiesTest { () -> nc2.addTransportType(TRANSPORT_WIFI)); } - @Test @IgnoreUpTo(Build.VERSION_CODES.R) // New behavior in updatable NetworkCapabilities (S+) + @Test public void testSetNetworkSpecifierOnTestMultiTransportNc() { final NetworkSpecifier specifier = CompatUtil.makeEthernetNetworkSpecifier("eth0"); NetworkCapabilities nc = new NetworkCapabilities.Builder() @@ -859,7 +862,7 @@ public class NetworkCapabilitiesTest { assertEquals(TRANSPORT_TEST, transportTypes[3]); } - @Test @IgnoreUpTo(Build.VERSION_CODES.Q) + @Test public void testTelephonyNetworkSpecifier() { final TelephonyNetworkSpecifier specifier = new TelephonyNetworkSpecifier(1); final NetworkCapabilities nc1 = new NetworkCapabilities.Builder() @@ -970,7 +973,7 @@ public class NetworkCapabilitiesTest { assertEquals(specifier, nc.getNetworkSpecifier()); } - @Test @IgnoreUpTo(Build.VERSION_CODES.Q) + @Test public void testAdministratorUidsAndOwnerUid() { // Test default owner uid. // If the owner uid is not set, the default value should be Process.INVALID_UID. @@ -1014,7 +1017,7 @@ public class NetworkCapabilitiesTest { return nc; } - @Test @IgnoreUpTo(Build.VERSION_CODES.R) + @Test public void testSubIds() throws Exception { final NetworkCapabilities ncWithoutId = capsWithSubIds(); final NetworkCapabilities ncWithId = capsWithSubIds(TEST_SUBID1); @@ -1036,7 +1039,7 @@ public class NetworkCapabilitiesTest { assertTrue(requestWithoutId.canBeSatisfiedBy(ncWithId)); } - @Test @IgnoreUpTo(Build.VERSION_CODES.R) + @Test public void testEqualsSubIds() throws Exception { assertEquals(capsWithSubIds(), capsWithSubIds()); assertNotEquals(capsWithSubIds(), capsWithSubIds(TEST_SUBID1)); @@ -1185,7 +1188,7 @@ public class NetworkCapabilitiesTest { } } - @Test @IgnoreUpTo(Build.VERSION_CODES.Q) + @Test public void testBuilder() { final int ownerUid = 1001; final int signalStrength = -80; @@ -1255,7 +1258,7 @@ public class NetworkCapabilitiesTest { } } - @Test @IgnoreUpTo(Build.VERSION_CODES.R) + @Test public void testBuilderWithoutDefaultCap() { final NetworkCapabilities nc = NetworkCapabilities.Builder.withoutDefaultCapabilities().build(); @@ -1266,12 +1269,12 @@ public class NetworkCapabilitiesTest { assertEquals(0, nc.getCapabilities().length); } - @Test @IgnoreUpTo(Build.VERSION_CODES.R) + @Test public void testRestrictCapabilitiesForTestNetworkByNotOwnerWithNonRestrictedNc() { testRestrictCapabilitiesForTestNetworkWithNonRestrictedNc(false /* isOwner */); } - @Test @IgnoreUpTo(Build.VERSION_CODES.R) + @Test public void testRestrictCapabilitiesForTestNetworkByOwnerWithNonRestrictedNc() { testRestrictCapabilitiesForTestNetworkWithNonRestrictedNc(true /* isOwner */); } @@ -1316,12 +1319,12 @@ public class NetworkCapabilitiesTest { assertEquals(expectedNcBuilder.build(), nonRestrictedNc); } - @Test @IgnoreUpTo(Build.VERSION_CODES.R) + @Test public void testRestrictCapabilitiesForTestNetworkByNotOwnerWithRestrictedNc() { testRestrictCapabilitiesForTestNetworkWithRestrictedNc(false /* isOwner */); } - @Test @IgnoreUpTo(Build.VERSION_CODES.R) + @Test public void testRestrictCapabilitiesForTestNetworkByOwnerWithRestrictedNc() { testRestrictCapabilitiesForTestNetworkWithRestrictedNc(true /* isOwner */); } diff --git a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java index d40bc9fd51..d41fc2c6f3 100644 --- a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java +++ b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java @@ -151,6 +151,7 @@ import android.os.Handler; import android.os.Looper; import android.os.MessageQueue; import android.os.Process; +import android.os.ServiceManager; import android.os.SystemClock; import android.os.SystemProperties; import android.os.UserHandle; @@ -660,10 +661,12 @@ public class ConnectivityManagerTest { // CaptivePortalApiUrl & CaptivePortalData will be preserved if the given uid holds the // NETWORK_SETTINGS permission. - assertEquals(capportUrl, + assertNotNull(lp.getCaptivePortalApiUrl()); + assertNotNull(lp.getCaptivePortalData()); + assertEquals(lp.getCaptivePortalApiUrl(), mCm.getRedactedLinkPropertiesForPackage(lp, privilegedUid, privilegedPkg) .getCaptivePortalApiUrl()); - assertEquals(capportData, + assertEquals(lp.getCaptivePortalData(), mCm.getRedactedLinkPropertiesForPackage(lp, privilegedUid, privilegedPkg) .getCaptivePortalData()); }); @@ -885,9 +888,21 @@ public class ConnectivityManagerTest { // // Note that this test this will still fail in instant mode if a device supports Ethernet // via other hardware means. We are not currently aware of any such device. - return (mContext.getSystemService(Context.ETHERNET_SERVICE) != null) || - mPackageManager.hasSystemFeature(FEATURE_ETHERNET) || - mPackageManager.hasSystemFeature(FEATURE_USB_HOST); + return hasEthernetService() + || mPackageManager.hasSystemFeature(FEATURE_ETHERNET) + || mPackageManager.hasSystemFeature(FEATURE_USB_HOST); + } + + private boolean hasEthernetService() { + // On Q creating EthernetManager from a thread that does not have a looper (like the test + // thread) crashes because it tried to use Looper.myLooper() through the default Handler + // constructor to run onAvailabilityChanged callbacks. Use ServiceManager to check whether + // the service exists instead. + // TODO: remove once Q is no longer supported in MTS, as ServiceManager is hidden API + if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.Q) { + return ServiceManager.getService(Context.ETHERNET_SERVICE) != null; + } + return mContext.getSystemService(Context.ETHERNET_SERVICE) != null; } private boolean shouldBeSupported(int networkType) {