From e59f5e36e57c2547273f716cd5902a473e09f90d Mon Sep 17 00:00:00 2001 From: markchien Date: Sat, 2 Apr 2022 00:45:53 +0800 Subject: [PATCH] ConnectivityCoverageTests refactoring ConnectivityCoverageTests is a combination of several test libs, which are jarjared differently. It causes duplicated classes not to be counted for coverage properly. Build the test suite directly and apply jarjar only once on top of everything. Bug: 227694415 Test: atest ConnectivityCoverageTests Change-Id: I4350ebdbf98030944ec3857e1ef67d76f26c3b16 --- TEST_MAPPING | 3 - Tethering/tests/integration/Android.bp | 65 ------------------- .../integration/AndroidManifest_coverage.xml | 29 --------- .../integration/AndroidTest_Coverage.xml | 13 ---- Tethering/tests/jarjar-rules.txt | 2 + tests/common/Android.bp | 27 ++++++-- tests/common/tethering-jni-jarjar-rules.txt | 10 +++ 7 files changed, 33 insertions(+), 116 deletions(-) delete mode 100644 Tethering/tests/integration/AndroidManifest_coverage.xml delete mode 100644 Tethering/tests/integration/AndroidTest_Coverage.xml create mode 100644 tests/common/tethering-jni-jarjar-rules.txt diff --git a/TEST_MAPPING b/TEST_MAPPING index 7fa4b7f178..95f854bf14 100644 --- a/TEST_MAPPING +++ b/TEST_MAPPING @@ -137,9 +137,6 @@ "exclude-annotation": "androidx.test.filters.RequiresDevice" } ] - }, - { - "name": "TetheringCoverageTests[CaptivePortalLoginGoogle.apk+NetworkStackGoogle.apk+com.google.android.resolv.apex+com.google.android.tethering.apex]" } ], "auto-postsubmit": [ diff --git a/Tethering/tests/integration/Android.bp b/Tethering/tests/integration/Android.bp index 6eaf68bc7e..a4d044849f 100644 --- a/Tethering/tests/integration/Android.bp +++ b/Tethering/tests/integration/Android.bp @@ -83,68 +83,3 @@ android_test { compile_multilib: "both", jarjar_rules: ":NetworkStackJarJarRules", } - -android_library { - name: "TetheringCoverageTestsLib", - min_sdk_version: "30", - static_libs: [ - "NetdStaticLibTestsLib", - "NetworkStaticLibTestsLib", - "NetworkStackTestsLib", - "TetheringTestsLatestSdkLib", - "TetheringIntegrationTestsLatestSdkLib", - ], - // Jarjar rules should normally be applied on final artifacts and not intermediate libraries as - // applying different rules on intermediate libraries can cause conflicts when combining them - // (the resulting artifact can end up with multiple incompatible implementations of the same - // classes). But this library is used to combine tethering coverage tests with connectivity - // coverage tests into a single coverage target. The tests need to use the same jarjar rules as - // covered production code for coverage to be calculated properly, so jarjar is applied - // separately on each set of tests. - jarjar_rules: ":TetheringCoverageJarJarRules", - manifest: "AndroidManifest_coverage.xml", - visibility: [ - "//packages/modules/Connectivity/tests:__subpackages__" - ], -} - -// Combine NetworkStack and Tethering jarjar rules for coverage target. The jarjar files are -// simply concatenated in the order specified in srcs. -genrule { - name: "TetheringCoverageJarJarRules", - srcs: [ - ":TetheringTestsJarJarRules", - ":NetworkStackJarJarRules", - ], - out: ["jarjar-rules-tethering-coverage.txt"], - cmd: "cat $(in) > $(out)", - visibility: ["//visibility:private"], -} - -// Special version of the tethering tests that includes all tests necessary for code coverage -// purposes. This is currently the union of TetheringTests, TetheringIntegrationTests and -// NetworkStackTests. -// TODO: remove in favor of ConnectivityCoverageTests, which includes below tests and more -android_test { - name: "TetheringCoverageTests", - platform_apis: true, - min_sdk_version: "30", - target_sdk_version: "31", - test_suites: ["device-tests", "mts-tethering"], - test_config: "AndroidTest_Coverage.xml", - defaults: ["libnetworkstackutilsjni_deps"], - static_libs: [ - "modules-utils-native-coverage-listener", - "TetheringCoverageTestsLib", - ], - jni_libs: [ - // For mockito extended - "libdexmakerjvmtiagent", - "libstaticjvmtiagent", - // For NetworkStackUtils included in NetworkStackBase - "libnetworkstackutilsjni", - "libcom_android_networkstack_tethering_util_jni", - ], - compile_multilib: "both", - manifest: "AndroidManifest_coverage.xml", -} diff --git a/Tethering/tests/integration/AndroidManifest_coverage.xml b/Tethering/tests/integration/AndroidManifest_coverage.xml deleted file mode 100644 index 06de00d785..0000000000 --- a/Tethering/tests/integration/AndroidManifest_coverage.xml +++ /dev/null @@ -1,29 +0,0 @@ - - - - - - - - - - diff --git a/Tethering/tests/integration/AndroidTest_Coverage.xml b/Tethering/tests/integration/AndroidTest_Coverage.xml deleted file mode 100644 index 33c5b3d8ec..0000000000 --- a/Tethering/tests/integration/AndroidTest_Coverage.xml +++ /dev/null @@ -1,13 +0,0 @@ - - - - - diff --git a/Tethering/tests/jarjar-rules.txt b/Tethering/tests/jarjar-rules.txt index a7c7488e94..cd8fd3a0d7 100644 --- a/Tethering/tests/jarjar-rules.txt +++ b/Tethering/tests/jarjar-rules.txt @@ -7,6 +7,8 @@ rule com.android.internal.util.MessageUtils* com.android.networkstack.tethering. rule com.android.internal.util.State* com.android.networkstack.tethering.util.State@1 rule com.android.internal.util.StateMachine* com.android.networkstack.tethering.util.StateMachine@1 rule com.android.internal.util.TrafficStatsConstants* com.android.networkstack.tethering.util.TrafficStatsConstants@1 +# Keep other com.android.internal.util as-is +rule com.android.internal.util.** @0 rule android.util.LocalLog* com.android.networkstack.tethering.util.LocalLog@1 diff --git a/tests/common/Android.bp b/tests/common/Android.bp index b23074db09..7bb7cb562b 100644 --- a/tests/common/Android.bp +++ b/tests/common/Android.bp @@ -43,10 +43,23 @@ java_library { ], } -// Connectivity coverage tests combines Tethering and Connectivity tests, each with their -// respective jarjar rules applied. -// Some tests may be duplicated (in particular static lib tests), as they need to be run under both -// jarjared packages to cover both usages. +// Combine Connectivity, NetworkStack and Tethering jarjar rules for coverage target. +// The jarjar files are simply concatenated in the order specified in srcs. +// jarjar stops at the first matching rule, so order of concatenation affects the output. +genrule { + name: "ConnectivityCoverageJarJarRules", + srcs: [ + "tethering-jni-jarjar-rules.txt", + ":connectivity-jarjar-rules", + ":TetheringTestsJarJarRules", + ":NetworkStackJarJarRules", + ], + out: ["jarjar-rules-connectivity-coverage.txt"], + // Concat files with a line break in the middle + cmd: "for src in $(in); do cat $${src}; echo; done > $(out)", + visibility: ["//visibility:private"], +} + android_library { name: "ConnectivityCoverageTestsLib", min_sdk_version: "30", @@ -54,8 +67,11 @@ android_library { "FrameworksNetTestsLib", "NetdStaticLibTestsLib", "NetworkStaticLibTestsLib", + "NetworkStackTestsLib", + "TetheringTestsLatestSdkLib", + "TetheringIntegrationTestsLatestSdkLib", ], - jarjar_rules: ":connectivity-jarjar-rules", + jarjar_rules: ":ConnectivityCoverageJarJarRules", manifest: "AndroidManifest_coverage.xml", visibility: ["//visibility:private"], } @@ -80,7 +96,6 @@ android_test { "mockito-target-extended-minus-junit4", "modules-utils-native-coverage-listener", "ConnectivityCoverageTestsLib", - "TetheringCoverageTestsLib", ], jni_libs: [ // For mockito extended diff --git a/tests/common/tethering-jni-jarjar-rules.txt b/tests/common/tethering-jni-jarjar-rules.txt new file mode 100644 index 0000000000..593ba1497f --- /dev/null +++ b/tests/common/tethering-jni-jarjar-rules.txt @@ -0,0 +1,10 @@ +# Match the tethering jarjar rules for utils backed by +# libcom_android_networkstack_tethering_util_jni, so that this JNI library can be used as-is in the +# test. The alternative would be to build a test-specific JNI library +# (libcom_android_connectivity_tests_coverage_jni ?) that registers classes following whatever +# jarjar rules the test is using, but this is a bit less realistic (using a different JNI library), +# and complicates the test build. It would be necessary if TetheringUtils had a different package +# name in test code though, as the JNI library name is deducted from the TetheringUtils package. +rule com.android.net.module.util.BpfMap* com.android.networkstack.tethering.util.BpfMap@1 +rule com.android.net.module.util.BpfUtils* com.android.networkstack.tethering.util.BpfUtils@1 +rule com.android.net.module.util.TcUtils* com.android.networkstack.tethering.util.TcUtils@1