From 77a40731f8388ae1eb708dc40c08c2da9d1fd04a Mon Sep 17 00:00:00 2001 From: Oli Lan Date: Thu, 22 Aug 2019 11:24:42 +0100 Subject: [PATCH] Wait for available rollbacks in RollbackTest. This changes RollbackTest to retry checks for available rollbacks for a certain period of time, using a new RollbackUtils method waitForAvailableRollbacks. Currently many of the tests in RollbackTest are flaky due to race conditions between checking for available rollbacks and the installer onFinished callback. This CL should fix all of those issues. The previous attempt to resolve this by adding NewRollbacks speculatively to the results of getAvailableRollbacks was not successful. This CL reverts that change so that getAvailableRollbacks once again only returns rollbacks that are actually available. Bug: 136548146 Test: atest RollbackTest run on cuttlefish on acloud repeatedly Change-Id: I0389ef2413e7642fe2f6ef1a33aef218dd1896ed Merged-In: I27e2513b34f49af2f41cd627a5d2550541831e11 --- .../content/rollback/RollbackManager.java | 5 +- .../rollback/RollbackManagerServiceImpl.java | 9 --- .../android/tests/rollback/RollbackTest.java | 75 ++++++------------- 3 files changed, 25 insertions(+), 64 deletions(-) diff --git a/core/java/android/content/rollback/RollbackManager.java b/core/java/android/content/rollback/RollbackManager.java index 1609f53d3d3b1..73b8a48d9153c 100644 --- a/core/java/android/content/rollback/RollbackManager.java +++ b/core/java/android/content/rollback/RollbackManager.java @@ -74,10 +74,7 @@ public final class RollbackManager { } /** - * Returns a list of all currently available rollbacks. This includes ones for very recently - * installed packages (even if onFinished has not yet been called). As a result, packages that - * very recently failed to install may also be included, but those rollbacks will fail with - * 'rollback not available'. + * Returns a list of all currently available rollbacks. * * @throws SecurityException if the caller does not have appropriate permissions. */ diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index 4b2f3636cf1f4..42f3e2fd31c91 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -282,15 +282,6 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { rollbacks.add(rollback.info); } } - - // Also return new rollbacks for which the PackageRollbackInfo is complete. - for (NewRollback newRollback : mNewRollbacks) { - if (newRollback.rollback.info.getPackages().size() - == newRollback.packageSessionIds.length - && !newRollback.isCancelled) { - rollbacks.add(newRollback.rollback.info); - } - } return new ParceledListSlice<>(rollbacks); } } diff --git a/tests/RollbackTest/RollbackTest/src/com/android/tests/rollback/RollbackTest.java b/tests/RollbackTest/RollbackTest/src/com/android/tests/rollback/RollbackTest.java index 581e7c4d5a605..228e50ed41a0a 100644 --- a/tests/RollbackTest/RollbackTest/src/com/android/tests/rollback/RollbackTest.java +++ b/tests/RollbackTest/RollbackTest/src/com/android/tests/rollback/RollbackTest.java @@ -19,6 +19,8 @@ package com.android.tests.rollback; import static com.android.cts.install.lib.InstallUtils.processUserData; import static com.android.cts.rollback.lib.RollbackInfoSubject.assertThat; import static com.android.cts.rollback.lib.RollbackUtils.getUniqueRollbackInfoForPackage; +import static com.android.cts.rollback.lib.RollbackUtils.waitForAvailableRollback; +import static com.android.cts.rollback.lib.RollbackUtils.waitForUnavailableRollback; import static com.google.common.truth.Truth.assertThat; @@ -118,8 +120,7 @@ public class RollbackTest { } // The app should not be available for rollback. - assertThat( - getUniqueRollbackInfoForPackage(rm.getAvailableRollbacks(), TestApp.A)).isNull(); + waitForUnavailableRollback(TestApp.A); // There should be no recently committed rollbacks for this package. assertThat(getUniqueRollbackInfoForPackage( @@ -134,9 +135,7 @@ public class RollbackTest { assertThat(InstallUtils.getInstalledVersion(TestApp.A)).isEqualTo(2); // The app should now be available for rollback. - RollbackInfo available = getUniqueRollbackInfoForPackage( - rm.getAvailableRollbacks(), TestApp.A); - assertThat(available).isNotNull(); + RollbackInfo available = waitForAvailableRollback(TestApp.A); assertThat(available).isNotStaged(); assertThat(available).packagesContainsExactly( Rollback.from(TestApp.A2).to(TestApp.A1)); @@ -197,14 +196,12 @@ public class RollbackTest { assertThat(InstallUtils.getInstalledVersion(TestApp.B)).isEqualTo(2); // Both test apps should now be available for rollback. - RollbackInfo rollbackA = getUniqueRollbackInfoForPackage( - rm.getAvailableRollbacks(), TestApp.A); + RollbackInfo rollbackA = waitForAvailableRollback(TestApp.A); assertThat(rollbackA).isNotNull(); assertThat(rollbackA).packagesContainsExactly( Rollback.from(TestApp.A2).to(TestApp.A1)); - RollbackInfo rollbackB = getUniqueRollbackInfoForPackage( - rm.getAvailableRollbacks(), TestApp.B); + RollbackInfo rollbackB = waitForAvailableRollback(TestApp.B); assertThat(rollbackB).isNotNull(); assertThat(rollbackB).packagesContainsExactly( Rollback.from(TestApp.B2).to(TestApp.B1)); @@ -213,14 +210,12 @@ public class RollbackTest { rm.reloadPersistedData(); // The apps should still be available for rollback. - rollbackA = getUniqueRollbackInfoForPackage( - rm.getAvailableRollbacks(), TestApp.A); + rollbackA = waitForAvailableRollback(TestApp.A); assertThat(rollbackA).isNotNull(); assertThat(rollbackA).packagesContainsExactly( Rollback.from(TestApp.A2).to(TestApp.A1)); - rollbackB = getUniqueRollbackInfoForPackage( - rm.getAvailableRollbacks(), TestApp.B); + rollbackB = waitForAvailableRollback(TestApp.B); assertThat(rollbackB).isNotNull(); assertThat(rollbackB).packagesContainsExactly( Rollback.from(TestApp.B2).to(TestApp.B1)); @@ -255,15 +250,13 @@ public class RollbackTest { assertThat(InstallUtils.getInstalledVersion(TestApp.B)).isEqualTo(2); // The app should now be available for rollback. - RollbackInfo availableA = getUniqueRollbackInfoForPackage( - rm.getAvailableRollbacks(), TestApp.A); + RollbackInfo availableA = waitForAvailableRollback(TestApp.A); assertThat(availableA).isNotNull(); assertThat(availableA).packagesContainsExactly( Rollback.from(TestApp.A2).to(TestApp.A1), Rollback.from(TestApp.B2).to(TestApp.B1)); - RollbackInfo availableB = getUniqueRollbackInfoForPackage( - rm.getAvailableRollbacks(), TestApp.B); + RollbackInfo availableB = waitForAvailableRollback(TestApp.B); assertThat(availableB).isNotNull(); assertThat(availableB).packagesContainsExactly( Rollback.from(TestApp.A2).to(TestApp.A1), @@ -276,13 +269,13 @@ public class RollbackTest { rm.reloadPersistedData(); // The apps should still be available for rollback. - availableA = getUniqueRollbackInfoForPackage(rm.getAvailableRollbacks(), TestApp.A); + availableA = waitForAvailableRollback(TestApp.A); assertThat(availableA).isNotNull(); assertThat(availableA).packagesContainsExactly( Rollback.from(TestApp.A2).to(TestApp.A1), Rollback.from(TestApp.B2).to(TestApp.B1)); - availableB = getUniqueRollbackInfoForPackage(rm.getAvailableRollbacks(), TestApp.B); + availableB = waitForAvailableRollback(TestApp.B); assertThat(availableB).isNotNull(); assertThat(availableB).packagesContainsExactly( Rollback.from(TestApp.A2).to(TestApp.A1), @@ -334,8 +327,7 @@ public class RollbackTest { assertThat(InstallUtils.getInstalledVersion(TestApp.A)).isEqualTo(2); // The app should now be available for rollback. - RollbackInfo available = getUniqueRollbackInfoForPackage( - rm.getAvailableRollbacks(), TestApp.A); + RollbackInfo available = waitForAvailableRollback(TestApp.A); assertThat(available).isNotNull(); // Roll back the app. @@ -405,13 +397,11 @@ public class RollbackTest { // Check that the rollback data has not expired Thread.sleep(1000); - RollbackInfo rollback = getUniqueRollbackInfoForPackage( - rm.getAvailableRollbacks(), TestApp.A); - assertThat(rollback).isNotNull(); + RollbackInfo rollback = waitForAvailableRollback(TestApp.A); assertThat(rollback).packagesContainsExactly( Rollback.from(TestApp.A2).to(TestApp.A1)); - // Give it a little more time, but still not the long enough to expire + // Give it a little more time, but still not long enough to expire Thread.sleep(expirationTime / 2); rollback = getUniqueRollbackInfoForPackage( rm.getAvailableRollbacks(), TestApp.A); @@ -536,9 +526,7 @@ public class RollbackTest { assertThat(InstallUtils.getInstalledVersion(TestApp.A)).isEqualTo(2); // The app should now be available for rollback. - RollbackInfo rollback = getUniqueRollbackInfoForPackage( - rm.getAvailableRollbacks(), TestApp.A); - assertThat(rollback).isNotNull(); + RollbackInfo rollback = waitForAvailableRollback(TestApp.A); assertThat(rollback).packagesContainsExactly( Rollback.from(TestApp.A2).to(TestApp.A1)); @@ -570,9 +558,7 @@ public class RollbackTest { Install.single(TestApp.A2).setEnableRollback().commit(); processUserData(TestApp.A); - RollbackManager rm = RollbackUtils.getRollbackManager(); - RollbackInfo rollback = getUniqueRollbackInfoForPackage( - rm.getAvailableRollbacks(), TestApp.A); + RollbackInfo rollback = waitForAvailableRollback(TestApp.A); RollbackUtils.rollback(rollback.getRollbackId()); processUserData(TestApp.A); } finally { @@ -598,10 +584,7 @@ public class RollbackTest { Install.single(TestApp.ASplit2).setEnableRollback().commit(); processUserData(TestApp.A); - RollbackManager rm = RollbackUtils.getRollbackManager(); - RollbackInfo rollback = getUniqueRollbackInfoForPackage( - rm.getAvailableRollbacks(), TestApp.A); - assertThat(rollback).isNotNull(); + RollbackInfo rollback = waitForAvailableRollback(TestApp.A); RollbackUtils.rollback(rollback.getRollbackId()); processUserData(TestApp.A); } finally { @@ -644,7 +627,6 @@ public class RollbackTest { Manifest.permission.INSTALL_PACKAGES, Manifest.permission.DELETE_PACKAGES, Manifest.permission.TEST_MANAGE_ROLLBACKS); - RollbackManager rm = RollbackUtils.getRollbackManager(); // Prep installation of the test apps. Uninstall.packages(TestApp.A); @@ -659,15 +641,11 @@ public class RollbackTest { // Both test apps should now be available for rollback, and the // RollbackInfo returned for the rollbacks should be correct. - RollbackInfo rollbackA = getUniqueRollbackInfoForPackage( - rm.getAvailableRollbacks(), TestApp.A); - assertThat(rollbackA).isNotNull(); + RollbackInfo rollbackA = waitForAvailableRollback(TestApp.A); assertThat(rollbackA).packagesContainsExactly( Rollback.from(TestApp.A2).to(TestApp.A1)); - RollbackInfo rollbackB = getUniqueRollbackInfoForPackage( - rm.getAvailableRollbacks(), TestApp.B); - assertThat(rollbackB).isNotNull(); + RollbackInfo rollbackB = waitForAvailableRollback(TestApp.B); assertThat(rollbackB).packagesContainsExactly( Rollback.from(TestApp.B2).to(TestApp.B1)); @@ -822,8 +800,7 @@ public class RollbackTest { assertThat(InstallUtils.getInstalledVersion(TestApp.B)).isEqualTo(2); // TestApp.A should now be available for rollback. - RollbackInfo rollback = getUniqueRollbackInfoForPackage( - rm.getAvailableRollbacks(), TestApp.A); + RollbackInfo rollback = waitForAvailableRollback(TestApp.A); assertThat(rollback).isNotNull(); assertThat(rollback).packagesContainsExactly( Rollback.from(TestApp.A2).to(TestApp.A1), @@ -938,7 +915,6 @@ public class RollbackTest { Manifest.permission.TEST_MANAGE_ROLLBACKS, Manifest.permission.FORCE_STOP_PACKAGES, Manifest.permission.RESTART_PACKAGES); - RollbackManager rm = RollbackUtils.getRollbackManager(); // Prep installation of the test apps. Uninstall.packages(TestApp.A, TestApp.B); @@ -952,13 +928,11 @@ public class RollbackTest { // Both test apps should now be available for rollback, and the // targetPackage returned for rollback should be correct. - RollbackInfo rollbackA = getUniqueRollbackInfoForPackage( - rm.getAvailableRollbacks(), TestApp.A); + RollbackInfo rollbackA = waitForAvailableRollback(TestApp.A); assertThat(rollbackA).packagesContainsExactly( Rollback.from(TestApp.A2).to(TestApp.A1)); - RollbackInfo rollbackB = getUniqueRollbackInfoForPackage( - rm.getAvailableRollbacks(), TestApp.B); + RollbackInfo rollbackB = waitForAvailableRollback(TestApp.B); assertThat(rollbackB).packagesContainsExactly( Rollback.from(TestApp.B2).to(TestApp.B1)); @@ -1002,8 +976,7 @@ public class RollbackTest { Install.single(TestApp.A2).setEnableRollback().commit(); assertThat(InstallUtils.getInstalledVersion(TestApp.A)).isEqualTo(2); - RollbackInfo rollback = getUniqueRollbackInfoForPackage( - rm.getAvailableRollbacks(), TestApp.A); + RollbackInfo rollback = waitForAvailableRollback(TestApp.A); assertThat(rollback).packagesContainsExactly( Rollback.from(TestApp.A2).to(TestApp.A1));