Move test state to TestInformation#properties.
In odsign_e2e_tests, we store some state in static class members.
jdesprez@ pointed out that this can lead to big issues (see details in
the bug).
This CL solves the problem by moving test state to
TestInformation#properties.
Bug: 215199775
Test: atest odsign_e2e_tests
Change-Id: I2987af4c0dd70ab2670891b3b424811559c5693c
diff --git a/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java b/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java
index 0a907f2..7a5aa0d 100644
--- a/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java
+++ b/test/odsign/test-src/com/android/tests/odsign/OdrefreshHostTest.java
@@ -27,11 +27,13 @@
import com.android.tradefed.testtype.junit4.BaseHostJUnit4Test;
import com.android.tradefed.testtype.junit4.BeforeClassWithInfo;
+import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
+import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
import java.util.regex.Pattern;
@@ -49,29 +51,40 @@
private static final String ODREFRESH_COMMAND =
ODREFRESH_BIN + " --partial-compilation --no-refresh --compile";
- private static OdsignTestUtils sTestUtils;
+ private static final String TAG = "OdrefreshHostTest";
+ private static final String ZYGOTE_ARTIFACTS_KEY = TAG + ":ZYGOTE_ARTIFACTS";
+ private static final String SYSTEM_SERVER_ARTIFACTS_KEY = TAG + ":SYSTEM_SERVER_ARTIFACTS";
- private static Set<String> sZygoteArtifacts;
- private static Set<String> sSystemServerArtifacts;
+ private OdsignTestUtils mTestUtils;
@BeforeClassWithInfo
public static void beforeClassWithDevice(TestInformation testInfo) throws Exception {
- sTestUtils = new OdsignTestUtils(testInfo);
- sTestUtils.installTestApex();
- sTestUtils.enableAdbRootOrSkipTest();
+ OdsignTestUtils testUtils = new OdsignTestUtils(testInfo);
+ testUtils.installTestApex();
+ testUtils.enableAdbRootOrSkipTest();
- sZygoteArtifacts = new HashSet<>();
- for (String zygoteName : sTestUtils.ZYGOTE_NAMES) {
- sZygoteArtifacts.addAll(
- sTestUtils.getZygoteLoadedArtifacts(zygoteName).orElse(new HashSet<>()));
+ HashSet<String> zygoteArtifacts = new HashSet<>();
+ for (String zygoteName : testUtils.ZYGOTE_NAMES) {
+ zygoteArtifacts.addAll(
+ testUtils.getZygoteLoadedArtifacts(zygoteName).orElse(new HashSet<>()));
}
- sSystemServerArtifacts = sTestUtils.getSystemServerLoadedArtifacts();
+ Set<String> systemServerArtifacts = testUtils.getSystemServerLoadedArtifacts();
+
+ testInfo.properties().put(ZYGOTE_ARTIFACTS_KEY, String.join(":", zygoteArtifacts));
+ testInfo.properties()
+ .put(SYSTEM_SERVER_ARTIFACTS_KEY, String.join(":", systemServerArtifacts));
}
@AfterClassWithInfo
public static void afterClassWithDevice(TestInformation testInfo) throws Exception {
- sTestUtils.uninstallTestApex();
- sTestUtils.restoreAdbRoot();
+ OdsignTestUtils testUtils = new OdsignTestUtils(testInfo);
+ testUtils.uninstallTestApex();
+ testUtils.restoreAdbRoot();
+ }
+
+ @Before
+ public void setUp() throws Exception {
+ mTestUtils = new OdsignTestUtils(getTestInformation());
}
@Test
@@ -80,8 +93,8 @@
long timeMs = getCurrentTimeMs();
getDevice().executeShellV2Command(ODREFRESH_COMMAND);
- assertArtifactsModifiedAfter(sZygoteArtifacts, timeMs);
- assertArtifactsModifiedAfter(sSystemServerArtifacts, timeMs);
+ assertArtifactsModifiedAfter(getZygoteArtifacts(), timeMs);
+ assertArtifactsModifiedAfter(getSystemServerArtifacts(), timeMs);
}
@Test
@@ -90,8 +103,8 @@
long timeMs = getCurrentTimeMs();
getDevice().executeShellV2Command(ODREFRESH_COMMAND);
- assertArtifactsNotModifiedAfter(sZygoteArtifacts, timeMs);
- assertArtifactsModifiedAfter(sSystemServerArtifacts, timeMs);
+ assertArtifactsNotModifiedAfter(getZygoteArtifacts(), timeMs);
+ assertArtifactsModifiedAfter(getSystemServerArtifacts(), timeMs);
}
@Test
@@ -100,8 +113,8 @@
long timeMs = getCurrentTimeMs();
getDevice().executeShellV2Command(ODREFRESH_COMMAND);
- assertArtifactsModifiedAfter(sZygoteArtifacts, timeMs);
- assertArtifactsModifiedAfter(sSystemServerArtifacts, timeMs);
+ assertArtifactsModifiedAfter(getZygoteArtifacts(), timeMs);
+ assertArtifactsModifiedAfter(getSystemServerArtifacts(), timeMs);
}
@Test
@@ -110,19 +123,19 @@
long timeMs = getCurrentTimeMs();
getDevice().executeShellV2Command(ODREFRESH_COMMAND);
- assertArtifactsNotModifiedAfter(sZygoteArtifacts, timeMs);
- assertArtifactsModifiedAfter(sSystemServerArtifacts, timeMs);
+ assertArtifactsNotModifiedAfter(getZygoteArtifacts(), timeMs);
+ assertArtifactsModifiedAfter(getSystemServerArtifacts(), timeMs);
}
@Test
public void verifyMissingArtifactTriggersCompilation() throws Exception {
Set<String> missingArtifacts = simulateMissingArtifacts();
Set<String> remainingArtifacts = new HashSet<>();
- remainingArtifacts.addAll(sZygoteArtifacts);
- remainingArtifacts.addAll(sSystemServerArtifacts);
+ remainingArtifacts.addAll(getZygoteArtifacts());
+ remainingArtifacts.addAll(getSystemServerArtifacts());
remainingArtifacts.removeAll(missingArtifacts);
- sTestUtils.removeCompilationLogToAvoidBackoff();
+ mTestUtils.removeCompilationLogToAvoidBackoff();
long timeMs = getCurrentTimeMs();
getDevice().executeShellV2Command(ODREFRESH_COMMAND);
@@ -132,12 +145,12 @@
@Test
public void verifyNoCompilationWhenCacheIsGood() throws Exception {
- sTestUtils.removeCompilationLogToAvoidBackoff();
+ mTestUtils.removeCompilationLogToAvoidBackoff();
long timeMs = getCurrentTimeMs();
getDevice().executeShellV2Command(ODREFRESH_COMMAND);
- assertArtifactsNotModifiedAfter(sZygoteArtifacts, timeMs);
- assertArtifactsNotModifiedAfter(sSystemServerArtifacts, timeMs);
+ assertArtifactsNotModifiedAfter(getZygoteArtifacts(), timeMs);
+ assertArtifactsNotModifiedAfter(getSystemServerArtifacts(), timeMs);
}
@Test
@@ -165,20 +178,20 @@
@Test
public void verifyCompilationOsMode() throws Exception {
- sTestUtils.removeCompilationLogToAvoidBackoff();
+ mTestUtils.removeCompilationLogToAvoidBackoff();
long timeMs = getCurrentTimeMs();
getDevice().executeShellV2Command(ODREFRESH_BIN + " --compilation-os-mode --compile");
// odrefresh should unconditionally compile everything in Compilation OS.
- assertArtifactsModifiedAfter(sZygoteArtifacts, timeMs);
- assertArtifactsModifiedAfter(sSystemServerArtifacts, timeMs);
+ assertArtifactsModifiedAfter(getZygoteArtifacts(), timeMs);
+ assertArtifactsModifiedAfter(getSystemServerArtifacts(), timeMs);
String cacheInfo = getDevice().pullFileContents(CACHE_INFO_FILE);
assertThat(cacheInfo).contains("compilationOsMode=\"true\"");
assertThat(cacheInfo).doesNotContain("lastUpdateMillis=");
// Compilation OS does not write the compilation log to the host.
- sTestUtils.removeCompilationLogToAvoidBackoff();
+ mTestUtils.removeCompilationLogToAvoidBackoff();
// Simulate the odrefresh invocation on the next boot.
timeMs = getCurrentTimeMs();
@@ -186,8 +199,8 @@
// odrefresh should not re-compile anything regardless of the missing `lastUpdateMillis`
// field.
- assertArtifactsNotModifiedAfter(sZygoteArtifacts, timeMs);
- assertArtifactsNotModifiedAfter(sSystemServerArtifacts, timeMs);
+ assertArtifactsNotModifiedAfter(getZygoteArtifacts(), timeMs);
+ assertArtifactsNotModifiedAfter(getSystemServerArtifacts(), timeMs);
// The cache info should be updated.
cacheInfo = getDevice().pullFileContents(CACHE_INFO_FILE);
@@ -269,7 +282,7 @@
private Set<String> simulateMissingArtifacts() throws Exception {
Set<String> missingArtifacts = new HashSet<>();
- String sample = sSystemServerArtifacts.iterator().next();
+ String sample = getSystemServerArtifacts().iterator().next();
for (String extension : OdsignTestUtils.APP_ARTIFACT_EXTENSIONS) {
String artifact = replaceExtension(sample, extension);
getDevice().deleteFile(artifact);
@@ -337,4 +350,20 @@
assertTrue("Extension not found in filename: " + filename, index != -1);
return filename.substring(0, index) + extension;
}
+
+ private Set<String> getColonSeparatedSet(String key) {
+ String value = getTestInformation().properties().get(key);
+ if (value == null || value.isEmpty()) {
+ return new HashSet<>();
+ }
+ return new HashSet<>(Arrays.asList(value.split(":")));
+ }
+
+ private Set<String> getZygoteArtifacts() {
+ return getColonSeparatedSet(ZYGOTE_ARTIFACTS_KEY);
+ }
+
+ private Set<String> getSystemServerArtifacts() {
+ return getColonSeparatedSet(SYSTEM_SERVER_ARTIFACTS_KEY);
+ }
}
diff --git a/test/odsign/test-src/com/android/tests/odsign/OdsignTestUtils.java b/test/odsign/test-src/com/android/tests/odsign/OdsignTestUtils.java
index e3a2eaa..a18286e 100644
--- a/test/odsign/test-src/com/android/tests/odsign/OdsignTestUtils.java
+++ b/test/odsign/test-src/com/android/tests/odsign/OdsignTestUtils.java
@@ -50,12 +50,13 @@
private static final Duration BOOT_COMPLETE_TIMEOUT = Duration.ofMinutes(2);
+ private static final String TAG = "OdsignTestUtils";
+ private static final String WAS_ADB_ROOT_KEY = TAG + ":WAS_ADB_ROOT";
+ private static final String ADB_ROOT_ENABLED_KEY = TAG + ":ADB_ROOT_ENABLED";
+
private final InstallUtilsHost mInstallUtils;
private final TestInformation mTestInfo;
- private boolean mWasAdbRoot = false;
- private boolean mAdbRootEnabled = false;
-
public OdsignTestUtils(TestInformation testInfo) throws Exception {
assertNotNull(testInfo.getDevice());
mInstallUtils = new InstallUtilsHost(testInfo);
@@ -151,17 +152,33 @@
* Enables adb root or skips the test if adb root is not supported.
*/
public void enableAdbRootOrSkipTest() throws Exception {
- mWasAdbRoot = mTestInfo.getDevice().isAdbRoot();
- mAdbRootEnabled = mTestInfo.getDevice().enableAdbRoot();
- assumeTrue("ADB root failed and required to get process maps", mAdbRootEnabled);
+ setBoolean(WAS_ADB_ROOT_KEY, mTestInfo.getDevice().isAdbRoot());
+ boolean adbRootEnabled = mTestInfo.getDevice().enableAdbRoot();
+ assumeTrue("ADB root failed and required to get process maps", adbRootEnabled);
+ setBoolean(ADB_ROOT_ENABLED_KEY, adbRootEnabled);
}
/**
* Restores the device to the state before {@link enableAdbRootOrSkipTest} was called.
*/
public void restoreAdbRoot() throws Exception {
- if (mAdbRootEnabled && !mWasAdbRoot) {
+ if (getBooleanOrDefault(ADB_ROOT_ENABLED_KEY) && !getBooleanOrDefault(WAS_ADB_ROOT_KEY)) {
mTestInfo.getDevice().disableAdbRoot();
}
}
+
+ /**
+ * Returns the value of a boolean test property, or false if it does not exist.
+ */
+ private boolean getBooleanOrDefault(String key) {
+ String value = mTestInfo.properties().get(key);
+ if (value == null) {
+ return false;
+ }
+ return Boolean.parseBoolean(value);
+ }
+
+ private void setBoolean(String key, boolean value) {
+ mTestInfo.properties().put(key, Boolean.toString(value));
+ }
}
diff --git a/test/odsign/test-src/com/android/tests/odsign/OnDeviceSigningHostTest.java b/test/odsign/test-src/com/android/tests/odsign/OnDeviceSigningHostTest.java
index fe8465a..62d61a2 100644
--- a/test/odsign/test-src/com/android/tests/odsign/OnDeviceSigningHostTest.java
+++ b/test/odsign/test-src/com/android/tests/odsign/OnDeviceSigningHostTest.java
@@ -25,6 +25,7 @@
import com.android.tradefed.testtype.junit4.BeforeClassWithInfo;
import com.android.tradefed.testtype.junit4.DeviceTestRunOptions;
+import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -43,18 +44,24 @@
private static final String TEST_APP_PACKAGE_NAME = "com.android.tests.odsign";
private static final String TEST_APP_APK = "odsign_e2e_test_app.apk";
- private static OdsignTestUtils sTestUtils;
+ private OdsignTestUtils mTestUtils;
@BeforeClassWithInfo
public static void beforeClassWithDevice(TestInformation testInfo) throws Exception {
- sTestUtils = new OdsignTestUtils(testInfo);
- sTestUtils.installTestApex();
+ OdsignTestUtils testUtils = new OdsignTestUtils(testInfo);
+ testUtils.installTestApex();
}
@AfterClassWithInfo
public static void afterClassWithDevice(TestInformation testInfo) throws Exception {
- sTestUtils.uninstallTestApex();
- sTestUtils.restoreAdbRoot();
+ OdsignTestUtils testUtils = new OdsignTestUtils(testInfo);
+ testUtils.uninstallTestApex();
+ testUtils.restoreAdbRoot();
+ }
+
+ @Before
+ public void setUp() throws Exception {
+ mTestUtils = new OdsignTestUtils(getTestInformation());
}
@Test
@@ -78,10 +85,10 @@
@Test
public void verifyGeneratedArtifactsLoaded() throws Exception {
// Checking zygote and system_server need the device have adb root to walk process maps.
- sTestUtils.enableAdbRootOrSkipTest();
+ mTestUtils.enableAdbRootOrSkipTest();
// Check there is a compilation log, we expect compilation to have occurred.
- assertTrue("Compilation log not found", sTestUtils.haveCompilationLog());
+ assertTrue("Compilation log not found", mTestUtils.haveCompilationLog());
// Check both zygote and system_server processes to see that they have loaded the
// artifacts compiled and signed by odrefresh and odsign. We check both here rather than
@@ -93,21 +100,21 @@
@Test
public void verifyGeneratedArtifactsLoadedAfterReboot() throws Exception {
- sTestUtils.enableAdbRootOrSkipTest();
+ mTestUtils.enableAdbRootOrSkipTest();
- sTestUtils.reboot();
+ mTestUtils.reboot();
verifyGeneratedArtifactsLoaded();
}
@Test
public void verifyGeneratedArtifactsLoadedAfterPartialCompilation() throws Exception {
- sTestUtils.enableAdbRootOrSkipTest();
+ mTestUtils.enableAdbRootOrSkipTest();
- Set<String> mappedArtifacts = sTestUtils.getSystemServerLoadedArtifacts();
+ Set<String> mappedArtifacts = mTestUtils.getSystemServerLoadedArtifacts();
// Delete an arbitrary artifact.
getDevice().deleteFile(mappedArtifacts.iterator().next());
- sTestUtils.removeCompilationLogToAvoidBackoff();
- sTestUtils.reboot();
+ mTestUtils.removeCompilationLogToAvoidBackoff();
+ mTestUtils.reboot();
verifyGeneratedArtifactsLoaded();
}
@@ -134,7 +141,7 @@
.concat(Arrays.stream(classpathElements), Arrays.stream(standaloneJars))
.toArray(String[]::new);
- final Set<String> mappedArtifacts = sTestUtils.getSystemServerLoadedArtifacts();
+ final Set<String> mappedArtifacts = mTestUtils.getSystemServerLoadedArtifacts();
assertTrue(
"No mapped artifacts under " + OdsignTestUtils.ART_APEX_DALVIK_CACHE_DIRNAME,
mappedArtifacts.size() > 0);
@@ -182,7 +189,7 @@
int zygoteCount = 0;
for (String zygoteName : OdsignTestUtils.ZYGOTE_NAMES) {
final Optional<Set<String>> mappedArtifacts =
- sTestUtils.getZygoteLoadedArtifacts(zygoteName);
+ mTestUtils.getZygoteLoadedArtifacts(zygoteName);
if (!mappedArtifacts.isPresent()) {
continue;
}