Merge "Fix PackageManagerServiceHostTests disk usage" into rvc-dev

services/core/java/com/android/server/pm/TEST_MAPPING

@@ -71,7 +71,12 @@
       ]
     },
     {
-      "name": "PackageManagerServiceHostTests"
+      "name": "PackageManagerServiceHostTests",
+      "options": [
+        {
+          "include-annotation": "android.platform.test.annotations.Presubmit"
+        }
+      ]
     }
   ],
   "postsubmit": [
@@ -86,6 +91,9 @@
     {
       "name": "CtsAppSecurityHostTestCases"
     },
+    {
+      "name": "PackageManagerServiceHostTests"
+    },
     {
       "name": "FrameworksServicesTests",
       "options": [

services/tests/PackageManagerServiceTests/OWNERS

@@ -0,0 +1,3 @@
+chiuwinson@google.com
+patb@google.com
+toddke@google.com

services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/HostUtils.kt

@@ -22,10 +22,16 @@ import java.io.File
 import java.io.FileOutputStream
 
 internal fun SystemPreparer.pushApk(file: String, partition: Partition) =
-        pushResourceFile(file, HostUtils.makePathForApk(file, partition))
+        pushResourceFile(file, HostUtils.makePathForApk(file, partition).toString())
 
-internal fun SystemPreparer.deleteApk(file: String, partition: Partition) =
-        deleteFile(partition.baseFolder.resolve(file.removeSuffix(".apk")).toString())
+internal fun SystemPreparer.deleteApkFolders(
+    partition: Partition,
+    vararg javaResourceNames: String
+) = apply {
+    javaResourceNames.forEach {
+        deleteFile(partition.baseAppFolder.resolve(it.removeSuffix(".apk")).toString())
+    }
+}
 
 internal object HostUtils {
 
@@ -40,10 +46,9 @@ internal object HostUtils {
             makePathForApk(File(fileName), partition)
 
     fun makePathForApk(file: File, partition: Partition) =
-            partition.baseFolder
+            partition.baseAppFolder
                     .resolve(file.nameWithoutExtension)
                     .resolve(file.name)
-                    .toString()
 
     fun copyResourceToHostFile(javaResourceName: String, file: File): File {
         javaClass.classLoader!!.getResource(javaResourceName).openStream().use { input ->

services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/InvalidNewSystemAppTest.kt

@@ -45,23 +45,24 @@ class InvalidNewSystemAppTest : BaseHostJUnit4Test() {
 
     private val tempFolder = TemporaryFolder()
     private val preparer: SystemPreparer = SystemPreparer(tempFolder,
-            SystemPreparer.RebootStrategy.START_STOP, deviceRebootRule) { this.device }
+            SystemPreparer.RebootStrategy.FULL, deviceRebootRule) { this.device }
 
     @get:Rule
     val rules = RuleChain.outerRule(tempFolder).around(preparer)!!
+    private val filePath = HostUtils.makePathForApk("PackageManagerDummyApp.apk", Partition.PRODUCT)
 
     @Before
     @After
-    fun uninstallDataPackage() {
+    fun removeApk() {
         device.uninstallPackage(TEST_PKG_NAME)
+        device.deleteFile(filePath.parent.toString())
+        device.reboot()
     }
 
     @Test
     fun verify() {
         // First, push a system app to the device and then update it so there's a data variant
-        val filePath = HostUtils.makePathForApk("PackageManagerDummyApp.apk", Partition.PRODUCT)
-
-        preparer.pushResourceFile(VERSION_ONE, filePath)
+        preparer.pushResourceFile(VERSION_ONE, filePath.toString())
                 .reboot()
 
         val versionTwoFile = HostUtils.copyResourceToHostFile(VERSION_TWO, tempFolder.newFile())
@@ -69,8 +70,8 @@ class InvalidNewSystemAppTest : BaseHostJUnit4Test() {
         assertThat(device.installPackage(versionTwoFile, true)).isNull()
 
         // Then push a bad update to the system, overwriting the existing file as if an OTA occurred
-        preparer.deleteFile(filePath)
-                .pushResourceFile(VERSION_THREE_INVALID, filePath)
+        preparer.deleteFile(filePath.toString())
+                .pushResourceFile(VERSION_THREE_INVALID, filePath.toString())
                 .reboot()
 
         // This will remove the package from the device, which is expected

services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/OriginalPackageMigrationTest.kt

@@ -20,6 +20,8 @@ import com.android.internal.util.test.SystemPreparer
 import com.android.tradefed.testtype.DeviceJUnit4ClassRunner
 import com.android.tradefed.testtype.junit4.BaseHostJUnit4Test
 import com.google.common.truth.Truth.assertThat
+import org.junit.After
+import org.junit.Before
 import org.junit.ClassRule
 import org.junit.Rule
 import org.junit.Test
@@ -43,11 +45,18 @@ class OriginalPackageMigrationTest : BaseHostJUnit4Test() {
 
     private val tempFolder = TemporaryFolder()
     private val preparer: SystemPreparer = SystemPreparer(tempFolder,
-            SystemPreparer.RebootStrategy.START_STOP, deviceRebootRule) { this.device }
+            SystemPreparer.RebootStrategy.FULL, deviceRebootRule) { this.device }
 
     @get:Rule
     val rules = RuleChain.outerRule(tempFolder).around(preparer)!!
 
+    @Before
+    @After
+    fun deleteApkFolders() {
+        preparer.deleteApkFolders(Partition.SYSTEM, VERSION_ONE, VERSION_TWO, VERSION_THREE,
+                NEW_PKG)
+    }
+
     @Test
     fun lowerVersion() {
         runForApk(VERSION_ONE)
@@ -71,28 +80,28 @@ class OriginalPackageMigrationTest : BaseHostJUnit4Test() {
         preparer.pushApk(apk, Partition.SYSTEM)
                 .reboot()
 
-        device.getAppPackageInfo(TEST_PKG_NAME).run {
-            assertThat(codePath).contains(apk.removeSuffix(".apk"))
-        }
+        assertCodePath(apk)
 
         // Ensure data is preserved by writing to the original dataDir
         val file = tempFolder.newFile().apply { writeText("Test") }
         device.pushFile(file, "${HostUtils.getDataDir(device, TEST_PKG_NAME)}/files/test.txt")
 
-        preparer.deleteApk(apk, Partition.SYSTEM)
+        preparer.deleteApkFolders(Partition.SYSTEM, apk)
                 .pushApk(NEW_PKG, Partition.SYSTEM)
                 .reboot()
 
-        device.getAppPackageInfo(TEST_PKG_NAME)
-                .run {
-                    assertThat(this.toString()).isNotEmpty()
-                    assertThat(codePath)
-                            .contains(NEW_PKG.removeSuffix(".apk"))
-                }
+        assertCodePath(NEW_PKG)
 
         // And then reading the data contents back
         assertThat(device.pullFileContents(
                 "${HostUtils.getDataDir(device, TEST_PKG_NAME)}/files/test.txt"))
                 .isEqualTo("Test")
     }
+
+    private fun assertCodePath(apk: String) {
+        // dumpsys package and therefore device.getAppPackageInfo doesn't work here for some reason,
+        // so parse the package dump directly to see if the path matches.
+        assertThat(device.executeShellCommand("pm dump $TEST_PKG_NAME"))
+                .contains(HostUtils.makePathForApk(apk, Partition.SYSTEM).parent.toString())
+    }
 }

services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/Partition.kt

@@ -20,7 +20,7 @@ import java.nio.file.Path
 import java.nio.file.Paths
 
 // Unfortunately no easy way to access PMS SystemPartitions, so mock them here
-internal enum class Partition(val baseFolder: Path) {
+internal enum class Partition(val baseAppFolder: Path) {
     SYSTEM("/system/app"),
     VENDOR("/vendor/app"),
     PRODUCT("/product/app"),

tests/utils/hostutils/src/com/android/internal/util/test/SystemPreparer.java

@@ -356,6 +356,9 @@ public class SystemPreparer extends ExternalResource {
         /**
          * Uses shell stop && start to "reboot" the device. May leave invalid state after each test.
          * Whether this matters or not depends on what's being tested.
+         *
+         * TODO(b/159540015): There's a bug with this causing unnecessary disk space usage, which
+         *  can eventually lead to an insufficient storage space error.
          */
         START_STOP
     }