Fix remounting overlay tests failure

The tests were running before boot complete but required
StorageManager to finish initialization before a package could be
installed.

This change makes the test wait for the device to be available by
using reboot(). Also remove code to retrieve resource value from
target APK and use "cmd overlay lookup" to see if the resources
are overlaid.

Bug: 147660952
Test: atest OverlayRemountedTest
Change-Id: I5929d3a57ad492ce0128b4a18e636ae96982bc62
diff --git a/core/tests/overlaytests/remount/host/AndroidTest.xml b/core/tests/overlaytests/remount/host/AndroidTest.xml
index 11eadf1a..087b731 100644
--- a/core/tests/overlaytests/remount/host/AndroidTest.xml
+++ b/core/tests/overlaytests/remount/host/AndroidTest.xml
@@ -19,9 +19,6 @@
     <option name="test-tag" value="OverlayRemountedTest" />
 
     <target_preparer class="com.android.tradefed.targetprep.RootTargetPreparer"/>
-    <target_preparer class="com.android.tradefed.targetprep.RunCommandTargetPreparer">
-        <option name="run-command" value="remount" />
-    </target_preparer>
 
     <test class="com.android.tradefed.testtype.HostTest">
         <option name="jar" value="OverlayRemountedTest.jar" />
diff --git a/core/tests/overlaytests/remount/host/src/com/android/overlaytest/remounted/OverlayHostTest.java b/core/tests/overlaytests/remount/host/src/com/android/overlaytest/remounted/OverlayHostTest.java
deleted file mode 100644
index 84af187..0000000
--- a/core/tests/overlaytests/remount/host/src/com/android/overlaytest/remounted/OverlayHostTest.java
+++ /dev/null
@@ -1,114 +0,0 @@
-/*
- * Copyright (C) 2019 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package com.android.overlaytest.remounted;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-
-import com.android.tradefed.device.DeviceNotAvailableException;
-import com.android.tradefed.testtype.junit4.BaseHostJUnit4Test;
-
-import org.junit.Rule;
-import org.junit.rules.RuleChain;
-import org.junit.rules.TemporaryFolder;
-
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
-public class OverlayHostTest extends BaseHostJUnit4Test {
-    private static final long TIME_OUT_MS = 30000;
-    private static final String RES_INSTRUMENTATION_ARG = "res";
-    private static final String OVERLAY_INSTRUMENTATION_ARG = "overlays";
-    private static final String RESOURCES_TYPE_SUFFIX = "_type";
-    private static final String RESOURCES_DATA_SUFFIX = "_data";
-
-    public final TemporaryFolder mTemporaryFolder = new TemporaryFolder();
-    public final SystemPreparer mPreparer = new SystemPreparer(mTemporaryFolder, this::getDevice);
-
-    @Rule
-    public final RuleChain ruleChain = RuleChain.outerRule(mTemporaryFolder).around(mPreparer);
-    private Map<String, String> mLastResults;
-
-    /**
-     * Retrieves the values of the resources in the test package. The test package must use the
-     * {@link com.android.overlaytest.remounted.target.ResourceRetrievalRunner} instrumentation.
-     **/
-    void retrieveResource(String testPackageName, List<String> requiredOverlayPaths,
-            String... resourceNames) throws DeviceNotAvailableException {
-        final HashMap<String, String> args = new HashMap<>();
-        if (!requiredOverlayPaths.isEmpty()) {
-            // Enclose the require overlay paths in quotes so the arguments will be string arguments
-            // rather than file arguments.
-            args.put(OVERLAY_INSTRUMENTATION_ARG,
-                    String.format("\"%s\"", String.join(" ", requiredOverlayPaths)));
-        }
-
-        if (resourceNames.length == 0) {
-            throw new IllegalArgumentException("Must specify at least one resource to retrieve.");
-        }
-
-        // Pass the names of the resources to retrieve into the test as one string.
-        args.put(RES_INSTRUMENTATION_ARG,
-                String.format("\"%s\"", String.join(" ", resourceNames)));
-
-        runDeviceTests(getDevice(), null, testPackageName, null, null, null, TIME_OUT_MS,
-                TIME_OUT_MS, TIME_OUT_MS, false, false, args);
-
-        // Retrieve the results of the most recently run test.
-        mLastResults = (getLastDeviceRunResults().getRunMetrics() == mLastResults) ? null :
-                getLastDeviceRunResults().getRunMetrics();
-    }
-
-    /** Returns the base resource directories of the specified packages. */
-    List<String> getPackagePaths(String... packageNames)
-            throws DeviceNotAvailableException {
-        final ArrayList<String> paths = new ArrayList<>();
-        for (String packageName : packageNames) {
-            // Use the package manager shell command to find the path of the package.
-            final String result = getDevice().executeShellCommand(
-                    String.format("pm dump %s | grep \"resourcePath=\"", packageName));
-            assertNotNull("Failed to find path for package " + packageName, result);
-            int splitIndex = result.indexOf('=');
-            assertTrue(splitIndex >= 0);
-            paths.add(result.substring(splitIndex + 1).trim());
-        }
-        return paths;
-    }
-
-    /** Builds the full name of a resource in the form package:type/entry. */
-    String resourceName(String pkg, String type, String entry) {
-        return String.format("%s:%s/%s", pkg, type, entry);
-    }
-
-    /**
-     * Asserts that the type and data of a a previously retrieved is the same as expected.
-     * @param resourceName the full name of the resource in the form package:type/entry
-     * @param type the expected {@link android.util.TypedValue} type of the resource
-     * @param data the expected value of the resource when coerced to a string using
-     *             {@link android.util.TypedValue#coerceToString()}
-     **/
-    void assertResource(String resourceName, int type, String data) {
-        assertNotNull("Failed to get test results", mLastResults);
-        assertNotEquals("No resource values were retrieved", mLastResults.size(), 0);
-        assertEquals("" + type, mLastResults.get(resourceName + RESOURCES_TYPE_SUFFIX));
-        assertEquals("" + data, mLastResults.get(resourceName + RESOURCES_DATA_SUFFIX));
-    }
-}
diff --git a/core/tests/overlaytests/remount/host/src/com/android/overlaytest/remounted/OverlaySharedLibraryTest.java b/core/tests/overlaytests/remount/host/src/com/android/overlaytest/remounted/OverlaySharedLibraryTest.java
index 4939e16..06b2ac8 100644
--- a/core/tests/overlaytests/remount/host/src/com/android/overlaytest/remounted/OverlaySharedLibraryTest.java
+++ b/core/tests/overlaytests/remount/host/src/com/android/overlaytest/remounted/OverlaySharedLibraryTest.java
@@ -16,17 +16,21 @@
 
 package com.android.overlaytest.remounted;
 
+import static org.junit.Assert.assertTrue;
+
 import com.android.tradefed.device.DeviceNotAvailableException;
 import com.android.tradefed.testtype.DeviceJUnit4ClassRunner;
+import com.android.tradefed.testtype.junit4.BaseHostJUnit4Test;
 
+import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.RuleChain;
+import org.junit.rules.TemporaryFolder;
 import org.junit.runner.RunWith;
 
-import java.util.Collections;
-import java.util.List;
-
 @RunWith(DeviceJUnit4ClassRunner.class)
-public class OverlaySharedLibraryTest extends OverlayHostTest {
+public class OverlaySharedLibraryTest extends BaseHostJUnit4Test {
     private static final String TARGET_APK = "OverlayRemountedTest_Target.apk";
     private static final String TARGET_PACKAGE = "com.android.overlaytest.remounted.target";
     private static final String SHARED_LIBRARY_APK =
@@ -38,6 +42,17 @@
     private static final String SHARED_LIBRARY_OVERLAY_PACKAGE =
             "com.android.overlaytest.remounted.shared_library.overlay";
 
+    public final TemporaryFolder temporaryFolder = new TemporaryFolder();
+    public final SystemPreparer preparer = new SystemPreparer(temporaryFolder, this::getDevice);
+
+    @Rule
+    public final RuleChain ruleChain = RuleChain.outerRule(temporaryFolder).around(preparer);
+
+    @Before
+    public void startBefore() throws DeviceNotAvailableException {
+        getDevice().waitForDeviceAvailable();
+    }
+
     @Test
     public void testSharedLibrary() throws Exception {
         final String targetResource = resourceName(TARGET_PACKAGE, "bool",
@@ -45,23 +60,20 @@
         final String libraryResource = resourceName(SHARED_LIBRARY_PACKAGE, "bool",
                 "shared_library_overlaid");
 
-        mPreparer.pushResourceFile(SHARED_LIBRARY_APK, "/product/app/SharedLibrary.apk")
+        preparer.pushResourceFile(SHARED_LIBRARY_APK, "/product/app/SharedLibrary.apk")
                 .installResourceApk(SHARED_LIBRARY_OVERLAY_APK, SHARED_LIBRARY_OVERLAY_PACKAGE)
                 .reboot()
                 .setOverlayEnabled(SHARED_LIBRARY_OVERLAY_PACKAGE, false)
                 .installResourceApk(TARGET_APK, TARGET_PACKAGE);
 
         // The shared library resource is not currently overlaid.
-        retrieveResource(Collections.emptyList(), targetResource, libraryResource);
-        assertResource(targetResource, 0x12 /* TYPE_INT_BOOLEAN */, "false");
-        assertResource(libraryResource, 0x12 /* TYPE_INT_BOOLEAN */, "false");
+        assertResource(targetResource, "false");
+        assertResource(libraryResource, "false");
 
         // Overlay the shared library resource.
-        mPreparer.setOverlayEnabled(SHARED_LIBRARY_OVERLAY_PACKAGE, true);
-        retrieveResource(getPackagePaths(SHARED_LIBRARY_OVERLAY_PACKAGE), targetResource,
-                libraryResource);
-        assertResource(targetResource, 0x12 /* TYPE_INT_BOOLEAN */, "true");
-        assertResource(libraryResource, 0x12 /* TYPE_INT_BOOLEAN */, "true");
+        preparer.setOverlayEnabled(SHARED_LIBRARY_OVERLAY_PACKAGE, true);
+        assertResource(targetResource, "true");
+        assertResource(libraryResource, "true");
     }
 
     @Test
@@ -71,20 +83,27 @@
         final String libraryResource = resourceName(SHARED_LIBRARY_PACKAGE, "bool",
                 "shared_library_overlaid");
 
-        mPreparer.pushResourceFile(SHARED_LIBRARY_APK, "/product/app/SharedLibrary.apk")
+        preparer.pushResourceFile(SHARED_LIBRARY_APK, "/product/app/SharedLibrary.apk")
                 .installResourceApk(SHARED_LIBRARY_OVERLAY_APK, SHARED_LIBRARY_OVERLAY_PACKAGE)
                 .setOverlayEnabled(SHARED_LIBRARY_OVERLAY_PACKAGE, true)
                 .reboot()
                 .installResourceApk(TARGET_APK, TARGET_PACKAGE);
 
-        retrieveResource(getPackagePaths(SHARED_LIBRARY_OVERLAY_PACKAGE), targetResource,
-                libraryResource);
-        assertResource(targetResource, 0x12 /* TYPE_INT_BOOLEAN */, "true");
-        assertResource(libraryResource, 0x12 /* TYPE_INT_BOOLEAN */, "true");
+        assertResource(targetResource, "true");
+        assertResource(libraryResource, "true");
     }
 
-    private void retrieveResource(List<String> requiredOverlayPaths, String... resourceNames)
+    /** Builds the full name of a resource in the form package:type/entry. */
+    String resourceName(String pkg, String type, String entry) {
+        return String.format("%s:%s/%s", pkg, type, entry);
+    }
+
+    void assertResource(String resourceName, String expectedValue)
             throws DeviceNotAvailableException {
-        retrieveResource(TARGET_PACKAGE, requiredOverlayPaths, resourceNames);
+        final String result = getDevice().executeShellCommand(
+                String.format("cmd overlay lookup %s %s", TARGET_PACKAGE, resourceName));
+        assertTrue(String.format("expected: <[%s]> in: <[%s]>", expectedValue, result),
+                result.equals(expectedValue + "\n") ||
+                result.endsWith("-> " + expectedValue + "\n"));
     }
 }
diff --git a/core/tests/overlaytests/remount/host/src/com/android/overlaytest/remounted/SystemPreparer.java b/core/tests/overlaytests/remount/host/src/com/android/overlaytest/remounted/SystemPreparer.java
index 7028f2f..8696091 100644
--- a/core/tests/overlaytests/remount/host/src/com/android/overlaytest/remounted/SystemPreparer.java
+++ b/core/tests/overlaytests/remount/host/src/com/android/overlaytest/remounted/SystemPreparer.java
@@ -38,8 +38,7 @@
 import java.util.concurrent.TimeoutException;
 
 class SystemPreparer extends ExternalResource {
-    private static final long REBOOT_SLEEP_MS = 30000;
-    private static final long OVERLAY_ENABLE_TIMEOUT_MS = 20000;
+    private static final long OVERLAY_ENABLE_TIMEOUT_MS = 30000;
 
     // The paths of the files pushed onto the device through this rule.
     private ArrayList<String> mPushedFiles = new ArrayList<>();
@@ -59,6 +58,7 @@
     SystemPreparer pushResourceFile(String resourcePath,
             String outputPath) throws DeviceNotAvailableException, IOException {
         final ITestDevice device = mDeviceProvider.getDevice();
+        device.executeAdbCommand("remount");
         assertTrue(device.pushFile(copyResourceToTemp(resourcePath), outputPath));
         mPushedFiles.add(outputPath);
         return this;
@@ -77,7 +77,7 @@
 
     /** Sets the enable state of an overlay pacakage. */
     SystemPreparer setOverlayEnabled(String packageName, boolean enabled)
-            throws ExecutionException, TimeoutException {
+            throws ExecutionException, DeviceNotAvailableException {
         final ITestDevice device = mDeviceProvider.getDevice();
 
         // Wait for the overlay to change its enabled state.
@@ -86,8 +86,10 @@
                 device.executeShellCommand(String.format("cmd overlay %s %s",
                         enabled ? "enable" : "disable", packageName));
 
-                final String pattern = (enabled ? "[x]" : "[ ]") + " " + packageName;
-                if (device.executeShellCommand("cmd overlay list").contains(pattern)) {
+                final String result = device.executeShellCommand("cmd overlay dump " + packageName);
+                final int startIndex = result.indexOf("mIsEnabled");
+                final int endIndex = result.indexOf('\n', startIndex);
+                if (result.substring(startIndex, endIndex).contains((enabled) ? "true" : "false")) {
                     return true;
                 }
             }
@@ -98,6 +100,8 @@
         try {
             enabledListener.get(OVERLAY_ENABLE_TIMEOUT_MS, MILLISECONDS);
         } catch (InterruptedException ignored) {
+        } catch (TimeoutException e) {
+            throw new IllegalStateException(device.executeShellCommand("cmd overlay list"));
         }
 
         return this;
@@ -106,14 +110,7 @@
     /** Restarts the device and waits until after boot is completed. */
     SystemPreparer reboot() throws DeviceNotAvailableException {
         final ITestDevice device = mDeviceProvider.getDevice();
-        device.executeShellCommand("stop");
-        device.executeShellCommand("start");
-        try {
-            // Sleep until the device is ready for test execution.
-            Thread.sleep(REBOOT_SLEEP_MS);
-        } catch (InterruptedException ignored) {
-        }
-
+        device.reboot();
         return this;
     }
 
@@ -141,12 +138,14 @@
     protected void after() {
         final ITestDevice device = mDeviceProvider.getDevice();
         try {
+            device.executeAdbCommand("remount");
             for (final String file : mPushedFiles) {
                 device.deleteFile(file);
             }
             for (final String packageName : mInstalledPackages) {
                 device.uninstallPackage(packageName);
             }
+            device.reboot();
         } catch (DeviceNotAvailableException e) {
             Assert.fail(e.toString());
         }
diff --git a/core/tests/overlaytests/remount/target/AndroidManifest.xml b/core/tests/overlaytests/remount/target/AndroidManifest.xml
index 32fec43..dc07dca 100644
--- a/core/tests/overlaytests/remount/target/AndroidManifest.xml
+++ b/core/tests/overlaytests/remount/target/AndroidManifest.xml
@@ -23,8 +23,4 @@
         <uses-library android:name="com.android.overlaytest.remounted.shared_library"
                       android:required="true" />
     </application>
-
-    <instrumentation android:name="com.android.overlaytest.remounted.target.ResourceRetrievalRunner"
-                     android:targetPackage="com.android.overlaytest.remounted.target"
-                     android:label="Remounted system RRO tests" />
 </manifest>
diff --git a/core/tests/overlaytests/remount/target/src/com/android/overlaytest/remounted/target/ResourceRetrievalRunner.java b/core/tests/overlaytests/remount/target/src/com/android/overlaytest/remounted/target/ResourceRetrievalRunner.java
deleted file mode 100644
index 2e4c211..0000000
--- a/core/tests/overlaytests/remount/target/src/com/android/overlaytest/remounted/target/ResourceRetrievalRunner.java
+++ /dev/null
@@ -1,140 +0,0 @@
-/*
- * Copyright (C) 2019 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package com.android.overlaytest.remounted.target;
-
-import android.app.Activity;
-import android.app.Instrumentation;
-import android.content.res.Resources;
-import android.os.Bundle;
-import android.util.Log;
-import android.util.TypedValue;
-
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.concurrent.Executor;
-import java.util.concurrent.FutureTask;
-import java.util.concurrent.TimeUnit;
-
-/**
- * An {@link Instrumentation} that retrieves the value of specified resources within the
- * application.
- **/
-public class ResourceRetrievalRunner extends Instrumentation {
-    private static final String TAG = ResourceRetrievalRunner.class.getSimpleName();
-
-    // A list of whitespace separated resource names of which to retrieve the resource values.
-    private static final String RESOURCE_LIST_TAG = "res";
-
-    // A list of whitespace separated overlay package paths that must be present before retrieving
-    // resource values.
-    private static final String REQUIRED_OVERLAYS_LIST_TAG = "overlays";
-
-    // The suffixes of the keys returned from the instrumentation. To retrieve the type of a
-    // resource looked up with the instrumentation, append the {@link #RESOURCES_TYPE_SUFFIX} suffix
-    // to the end of the name of the resource. For the value of a resource, use
-    // {@link #RESOURCES_DATA_SUFFIX} instead.
-    private static final String RESOURCES_TYPE_SUFFIX = "_type";
-    private static final String RESOURCES_DATA_SUFFIX = "_data";
-
-    // The amount of time in seconds to wait for the overlays to be present in the AssetManager.
-    private static final int OVERLAY_PATH_TIMEOUT = 60;
-
-    private final ArrayList<String> mResourceNames = new ArrayList<>();
-    private final ArrayList<String> mOverlayPaths = new ArrayList<>();
-    private final Bundle mResult = new Bundle();
-
-    /**
-     * Receives the instrumentation arguments and runs the resource retrieval.
-     * The entry with key {@link #RESOURCE_LIST_TAG} in the {@link Bundle} arguments is a
-     * whitespace separated string of resource names of which to retrieve the resource values.
-     * The entry with key {@link #REQUIRED_OVERLAYS_LIST_TAG} in the {@link Bundle} arguments is a
-     * whitespace separated string of overlay package paths prefixes that must be present before
-     * retrieving the resource values.
-     */
-    @Override
-    public void onCreate(Bundle arguments) {
-        super.onCreate(arguments);
-        mResourceNames.addAll(Arrays.asList(arguments.getString(RESOURCE_LIST_TAG).split(" ")));
-        if (arguments.containsKey(REQUIRED_OVERLAYS_LIST_TAG)) {
-            mOverlayPaths.addAll(Arrays.asList(
-                    arguments.getString(REQUIRED_OVERLAYS_LIST_TAG).split(" ")));
-        }
-        start();
-    }
-
-    @Override
-    public void onStart() {
-        final Resources res = getContext().getResources();
-        res.getAssets().setResourceResolutionLoggingEnabled(true);
-
-        if (!mOverlayPaths.isEmpty()) {
-            Log.d(TAG, String.format("Waiting for overlay paths [%s]",
-                    String.join(",", mOverlayPaths)));
-
-            // Wait for all required overlays to be present in the AssetManager.
-            final FutureTask<Boolean> overlayListener = new FutureTask<>(() -> {
-                while (!mOverlayPaths.isEmpty()) {
-                    final String[] apkPaths = res.getAssets().getApkPaths();
-                    for (String path : apkPaths) {
-                        for (String overlayPath : mOverlayPaths) {
-                            if (path.startsWith(overlayPath)) {
-                                mOverlayPaths.remove(overlayPath);
-                                break;
-                            }
-                        }
-                    }
-                }
-                return true;
-            });
-
-            try {
-                final Executor executor = (t) -> new Thread(t).start();
-                executor.execute(overlayListener);
-                overlayListener.get(OVERLAY_PATH_TIMEOUT, TimeUnit.SECONDS);
-            } catch (Exception e) {
-                Log.e(TAG, String.format("Failed to wait for required overlays [%s]",
-                        String.join(",", mOverlayPaths)), e);
-                finish(Activity.RESULT_CANCELED, mResult);
-            }
-        }
-
-        // Retrieve the values for each resource passed in.
-        final TypedValue typedValue = new TypedValue();
-        for (final String resourceName : mResourceNames) {
-            try {
-                final int resId = res.getIdentifier(resourceName, null, null);
-                res.getValue(resId, typedValue, true);
-                Log.d(TAG, String.format("Resolution for 0x%s: %s", Integer.toHexString(resId),
-                        res.getAssets().getLastResourceResolution()));
-            } catch (Resources.NotFoundException e) {
-                Log.e(TAG, "Failed to retrieve value for resource " + resourceName, e);
-                finish(Activity.RESULT_CANCELED, mResult);
-            }
-
-            putValue(resourceName, typedValue);
-        }
-
-        finish(Activity.RESULT_OK, mResult);
-    }
-
-    private void putValue(String resourceName, TypedValue value) {
-        mResult.putInt(resourceName + RESOURCES_TYPE_SUFFIX, value.type);
-        final CharSequence textValue = value.coerceToString();
-        mResult.putString(resourceName + RESOURCES_DATA_SUFFIX,
-                textValue == null ? "null" : textValue.toString());
-    }
-}