From f39108b6efc899980435c379909e9b8b7fdf52f2 Mon Sep 17 00:00:00 2001
From: Chris Forbes <chrisforbes@google.com>
Date: Tue, 20 Nov 2018 14:56:46 -0800
Subject: [PATCH] Move APK install/uninstall to tradefed target_preparer

We shouldn't be doing this by hand with a modern tradefed. The previous
implementation was also insanely wasteful -- we'd uninstall and
reinstall the target package every 1K tests.

V3: Adjust testrunner unit tests to not expect installs/uninstalls to
happen
V4: Remove time claim. There is still a significant pause between batches.

Change-Id: Ida67984ce45e60eb025fabfd997c7a9015496333
(cherry picked from commit c2b068c5a09f883f3a6af0e7d4aff639e8f8a070)
Merged-In: Ida67984ce45e60eb025fabfd997c7a9015496333
---
 android/cts/AndroidTest.xml                        |   4 +
 .../drawelements/deqp/runner/DeqpTestRunner.java   |  30 -----
 .../deqp/runner/DeqpTestRunnerTest.java            | 132 ---------------------
 scripts/mustpass.py                                |   6 +
 4 files changed, 10 insertions(+), 162 deletions(-)

diff --git a/android/cts/AndroidTest.xml b/android/cts/AndroidTest.xml
index c628c37..6ae5b15 100644
--- a/android/cts/AndroidTest.xml
+++ b/android/cts/AndroidTest.xml
@@ -18,6 +18,10 @@
 	<!--
      This file has been automatically generated. Edit with caution.
      -->
+	<target_preparer class="com.android.tradefed.targetprep.suite.SuiteApkInstaller">
+		<option name="cleanup-apks" value="true"/>
+		<option name="test-file-name" value="com.drawelements.deqp.apk"/>
+	</target_preparer>
 	<option name="test-suite-tag" value="cts"/>
 	<option key="component" name="config-descriptor:metadata" value="deqp"/>
 	<test class="com.drawelements.deqp.runner.DeqpTestRunner">
diff --git a/android/cts/runner/src/com/drawelements/deqp/runner/DeqpTestRunner.java b/android/cts/runner/src/com/drawelements/deqp/runner/DeqpTestRunner.java
index a590e1f..1e239aa 100644
--- a/android/cts/runner/src/com/drawelements/deqp/runner/DeqpTestRunner.java
+++ b/android/cts/runner/src/com/drawelements/deqp/runner/DeqpTestRunner.java
@@ -1768,29 +1768,6 @@ public class DeqpTestRunner implements IBuildReceiver, IDeviceTest,
     }
 
     /**
-     * Install dEQP OnDevice Package
-     */
-    private void installTestApk() throws DeviceNotAvailableException {
-        try {
-            File apkFile = new File(mBuildHelper.getTestsDir(), DEQP_ONDEVICE_APK);
-            String[] options = {AbiUtils.createAbiFlag(mAbi.getName())};
-            String errorCode = getDevice().installPackage(apkFile, true, options);
-            if (errorCode != null) {
-                CLog.e("Failed to install %s. Reason: %s", DEQP_ONDEVICE_APK, errorCode);
-            }
-        } catch (FileNotFoundException e) {
-            CLog.e("Could not find test apk %s", DEQP_ONDEVICE_APK);
-        }
-    }
-
-    /**
-     * Uninstall dEQP OnDevice Package
-     */
-    private void uninstallTestApk() throws DeviceNotAvailableException {
-        getDevice().uninstallPackage(DEQP_ONDEVICE_PKG);
-    }
-
-    /**
      * Parse gl nature from package name
      */
     private boolean isOpenGlEsPackage() {
@@ -2061,22 +2038,15 @@ public class DeqpTestRunner implements IBuildReceiver, IDeviceTest,
                 // the names of the tests only
                 fakePassTests(listener);
             } else if (!mRemainingTests.isEmpty()) {
-                // Make sure there is no pre-existing package form earlier interrupted test run.
-                uninstallTestApk();
-                installTestApk();
-
                 mInstanceListerner.setSink(listener);
                 mDeviceRecovery.setDevice(mDevice);
                 runTests();
-
-                uninstallTestApk();
             }
         } catch (CapabilityQueryFailureException ex) {
             // Platform is not behaving correctly, for example crashing when trying to create
             // a window. Instead of silenty failing, signal failure by leaving the rest of the
             // test cases in "NotExecuted" state
             CLog.e("Capability query failed - leaving tests unexecuted.");
-            uninstallTestApk();
         } finally {
             listener.testRunEnded(System.currentTimeMillis() - startTime, emptyMap);
         }
diff --git a/android/cts/runner/tests/src/com/drawelements/deqp/runner/DeqpTestRunnerTest.java b/android/cts/runner/tests/src/com/drawelements/deqp/runner/DeqpTestRunnerTest.java
index 83d1ca6..06e5b0c 100644
--- a/android/cts/runner/tests/src/com/drawelements/deqp/runner/DeqpTestRunnerTest.java
+++ b/android/cts/runner/tests/src/com/drawelements/deqp/runner/DeqpTestRunnerTest.java
@@ -224,13 +224,6 @@ public class DeqpTestRunnerTest extends TestCase {
         if (majorVersion > requiredMajorVersion
                 || (majorVersion == requiredMajorVersion && minorVersion >= requiredMinorVersion)) {
 
-            EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG)))
-                    .andReturn("").once();
-            EasyMock.expect(mockDevice.installPackage(EasyMock.<File>anyObject(),
-                    EasyMock.eq(true),
-                    EasyMock.eq(AbiUtils.createAbiFlag(ABI.getName()))))
-                    .andReturn(null).once();
-
             expectRenderConfigQuery(mockDevice, requiredMajorVersion,
                     requiredMinorVersion);
 
@@ -244,9 +237,6 @@ public class DeqpTestRunnerTest extends TestCase {
 
             runInstrumentationLineAndAnswer(mockDevice, mockIDevice, testTrie, commandLine,
                     output);
-
-            EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG)))
-                    .andReturn("").once();
         }
 
         mockListener.testRunStarted(getTestId(deqpTest), 1);
@@ -363,13 +353,6 @@ public class DeqpTestRunnerTest extends TestCase {
         EasyMock.expect(mockDevice.getProperty("ro.opengles.version"))
                 .andReturn(Integer.toString(version)).atLeastOnce();
 
-        EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG))).andReturn("")
-                .once();
-
-        EasyMock.expect(mockDevice.installPackage(EasyMock.<File>anyObject(),
-                EasyMock.eq(true), EasyMock.eq(AbiUtils.createAbiFlag(ABI.getName()))))
-                .andReturn(null).once();
-
         expectRenderConfigQuery(mockDevice, 3, 0);
 
         String commandLine = String.format(
@@ -402,9 +385,6 @@ public class DeqpTestRunnerTest extends TestCase {
         mockListener.testRunEnded(EasyMock.anyLong(), EasyMock.<Map<String, String>>notNull());
         EasyMock.expectLastCall().once();
 
-        EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG))).andReturn("")
-                .once();
-
         EasyMock.replay(mockDevice, mockIDevice);
         EasyMock.replay(mockListener);
 
@@ -530,12 +510,6 @@ public class DeqpTestRunnerTest extends TestCase {
         EasyMock.expect(mockDevice.getProperty("ro.opengles.version"))
                 .andReturn(Integer.toString(version)).atLeastOnce();
 
-        EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG))).andReturn("")
-                .once();
-        EasyMock.expect(mockDevice.installPackage(EasyMock.<File>anyObject(),
-                EasyMock.eq(true), EasyMock.eq(AbiUtils.createAbiFlag(ABI.getName()))))
-                .andReturn(null).once();
-
         expectRenderConfigQuery(mockDevice, 3, 0);
 
         String commandLine = String.format(
@@ -564,9 +538,6 @@ public class DeqpTestRunnerTest extends TestCase {
         mockListener.testRunEnded(EasyMock.anyLong(), EasyMock.<Map<String, String>>notNull());
         EasyMock.expectLastCall().once();
 
-        EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG))).andReturn("")
-                .once();
-
         EasyMock.replay(mockDevice, mockIDevice);
         EasyMock.replay(mockListener);
 
@@ -628,15 +599,6 @@ public class DeqpTestRunnerTest extends TestCase {
                 .andReturn(Integer.toString(version)).atLeastOnce();
 
         boolean thereAreTests = !expectedTests.isEmpty();
-        if (thereAreTests)
-        {
-            // only expect to install/uninstall packages if there are any tests
-            EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG))).andReturn("")
-                .once();
-            EasyMock.expect(mockDevice.installPackage(EasyMock.<File>anyObject(),
-                EasyMock.eq(true), EasyMock.eq(AbiUtils.createAbiFlag(ABI.getName()))))
-                .andReturn(null).once();
-        }
 
         ITestInvocationListener mockListener
                 = EasyMock.createStrictMock(ITestInvocationListener.class);
@@ -665,13 +627,6 @@ public class DeqpTestRunnerTest extends TestCase {
         mockListener.testRunEnded(EasyMock.anyLong(), EasyMock.<Map<String, String>>notNull());
         EasyMock.expectLastCall().once();
 
-        if (thereAreTests)
-        {
-            // package will only be installed if there are tests to run
-            EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG))).andReturn("")
-                .once();
-        }
-
         EasyMock.replay(mockDevice, mockIDevice);
         EasyMock.replay(mockListener);
 
@@ -872,12 +827,6 @@ public class DeqpTestRunnerTest extends TestCase {
         EasyMock.expect(mockDevice.getProperty("ro.opengles.version"))
                 .andReturn(Integer.toString(version)).atLeastOnce();
 
-        EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG))).andReturn("")
-                .once();
-        EasyMock.expect(mockDevice.installPackage(EasyMock.<File>anyObject(),
-                EasyMock.eq(true), EasyMock.eq(AbiUtils.createAbiFlag(ABI.getName()))))
-                .andReturn(null).once();
-
         expectRenderConfigQuery(mockDevice, 3, 0);
 
         String commandLine = String.format(
@@ -922,9 +871,6 @@ public class DeqpTestRunnerTest extends TestCase {
         mockListener.testRunEnded(EasyMock.anyLong(), EasyMock.<Map<String, String>>notNull());
         EasyMock.expectLastCall().once();
 
-        EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG))).andReturn("")
-                .once();
-
         EasyMock.replay(mockDevice, mockIDevice);
         EasyMock.replay(mockListener);
 
@@ -962,17 +908,6 @@ public class DeqpTestRunnerTest extends TestCase {
         EasyMock.expect(mockDevice.executeShellCommand("pm list features"))
                 .andReturn("not a valid format");
 
-        EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG))).
-            andReturn("").once();
-
-        EasyMock.expect(mockDevice.installPackage(EasyMock.<File>anyObject(),
-                EasyMock.eq(true),
-                EasyMock.eq(AbiUtils.createAbiFlag(ABI.getName())))).andReturn(null)
-                .once();
-
-        EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG)))
-                .andReturn("").once();
-
         mockListener.testRunStarted(getTestId(deqpTest), 1);
         EasyMock.expectLastCall().once();
 
@@ -1008,14 +943,6 @@ public class DeqpTestRunnerTest extends TestCase {
         EasyMock.expect(mockDevice.getProperty("ro.opengles.version"))
                 .andReturn(Integer.toString(version)).atLeastOnce();
 
-        EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG))).
-            andReturn("").once();
-
-        EasyMock.expect(mockDevice.installPackage(EasyMock.<File>anyObject(),
-                EasyMock.eq(true),
-                EasyMock.eq(AbiUtils.createAbiFlag(ABI.getName())))).andReturn(null)
-                .once();
-
         expectRenderConfigQueryAndReturn(mockDevice,
                 "--deqp-gl-config-name=rgba8888d24s8 "
                 + "--deqp-screen-rotation=unspecified "
@@ -1023,9 +950,6 @@ public class DeqpTestRunnerTest extends TestCase {
                 + "--deqp-gl-major-version=3 "
                 + "--deqp-gl-minor-version=0", "Maybe?");
 
-        EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG)))
-                .andReturn("").once();
-
         mockListener.testRunStarted(getTestId(deqpTest), 1);
         EasyMock.expectLastCall().once();
 
@@ -1110,14 +1034,6 @@ public class DeqpTestRunnerTest extends TestCase {
                 (isLandscapeOrientation &&
                 featureString.contains(DeqpTestRunner.FEATURE_LANDSCAPE));
 
-        EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG))).
-            andReturn("").once();
-
-        EasyMock.expect(mockDevice.installPackage(EasyMock.<File>anyObject(),
-                EasyMock.eq(true),
-                EasyMock.eq(AbiUtils.createAbiFlag(ABI.getName())))).andReturn(null)
-                .once();
-
         if (executable) {
             expectRenderConfigQuery(mockDevice, String.format(
                     "--deqp-gl-config-name=rgba8888d24s8 --deqp-screen-rotation=%s "
@@ -1136,9 +1052,6 @@ public class DeqpTestRunnerTest extends TestCase {
                     output);
         }
 
-        EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG)))
-                .andReturn("").once();
-
         mockListener.testRunStarted(getTestId(deqpTest), 1);
         EasyMock.expectLastCall().once();
 
@@ -1342,23 +1255,12 @@ public class DeqpTestRunnerTest extends TestCase {
         EasyMock.expect(mockDevice.getProperty("ro.opengles.version"))
                 .andReturn(Integer.toString(version)).atLeastOnce();
 
-        EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG))).
-            andReturn("").once();
-
-        EasyMock.expect(mockDevice.installPackage(EasyMock.<File>anyObject(),
-                EasyMock.eq(true),
-                EasyMock.eq(AbiUtils.createAbiFlag(ABI.getName())))).andReturn(null)
-                .once();
-
         expectRenderConfigQueryAndReturn(mockDevice, String.format(
                 "--deqp-gl-config-name=%s --deqp-screen-rotation=unspecified "
                 + "--deqp-surface-type=window "
                 + "--deqp-gl-major-version=3 "
                 + "--deqp-gl-minor-version=0", pixelFormat), "No");
 
-        EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG)))
-                .andReturn("").once();
-
         mockListener.testRunStarted(getTestId(deqpTest), 1);
         EasyMock.expectLastCall().once();
 
@@ -1709,14 +1611,6 @@ public class DeqpTestRunnerTest extends TestCase {
         EasyMock.expect(mockDevice.getProperty("ro.opengles.version"))
                 .andReturn(Integer.toString(version)).atLeastOnce();
 
-        EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG))).
-            andReturn("").once();
-
-        EasyMock.expect(mockDevice.installPackage(EasyMock.<File>anyObject(),
-                EasyMock.eq(true),
-                EasyMock.eq(AbiUtils.createAbiFlag(ABI.getName())))).andReturn(null)
-                .once();
-
         expectRenderConfigQuery(mockDevice,
                 "--deqp-gl-config-name=rgba8888d24s8 --deqp-screen-rotation=unspecified "
                 + "--deqp-surface-type=window --deqp-gl-major-version=3 "
@@ -1766,12 +1660,6 @@ public class DeqpTestRunnerTest extends TestCase {
             EasyMock.expect(mockDevice.getProperty("ro.opengles.version"))
                     .andReturn(Integer.toString(version)).atLeastOnce();
 
-            EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG))).andReturn("")
-                    .once();
-            EasyMock.expect(mockDevice.installPackage(EasyMock.<File>anyObject(),
-                    EasyMock.eq(true), EasyMock.eq(AbiUtils.createAbiFlag(ABI.getName()))))
-                    .andReturn(null).once();
-
             mockListener.testRunStarted(getTestId(shard), shardTests.size());
             EasyMock.expectLastCall().once();
 
@@ -1794,9 +1682,6 @@ public class DeqpTestRunnerTest extends TestCase {
             mockListener.testRunEnded(EasyMock.anyLong(), EasyMock.<Map<String, String>>notNull());
             EasyMock.expectLastCall().once();
 
-            EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG))).andReturn("")
-                    .once();
-
             EasyMock.replay(mockDevice, mockIDevice);
             EasyMock.replay(mockListener);
 
@@ -1907,14 +1792,6 @@ public class DeqpTestRunnerTest extends TestCase {
         EasyMock.expect(mockDevice.getProperty("ro.opengles.version"))
                 .andReturn(Integer.toString(version)).atLeastOnce();
 
-        EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG))).
-            andReturn("").once();
-
-        EasyMock.expect(mockDevice.installPackage(EasyMock.<File>anyObject(),
-                EasyMock.eq(true),
-                EasyMock.eq(AbiUtils.createAbiFlag(ABI.getName())))).andReturn(null)
-                .once();
-
         expectRenderConfigQuery(mockDevice,
                 "--deqp-gl-config-name=rgba8888d24s8 --deqp-screen-rotation=unspecified "
                 + "--deqp-surface-type=window --deqp-gl-major-version=3 "
@@ -2078,12 +1955,6 @@ public class DeqpTestRunnerTest extends TestCase {
         EasyMock.expect(mockDevice.getProperty("ro.opengles.version"))
                 .andReturn(Integer.toString(version)).atLeastOnce();
 
-        EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG))).andReturn("")
-                .once();
-        EasyMock.expect(mockDevice.installPackage(EasyMock.<File>anyObject(),
-                EasyMock.eq(true), EasyMock.eq(AbiUtils.createAbiFlag(ABI.getName()))))
-                .andReturn(null).once();
-
         expectRenderConfigQuery(mockDevice, 3, 0);
 
         String commandLine = String.format(
@@ -2112,9 +1983,6 @@ public class DeqpTestRunnerTest extends TestCase {
         mockListener.testRunEnded(EasyMock.anyLong(), EasyMock.<Map<String, String>>notNull());
         EasyMock.expectLastCall().once();
 
-        EasyMock.expect(mockDevice.uninstallPackage(EasyMock.eq(DEQP_ONDEVICE_PKG))).andReturn("")
-                .once();
-
         EasyMock.replay(mockDevice, mockIDevice);
         EasyMock.replay(mockListener);
 
diff --git a/scripts/mustpass.py b/scripts/mustpass.py
index 34e11f1..49c6baa 100644
--- a/scripts/mustpass.py
+++ b/scripts/mustpass.py
@@ -305,6 +305,12 @@ def genAndroidTestXml (mustpass):
 	RUNNER_CLASS = "com.drawelements.deqp.runner.DeqpTestRunner"
 	configElement = ElementTree.Element("configuration")
 
+	# have the deqp package installed on the device for us
+	preparerElement = ElementTree.SubElement(configElement, "target_preparer")
+	preparerElement.set("class", "com.android.tradefed.targetprep.suite.SuiteApkInstaller")
+	addOptionElement(preparerElement, "cleanup-apks", "true")
+	addOptionElement(preparerElement, "test-file-name", "com.drawelements.deqp.apk")
+
 	# add in metadata option for component name
 	ElementTree.SubElement(configElement, "option", name="test-suite-tag", value="cts")
 	ElementTree.SubElement(configElement, "option", name="config-descriptor:metadata", key="component", value="deqp")
-- 
2.7.4