From db6d1df7916da1cb000d8441b39e57b24c4f0893 Mon Sep 17 00:00:00 2001 From: Winson Date: Tue, 23 Jun 2020 07:41:25 -0700 Subject: [PATCH] Fix PackageManagerServiceHostTests disk usage It seems adb shell stop/start has a bug with taking up disk space. For now, use a full reboot of the device for each test step. This will double the already extremely long test time, so the entire PackageManagerServiceHostTests module has been moved to postsubmit, except for tests annotated @Presubmit, of which there are none as of this change. Bug: 159540015 Bug: 159256824 Test: atest PackageManagerServiceHostTests Change-Id: I67da61cb02baa572fc298e6f617d6e53ec2c4724 --- .../java/com/android/server/pm/TEST_MAPPING | 10 +++++- .../tests/PackageManagerServiceTests/OWNERS | 3 ++ .../com/android/server/pm/test/HostUtils.kt | 15 ++++++--- .../server/pm/test/InvalidNewSystemAppTest.kt | 15 ++++----- .../pm/test/OriginalPackageMigrationTest.kt | 31 ++++++++++++------- .../com/android/server/pm/test/Partition.kt | 2 +- .../internal/util/test/SystemPreparer.java | 3 ++ 7 files changed, 54 insertions(+), 25 deletions(-) create mode 100644 services/tests/PackageManagerServiceTests/OWNERS diff --git a/services/core/java/com/android/server/pm/TEST_MAPPING b/services/core/java/com/android/server/pm/TEST_MAPPING index eb51cc3cd25ca..4b83d70ea34ee 100644 --- a/services/core/java/com/android/server/pm/TEST_MAPPING +++ b/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": [ diff --git a/services/tests/PackageManagerServiceTests/OWNERS b/services/tests/PackageManagerServiceTests/OWNERS new file mode 100644 index 0000000000000..182dfe8fca9e2 --- /dev/null +++ b/services/tests/PackageManagerServiceTests/OWNERS @@ -0,0 +1,3 @@ +chiuwinson@google.com +patb@google.com +toddke@google.com diff --git a/services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/HostUtils.kt b/services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/HostUtils.kt index 490f96d8f4263..234fcf19062df 100644 --- a/services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/HostUtils.kt +++ b/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 -> diff --git a/services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/InvalidNewSystemAppTest.kt b/services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/InvalidNewSystemAppTest.kt index 98e045d0a203b..39b40d8d3195b 100644 --- a/services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/InvalidNewSystemAppTest.kt +++ b/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 diff --git a/services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/OriginalPackageMigrationTest.kt b/services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/OriginalPackageMigrationTest.kt index 90494c5913633..fb0348c3146b3 100644 --- a/services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/OriginalPackageMigrationTest.kt +++ b/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()) + } } diff --git a/services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/Partition.kt b/services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/Partition.kt index 35192a73cedac..654c11c5bf816 100644 --- a/services/tests/PackageManagerServiceTests/host/src/com/android/server/pm/test/Partition.kt +++ b/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"), diff --git a/tests/utils/hostutils/src/com/android/internal/util/test/SystemPreparer.java b/tests/utils/hostutils/src/com/android/internal/util/test/SystemPreparer.java index 6bd6985f96750..f30c35aca8da6 100644 --- a/tests/utils/hostutils/src/com/android/internal/util/test/SystemPreparer.java +++ b/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 }