From ccd3020a91751595dd05f025f8bcacab3f56a504 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Thu, 21 Jul 2022 15:42:37 +0900 Subject: [PATCH] Simplify per-target-SDK LinkProperties tests. Currently these tests run as hostside tests and require a lot of boilerplate, including their own CtsHostsideNetworkTestsApp3 build target. Move the coverage to LinkPropertiesTest using the new-ish @CtsNetTestCasesMaxTargetSdk31 annotation, and remove the hostside tests. The only test that is not being moved is one that checks the behaviour of disabling the compat change on T. This test is not very useful because it can only run on userdebug builds and not on production builds, because CtsHostsideNetworkTestsApp3 targets SDK 33. We do have test coverage for disabling the compat change on an app targeting SDK 31, where it is allowed. Fix: 236087258 Test: atest CtsNetTestCasesMaxTargetSdk31 Test: atest CtsNetTestCasesLatestSdk:android.net.LinkPropertiesTest Change-Id: I6d4b1ba40f6cb63b30a600c227e9628858c03d73 --- .../java/android/net/LinkPropertiesTest.java | 90 +++++++++++-------- tests/cts/hostside/Android.bp | 3 - tests/cts/hostside/app3/Android.bp | 54 ----------- tests/cts/hostside/app3/AndroidManifest.xml | 27 ------ .../app3/ExcludedRoutesGatingTest.java | 80 ----------------- .../HostsideLinkPropertiesGatingTests.java | 85 ------------------ 6 files changed, 51 insertions(+), 288 deletions(-) delete mode 100644 tests/cts/hostside/app3/Android.bp delete mode 100644 tests/cts/hostside/app3/AndroidManifest.xml delete mode 100644 tests/cts/hostside/app3/src/com/android/cts/net/hostside/app3/ExcludedRoutesGatingTest.java delete mode 100644 tests/cts/hostside/src/com/android/cts/net/HostsideLinkPropertiesGatingTests.java diff --git a/tests/common/java/android/net/LinkPropertiesTest.java b/tests/common/java/android/net/LinkPropertiesTest.java index 9506fc9379..5ee375f2c4 100644 --- a/tests/common/java/android/net/LinkPropertiesTest.java +++ b/tests/common/java/android/net/LinkPropertiesTest.java @@ -36,10 +36,10 @@ import android.os.Build; import android.system.OsConstants; import android.util.ArraySet; -import androidx.core.os.BuildCompat; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; +import com.android.modules.utils.build.SdkLevel; import com.android.net.module.util.LinkPropertiesUtils.CompareResult; import com.android.testutils.ConnectivityModuleTest; import com.android.testutils.DevSdkIgnoreRule; @@ -114,11 +114,6 @@ public class LinkPropertiesTest { return InetAddresses.parseNumericAddress(addrString); } - private static boolean isAtLeastR() { - // BuildCompat.isAtLeastR is documented to return false on release SDKs (including R) - return Build.VERSION.SDK_INT > Build.VERSION_CODES.Q || BuildCompat.isAtLeastR(); - } - private void checkEmpty(final LinkProperties lp) { assertEquals(0, lp.getAllInterfaceNames().size()); assertEquals(0, lp.getAllAddresses().size()); @@ -139,7 +134,7 @@ public class LinkPropertiesTest { assertFalse(lp.isIpv6Provisioned()); assertFalse(lp.isPrivateDnsActive()); - if (isAtLeastR()) { + if (SdkLevel.isAtLeastR()) { assertNull(lp.getDhcpServerAddress()); assertFalse(lp.isWakeOnLanSupported()); assertNull(lp.getCaptivePortalApiUrl()); @@ -166,7 +161,7 @@ public class LinkPropertiesTest { lp.setMtu(MTU); lp.setTcpBufferSizes(TCP_BUFFER_SIZES); lp.setNat64Prefix(new IpPrefix("2001:db8:0:64::/96")); - if (isAtLeastR()) { + if (SdkLevel.isAtLeastR()) { lp.setDhcpServerAddress(DHCPSERVER); lp.setWakeOnLanSupported(true); lp.setCaptivePortalApiUrl(CAPPORT_API_URL); @@ -210,7 +205,7 @@ public class LinkPropertiesTest { assertTrue(source.isIdenticalTcpBufferSizes(target)); assertTrue(target.isIdenticalTcpBufferSizes(source)); - if (isAtLeastR()) { + if (SdkLevel.isAtLeastR()) { assertTrue(source.isIdenticalDhcpServerAddress(target)); assertTrue(source.isIdenticalDhcpServerAddress(source)); @@ -1295,56 +1290,73 @@ public class LinkPropertiesTest { assertEquals(2, lp.getRoutes().size()); } - @Test @IgnoreUpTo(Build.VERSION_CODES.R) - @CtsNetTestCasesMaxTargetSdk31(reason = "Compat change cannot be overridden when targeting T+") - @EnableCompatChanges({LinkProperties.EXCLUDED_ROUTES}) - public void testExcludedRoutesEnabled() { + private void assertExcludeRoutesVisible() { final LinkProperties lp = new LinkProperties(); assertEquals(0, lp.getRoutes().size()); - lp.addRoute(new RouteInfo(new IpPrefix(ADDRV4, 0), RTN_UNREACHABLE)); + lp.addRoute(new RouteInfo(new IpPrefix(ADDRV4, 31), RTN_UNREACHABLE)); assertEquals(1, lp.getRoutes().size()); - lp.addRoute(new RouteInfo(new IpPrefix(ADDRV6, 0), RTN_THROW)); + lp.addRoute(new RouteInfo(new IpPrefix(ADDRV6, 127), RTN_THROW)); assertEquals(2, lp.getRoutes().size()); lp.addRoute(new RouteInfo(GATEWAY1)); assertEquals(3, lp.getRoutes().size()); + + lp.addRoute(new RouteInfo(new IpPrefix(DNS6, 127), RTN_UNICAST)); + assertEquals(4, lp.getRoutes().size()); } - @Test @IgnoreUpTo(Build.VERSION_CODES.R) @IgnoreAfter(Build.VERSION_CODES.S_V2) - @CtsNetTestCasesMaxTargetSdk31(reason = "Compat change cannot be overridden when targeting T+") - @DisableCompatChanges({LinkProperties.EXCLUDED_ROUTES}) - public void testExcludedRoutesDisabled_S() { + private void assertExcludeRoutesNotVisible() { final LinkProperties lp = new LinkProperties(); assertEquals(0, lp.getRoutes().size()); - lp.addRoute(new RouteInfo(new IpPrefix(ADDRV4, 0), RTN_UNREACHABLE)); + lp.addRoute(new RouteInfo(new IpPrefix(ADDRV4, 31), RTN_UNREACHABLE)); + assertEquals(0, lp.getRoutes().size()); + + lp.addRoute(new RouteInfo(new IpPrefix(ADDRV6, 127), RTN_THROW)); + assertEquals(0, lp.getRoutes().size()); + + lp.addRoute(new RouteInfo(GATEWAY1)); assertEquals(1, lp.getRoutes().size()); - lp.addRoute(new RouteInfo(new IpPrefix(ADDRV6, 5), RTN_THROW)); - // RTN_THROW routes are visible on S when added by the caller (but they are not added by - // the system). This is uncommon usage but was tested by CTSv12. + lp.addRoute(new RouteInfo(new IpPrefix(DNS6, 127), RTN_UNICAST)); assertEquals(2, lp.getRoutes().size()); - - lp.addRoute(new RouteInfo(new IpPrefix(ADDRV6, 2), RTN_UNICAST)); - assertEquals(3, lp.getRoutes().size()); } - @Test @IgnoreUpTo(Build.VERSION_CODES.S_V2) + private void checkExcludeRoutesNotVisibleAfterS() { + if (!SdkLevel.isAtLeastT()) { + // RTN_THROW routes are visible on R and S when added by the caller (but they are not + // added by the system except for legacy VPN). + // This is uncommon usage but was tested by CTSr12. + assertExcludeRoutesVisible(); + } else { + assertExcludeRoutesNotVisible(); + } + } + + @Test @IgnoreUpTo(Build.VERSION_CODES.R) + @CtsNetTestCasesMaxTargetSdk31(reason = "Testing behaviour for target SDK 31") + public void testExcludedRoutesNotVisibleOnTargetSdk31() { + checkExcludeRoutesNotVisibleAfterS(); + } + + @Test + public void testExcludedRoutesVisibleOnTargetSdk33AndAbove() { + assertExcludeRoutesVisible(); + } + + @Test @IgnoreUpTo(Build.VERSION_CODES.R) + @CtsNetTestCasesMaxTargetSdk31(reason = "Compat change cannot be overridden when targeting T+") + @EnableCompatChanges({LinkProperties.EXCLUDED_ROUTES}) + public void testExcludedRoutesEnabledByCompatChange() { + assertExcludeRoutesVisible(); + } + + @Test @IgnoreUpTo(Build.VERSION_CODES.R) @CtsNetTestCasesMaxTargetSdk31(reason = "Compat change cannot be overridden when targeting T+") @DisableCompatChanges({LinkProperties.EXCLUDED_ROUTES}) - public void testExcludedRoutesDisabled() { - final LinkProperties lp = new LinkProperties(); - assertEquals(0, lp.getRoutes().size()); - - lp.addRoute(new RouteInfo(new IpPrefix(ADDRV4, 0), RTN_UNREACHABLE)); - assertEquals(0, lp.getRoutes().size()); - - lp.addRoute(new RouteInfo(new IpPrefix(ADDRV6, 5), RTN_THROW)); - assertEquals(0, lp.getRoutes().size()); - - lp.addRoute(new RouteInfo(new IpPrefix(ADDRV6, 2), RTN_UNICAST)); - assertEquals(1, lp.getRoutes().size()); + public void testExcludedRoutesDisabledByCompatChange() { + checkExcludeRoutesNotVisibleAfterS(); } } diff --git a/tests/cts/hostside/Android.bp b/tests/cts/hostside/Android.bp index ac84e570cd..47ea53e828 100644 --- a/tests/cts/hostside/Android.bp +++ b/tests/cts/hostside/Android.bp @@ -26,7 +26,6 @@ java_test_host { "tradefed", ], static_libs: [ - "CompatChangeGatingTestBase", "modules-utils-build-testing", ], // Tag this module as a cts test artifact @@ -38,8 +37,6 @@ java_test_host { data: [ ":CtsHostsideNetworkTestsApp", ":CtsHostsideNetworkTestsApp2", - ":CtsHostsideNetworkTestsApp3", - ":CtsHostsideNetworkTestsApp3PreT", ":CtsHostsideNetworkTestsAppNext", ], per_testcase_directory: true, diff --git a/tests/cts/hostside/app3/Android.bp b/tests/cts/hostside/app3/Android.bp deleted file mode 100644 index 141cf0324d..0000000000 --- a/tests/cts/hostside/app3/Android.bp +++ /dev/null @@ -1,54 +0,0 @@ -// -// Copyright (C) 2022 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -package { - default_applicable_licenses: ["Android-Apache-2.0"], -} - -java_defaults { - name: "CtsHostsideNetworkTestsApp3Defaults", - srcs: ["src/**/*.java"], - libs: [ - "junit", - ], - static_libs: [ - "ctstestrunner-axt", - "truth-prebuilt", - ], - - // Tag this module as a cts test artifact - test_suites: [ - "cts", - "general-tests", - ], -} - -android_test_helper_app { - name: "CtsHostsideNetworkTestsApp3", - defaults: [ - "cts_support_defaults", - "CtsHostsideNetworkTestsApp3Defaults", - ], -} - -android_test_helper_app { - name: "CtsHostsideNetworkTestsApp3PreT", - target_sdk_version: "31", - defaults: [ - "cts_support_defaults", - "CtsHostsideNetworkTestsApp3Defaults", - ], -} diff --git a/tests/cts/hostside/app3/AndroidManifest.xml b/tests/cts/hostside/app3/AndroidManifest.xml deleted file mode 100644 index eabcacbddf..0000000000 --- a/tests/cts/hostside/app3/AndroidManifest.xml +++ /dev/null @@ -1,27 +0,0 @@ - - - - - - - - - - - - diff --git a/tests/cts/hostside/app3/src/com/android/cts/net/hostside/app3/ExcludedRoutesGatingTest.java b/tests/cts/hostside/app3/src/com/android/cts/net/hostside/app3/ExcludedRoutesGatingTest.java deleted file mode 100644 index a1a8209db2..0000000000 --- a/tests/cts/hostside/app3/src/com/android/cts/net/hostside/app3/ExcludedRoutesGatingTest.java +++ /dev/null @@ -1,80 +0,0 @@ -/* - * Copyright (C) 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.cts.net.hostside.app3; - -import static org.junit.Assert.assertEquals; - -import android.Manifest; -import android.net.IpPrefix; -import android.net.LinkProperties; -import android.net.RouteInfo; - -import androidx.test.InstrumentationRegistry; -import androidx.test.runner.AndroidJUnit4; - -import org.junit.After; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; - -/** - * Tests to verify {@link LinkProperties#getRoutes} behavior, depending on - * {@LinkProperties#EXCLUDED_ROUTES} change state. - */ -@RunWith(AndroidJUnit4.class) -public class ExcludedRoutesGatingTest { - @Before - public void setUp() { - InstrumentationRegistry.getInstrumentation().getUiAutomation() - .adoptShellPermissionIdentity(Manifest.permission.LOG_COMPAT_CHANGE, - Manifest.permission.READ_COMPAT_CHANGE_CONFIG); - } - - @After - public void tearDown() { - InstrumentationRegistry.getInstrumentation().getUiAutomation() - .dropShellPermissionIdentity(); - } - - @Test - public void testExcludedRoutesChangeEnabled() { - final LinkProperties lp = makeLinkPropertiesWithExcludedRoutes(); - - // Excluded routes change is enabled: non-RTN_UNICAST routes are visible. - assertEquals(2, lp.getRoutes().size()); - assertEquals(2, lp.getAllRoutes().size()); - } - - @Test - public void testExcludedRoutesChangeDisabled() { - final LinkProperties lp = makeLinkPropertiesWithExcludedRoutes(); - - // Excluded routes change is disabled: non-RTN_UNICAST routes are filtered out. - assertEquals(0, lp.getRoutes().size()); - assertEquals(0, lp.getAllRoutes().size()); - } - - private LinkProperties makeLinkPropertiesWithExcludedRoutes() { - final LinkProperties lp = new LinkProperties(); - - lp.addRoute(new RouteInfo(new IpPrefix("10.0.0.0/8"), null, null, RouteInfo.RTN_THROW)); - lp.addRoute(new RouteInfo(new IpPrefix("2001:db8::/64"), null, null, - RouteInfo.RTN_UNREACHABLE)); - - return lp; - } -} diff --git a/tests/cts/hostside/src/com/android/cts/net/HostsideLinkPropertiesGatingTests.java b/tests/cts/hostside/src/com/android/cts/net/HostsideLinkPropertiesGatingTests.java deleted file mode 100644 index 9a1fa4234a..0000000000 --- a/tests/cts/hostside/src/com/android/cts/net/HostsideLinkPropertiesGatingTests.java +++ /dev/null @@ -1,85 +0,0 @@ -/* - * Copyright (C) 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.cts.net; - -import android.compat.cts.CompatChangeGatingTestCase; - -import java.util.Set; - -/** - * Tests for the {@link android.net.LinkProperties#EXCLUDED_ROUTES} compatibility change. - * - * TODO: see if we can delete this cumbersome host test by moving the coverage to CtsNetTestCases - * and CtsNetTestCasesMaxTargetSdk31. - */ -public class HostsideLinkPropertiesGatingTests extends CompatChangeGatingTestCase { - private static final String TEST_APK = "CtsHostsideNetworkTestsApp3.apk"; - private static final String TEST_APK_PRE_T = "CtsHostsideNetworkTestsApp3PreT.apk"; - private static final String TEST_PKG = "com.android.cts.net.hostside.app3"; - private static final String TEST_CLASS = ".ExcludedRoutesGatingTest"; - - private static final long EXCLUDED_ROUTES_CHANGE_ID = 186082280; - - protected void tearDown() throws Exception { - uninstallPackage(TEST_PKG, true); - } - - public void testExcludedRoutesChangeEnabled() throws Exception { - installPackage(TEST_APK, true); - runDeviceCompatTest("testExcludedRoutesChangeEnabled"); - } - - public void testExcludedRoutesChangeDisabledPreT() throws Exception { - installPackage(TEST_APK_PRE_T, true); - runDeviceCompatTest("testExcludedRoutesChangeDisabled"); - } - - public void testExcludedRoutesChangeDisabledByOverrideOnDebugBuild() throws Exception { - // Must install APK even when skipping test, because tearDown expects uninstall to succeed. - installPackage(TEST_APK, true); - - // This test uses an app with a target SDK where the compat change is on by default. - // Because user builds do not allow overriding compat changes, only run this test on debug - // builds. This seems better than deleting this test and not running it anywhere because we - // could in the future run this test on userdebug builds in presubmit. - // - // We cannot use assumeXyz here because CompatChangeGatingTestCase ultimately inherits from - // junit.framework.TestCase, which does not understand assumption failures. - if ("user".equals(getDevice().getProperty("ro.build.type"))) return; - - runDeviceCompatTestWithChangeDisabled("testExcludedRoutesChangeDisabled"); - } - - public void testExcludedRoutesChangeEnabledByOverridePreT() throws Exception { - installPackage(TEST_APK_PRE_T, true); - runDeviceCompatTestWithChangeEnabled("testExcludedRoutesChangeEnabled"); - } - - private void runDeviceCompatTest(String methodName) throws Exception { - runDeviceCompatTest(TEST_PKG, TEST_CLASS, methodName, Set.of(), Set.of()); - } - - private void runDeviceCompatTestWithChangeEnabled(String methodName) throws Exception { - runDeviceCompatTest(TEST_PKG, TEST_CLASS, methodName, Set.of(EXCLUDED_ROUTES_CHANGE_ID), - Set.of()); - } - - private void runDeviceCompatTestWithChangeDisabled(String methodName) throws Exception { - runDeviceCompatTest(TEST_PKG, TEST_CLASS, methodName, Set.of(), - Set.of(EXCLUDED_ROUTES_CHANGE_ID)); - } -}