From 84712e991cf4767cf2826cd0bb0d251928969339 Mon Sep 17 00:00:00 2001
From: Winson <chiuwinson@google.com>
Date: Wed, 24 Jun 2020 12:12:00 -0700
Subject: [PATCH] Fix package parsing equivalence test comparisons

If an APK is skipped because of a failure in v2, but succeeds
in v1, both variants need to be ignored since there's nothing
to compare when one side fails.

This also updates SystemPartitionParseTest to support
compressed packages, decompressing them to a temporary file
for parsing.

Bug: 159806243

Test: atest com.android.server.pm.parsing

Change-Id: I93faee0f0dc6ba33bb977ad947ed6c07c0a06395
---
 .../parsing/AndroidPackageParsingTestBase.kt  | 25 +++++-----
 .../pm/parsing/SystemPartitionParseTest.kt    | 50 +++++++++++++++----
 2 files changed, 52 insertions(+), 23 deletions(-)

diff --git a/services/tests/servicestests/src/com/android/server/pm/parsing/AndroidPackageParsingTestBase.kt b/services/tests/servicestests/src/com/android/server/pm/parsing/AndroidPackageParsingTestBase.kt
index 420ff19aab74d..0f5f0af8f8388 100644
--- a/services/tests/servicestests/src/com/android/server/pm/parsing/AndroidPackageParsingTestBase.kt
+++ b/services/tests/servicestests/src/com/android/server/pm/parsing/AndroidPackageParsingTestBase.kt
@@ -96,30 +96,29 @@ open class AndroidPackageParsingTestBase {
                     anyString(), anyInt())) { true }
         }
 
-        lateinit var oldPackages: List<PackageParser.Package>
+        val oldPackages = mutableListOf<PackageParser.Package>()
 
-        lateinit var newPackages: List<AndroidPackage>
+        val newPackages = mutableListOf<AndroidPackage>()
 
         @Suppress("ConstantConditionIf")
         @JvmStatic
         @BeforeClass
         fun setUpPackages() {
-            this.oldPackages = apks.mapNotNull {
-                try {
-                    packageParser.parsePackage(it, PackageParser.PARSE_IS_SYSTEM_DIR, false)
-                } catch (ignored: Exception) {
-                    // Parsing issues will be caught by SystemPartitionParseTest
-                    null
-                }
-            }
-
-            this.newPackages = apks.mapNotNull {
+            apks.mapNotNull {
                 try {
+                    packageParser.parsePackage(it, PackageParser.PARSE_IS_SYSTEM_DIR, false) to
                     packageParser2.parsePackage(it, PackageParser.PARSE_IS_SYSTEM_DIR, false)
                 } catch (ignored: Exception) {
-                    // Parsing issues will be caught by SystemPartitionParseTest
+                    // It is intentional that a failure of either call here will result in failing
+                    // both. Having null on one side would mean nothing to compare. Due to the
+                    // nature of presubmit, this may not be caused by the change being tested, so
+                    // it's unhelpful to consider it a failure. Actual parsing issues will be
+                    // reported by SystemPartitionParseTest in postsubmit.
                     null
                 }
+            }.forEach { (old, new) ->
+                oldPackages += old
+                newPackages += new
             }
 
             if (DUMP_HPROF_TO_EXTERNAL) {
diff --git a/services/tests/servicestests/src/com/android/server/pm/parsing/SystemPartitionParseTest.kt b/services/tests/servicestests/src/com/android/server/pm/parsing/SystemPartitionParseTest.kt
index 605841df0c5a4..4cd057cb482fd 100644
--- a/services/tests/servicestests/src/com/android/server/pm/parsing/SystemPartitionParseTest.kt
+++ b/services/tests/servicestests/src/com/android/server/pm/parsing/SystemPartitionParseTest.kt
@@ -20,7 +20,11 @@ import android.content.pm.PackageManager
 import android.content.pm.PackageParser
 import android.platform.test.annotations.Postsubmit
 import com.android.server.pm.PackageManagerService
+import com.android.server.pm.PackageManagerServiceUtils
+import org.junit.Rule
 import org.junit.Test
+import org.junit.rules.TemporaryFolder
+import java.io.File
 
 /**
  * This test parses all the system APKs on the device image to ensure that they succeed.
@@ -34,20 +38,46 @@ import org.junit.Test
 @Postsubmit
 class SystemPartitionParseTest {
 
-    private val APKS = PackageManagerService.SYSTEM_PARTITIONS
-            .flatMap { listOfNotNull(it.appFolder, it.privAppFolder, it.overlayFolder) }
-            .flatMap {
-                it.walkTopDown()
-                        .filter { it.name.endsWith(".apk") }
-                        .toList()
-            }
-            .distinct()
-
     private val parser = PackageParser2.forParsingFileWithDefaults()
 
+    @get:Rule
+    val tempFolder = TemporaryFolder()
+
+    private fun buildApks(): List<File> {
+        val files = PackageManagerService.SYSTEM_PARTITIONS
+                .flatMap { listOfNotNull(it.appFolder, it.privAppFolder, it.overlayFolder) }
+                .flatMap {
+                    it.listFiles()
+                            ?.toList()
+                            ?: emptyList()
+                }
+                .distinct()
+                .toMutableList()
+
+        val compressedFiles = mutableListOf<File>()
+
+        files.removeAll { it ->
+            it.listFiles()?.toList().orEmpty()
+                    .filter { it.name.endsWith(PackageManagerService.COMPRESSED_EXTENSION) }
+                    .also { compressedFiles.addAll(it) }
+                    .isNotEmpty()
+        }
+
+        compressedFiles.mapTo(files) { input ->
+            tempFolder.newFolder()
+                    .also {
+                        // Decompress to an APK file inside the temp folder which can be tested.
+                        it.resolve(input.nameWithoutExtension + ".apk")
+                            .apply { PackageManagerServiceUtils.decompressFile(input, this) }
+                    }
+        }
+
+        return files
+    }
+
     @Test
     fun verify() {
-        val exceptions = APKS
+        val exceptions = buildApks()
                 .map {
                     runCatching {
                         parser.parsePackage(it, PackageParser.PARSE_IS_SYSTEM_DIR, false)