From e71690f67ff57bba9c2e6bab31ae5e31c1d11da2 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Mon, 25 May 2020 14:39:44 +0800 Subject: [PATCH 1/2] Use Truth (1/n) This allows us to add more descriptive error messages in next CLs. Bug: 177861067 Test: atest StagedRollbackTest Change-Id: I92b1abbde96aebcbe7d3c5f162e80e17997408f6 --- .../rollback/host/StagedRollbackTest.java | 97 +++++++++---------- 1 file changed, 48 insertions(+), 49 deletions(-) diff --git a/tests/RollbackTest/StagedRollbackTest/src/com/android/tests/rollback/host/StagedRollbackTest.java b/tests/RollbackTest/StagedRollbackTest/src/com/android/tests/rollback/host/StagedRollbackTest.java index 2ebb9c13a0168..55c59bb7fd65a 100644 --- a/tests/RollbackTest/StagedRollbackTest/src/com/android/tests/rollback/host/StagedRollbackTest.java +++ b/tests/RollbackTest/StagedRollbackTest/src/com/android/tests/rollback/host/StagedRollbackTest.java @@ -17,10 +17,8 @@ package com.android.tests.rollback.host; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.junit.Assume.assumeTrue; @@ -62,9 +60,9 @@ public class StagedRollbackTest extends BaseHostJUnit4Test { * For example, runPhase("testApkOnlyEnableRollback"); */ private void runPhase(String phase) throws Exception { - assertTrue(runDeviceTests("com.android.tests.rollback", + assertThat(runDeviceTests("com.android.tests.rollback", "com.android.tests.rollback.StagedRollbackTest", - phase)); + phase)).isTrue(); } private static final String APK_IN_APEX_TESTAPEX_NAME = "com.android.apex.apkrollback.test"; @@ -150,17 +148,17 @@ public class StagedRollbackTest extends BaseHostJUnit4Test { // Trigger rollback and wait for reboot to happen runPhase("testBadApkOnly_Phase3"); - assertTrue(getDevice().waitForDeviceNotAvailable(TimeUnit.MINUTES.toMillis(2))); + assertThat(getDevice().waitForDeviceNotAvailable(TimeUnit.MINUTES.toMillis(2))).isTrue(); getDevice().waitForDeviceAvailable(); runPhase("testBadApkOnly_Phase4"); - assertTrue(mLogger.watchdogEventOccurred(ROLLBACK_INITIATE, null, - REASON_APP_CRASH, TESTAPP_A)); - assertTrue(mLogger.watchdogEventOccurred(ROLLBACK_BOOT_TRIGGERED, null, - null, null)); - assertTrue(mLogger.watchdogEventOccurred(ROLLBACK_SUCCESS, null, null, null)); + assertThat(mLogger.watchdogEventOccurred(ROLLBACK_INITIATE, null, + REASON_APP_CRASH, TESTAPP_A)).isTrue(); + assertThat(mLogger.watchdogEventOccurred(ROLLBACK_BOOT_TRIGGERED, null, + null, null)).isTrue(); + assertThat(mLogger.watchdogEventOccurred(ROLLBACK_SUCCESS, null, null, null)).isTrue(); } @Test @@ -183,17 +181,17 @@ public class StagedRollbackTest extends BaseHostJUnit4Test { // 3. Staged rollback session becomes ready. // 4. Device actually reboots. // So we give a generous timeout here. - assertTrue(getDevice().waitForDeviceNotAvailable(TimeUnit.MINUTES.toMillis(5))); + assertThat(getDevice().waitForDeviceNotAvailable(TimeUnit.MINUTES.toMillis(5))).isTrue(); getDevice().waitForDeviceAvailable(); // verify rollback committed runPhase("testNativeWatchdogTriggersRollback_Phase3"); - assertTrue(mLogger.watchdogEventOccurred(ROLLBACK_INITIATE, null, - REASON_NATIVE_CRASH, null)); - assertTrue(mLogger.watchdogEventOccurred(ROLLBACK_BOOT_TRIGGERED, null, - null, null)); - assertTrue(mLogger.watchdogEventOccurred(ROLLBACK_SUCCESS, null, null, null)); + assertThat(mLogger.watchdogEventOccurred(ROLLBACK_INITIATE, null, + REASON_NATIVE_CRASH, null)).isTrue(); + assertThat(mLogger.watchdogEventOccurred(ROLLBACK_BOOT_TRIGGERED, null, + null, null)).isTrue(); + assertThat(mLogger.watchdogEventOccurred(ROLLBACK_SUCCESS, null, null, null)).isTrue(); } @Test @@ -223,17 +221,17 @@ public class StagedRollbackTest extends BaseHostJUnit4Test { // 3. Staged rollback session becomes ready. // 4. Device actually reboots. // So we give a generous timeout here. - assertTrue(getDevice().waitForDeviceNotAvailable(TimeUnit.MINUTES.toMillis(5))); + assertThat(getDevice().waitForDeviceNotAvailable(TimeUnit.MINUTES.toMillis(5))).isTrue(); getDevice().waitForDeviceAvailable(); // verify all available rollbacks have been committed runPhase("testNativeWatchdogTriggersRollbackForAll_Phase4"); - assertTrue(mLogger.watchdogEventOccurred(ROLLBACK_INITIATE, null, - REASON_NATIVE_CRASH, null)); - assertTrue(mLogger.watchdogEventOccurred(ROLLBACK_BOOT_TRIGGERED, null, - null, null)); - assertTrue(mLogger.watchdogEventOccurred(ROLLBACK_SUCCESS, null, null, null)); + assertThat(mLogger.watchdogEventOccurred(ROLLBACK_INITIATE, null, + REASON_NATIVE_CRASH, null)).isTrue(); + assertThat(mLogger.watchdogEventOccurred(ROLLBACK_BOOT_TRIGGERED, null, + null, null)).isTrue(); + assertThat(mLogger.watchdogEventOccurred(ROLLBACK_SUCCESS, null, null, null)).isTrue(); } /** @@ -306,16 +304,16 @@ public class StagedRollbackTest extends BaseHostJUnit4Test { // Verify apex was installed and then crash the apk runPhase("testRollbackApexWithApkCrashing_Phase2"); // Wait for crash to trigger rollback - assertTrue(getDevice().waitForDeviceNotAvailable(TimeUnit.MINUTES.toMillis(5))); + assertThat(getDevice().waitForDeviceNotAvailable(TimeUnit.MINUTES.toMillis(5))).isTrue(); getDevice().waitForDeviceAvailable(); // Verify rollback occurred due to crash of apk-in-apex runPhase("testRollbackApexWithApkCrashing_Phase3"); - assertTrue(mLogger.watchdogEventOccurred(ROLLBACK_INITIATE, null, - REASON_APP_CRASH, TESTAPP_A)); - assertTrue(mLogger.watchdogEventOccurred(ROLLBACK_BOOT_TRIGGERED, null, - null, null)); - assertTrue(mLogger.watchdogEventOccurred(ROLLBACK_SUCCESS, null, null, null)); + assertThat(mLogger.watchdogEventOccurred(ROLLBACK_INITIATE, null, + REASON_APP_CRASH, TESTAPP_A)).isTrue(); + assertThat(mLogger.watchdogEventOccurred(ROLLBACK_BOOT_TRIGGERED, null, + null, null)).isTrue(); + assertThat(mLogger.watchdogEventOccurred(ROLLBACK_SUCCESS, null, null, null)).isTrue(); } /** @@ -356,10 +354,10 @@ public class StagedRollbackTest extends BaseHostJUnit4Test { // Verify that old files have been restored and new files are gone runAsRoot(() -> { - assertEquals(TEST_STRING_1, getDevice().pullFileContents(oldFilePath1)); - assertEquals(TEST_STRING_2, getDevice().pullFileContents(oldFilePath2)); - assertNull(getDevice().pullFile(newFilePath3)); - assertNull(getDevice().pullFile(newFilePath4)); + assertThat(getDevice().pullFileContents(oldFilePath1)).isEqualTo(TEST_STRING_1); + assertThat(getDevice().pullFileContents(oldFilePath2)).isEqualTo(TEST_STRING_2); + assertThat(getDevice().pullFile(newFilePath3)).isNull(); + assertThat(getDevice().pullFile(newFilePath4)).isNull(); }); // Verify snapshots are deleted after restoration @@ -411,10 +409,10 @@ public class StagedRollbackTest extends BaseHostJUnit4Test { // Verify that old files have been restored and new files are gone runAsRoot(() -> { - assertEquals(TEST_STRING_1, getDevice().pullFileContents(oldFilePath1)); - assertEquals(TEST_STRING_2, getDevice().pullFileContents(oldFilePath2)); - assertNull(getDevice().pullFile(newFilePath3)); - assertNull(getDevice().pullFile(newFilePath4)); + assertThat(getDevice().pullFileContents(oldFilePath1)).isEqualTo(TEST_STRING_1); + assertThat(getDevice().pullFileContents(oldFilePath2)).isEqualTo(TEST_STRING_2); + assertThat(getDevice().pullFile(newFilePath3)).isNull(); + assertThat(getDevice().pullFile(newFilePath4)).isNull(); }); // Verify snapshots are deleted after restoration @@ -464,10 +462,10 @@ public class StagedRollbackTest extends BaseHostJUnit4Test { // Verify that old files have been restored and new files are gone runAsRoot(() -> { - assertEquals(TEST_STRING_1, getDevice().pullFileContents(oldFilePath1)); - assertEquals(TEST_STRING_2, getDevice().pullFileContents(oldFilePath2)); - assertNull(getDevice().pullFile(newFilePath3)); - assertNull(getDevice().pullFile(newFilePath4)); + assertThat(getDevice().pullFileContents(oldFilePath1)).isEqualTo(TEST_STRING_1); + assertThat(getDevice().pullFileContents(oldFilePath2)).isEqualTo(TEST_STRING_2); + assertThat(getDevice().pullFile(newFilePath3)).isNull(); + assertThat(getDevice().pullFile(newFilePath4)).isNull(); }); // Verify snapshots are deleted after restoration @@ -515,10 +513,10 @@ public class StagedRollbackTest extends BaseHostJUnit4Test { // Verify that old files have been restored and new files are gone runAsRoot(() -> { - assertEquals(TEST_STRING_1, getDevice().pullFileContents(oldFilePath1)); - assertEquals(TEST_STRING_2, getDevice().pullFileContents(oldFilePath2)); - assertNull(getDevice().pullFile(newFilePath3)); - assertNull(getDevice().pullFile(newFilePath4)); + assertThat(getDevice().pullFileContents(oldFilePath1)).isEqualTo(TEST_STRING_1); + assertThat(getDevice().pullFileContents(oldFilePath2)).isEqualTo(TEST_STRING_2); + assertThat(getDevice().pullFile(newFilePath3)).isNull(); + assertThat(getDevice().pullFile(newFilePath4)).isNull(); }); } @@ -549,7 +547,7 @@ public class StagedRollbackTest extends BaseHostJUnit4Test { runPhase("testCleanUp"); runAsRoot(() -> { for (String dir : after) { - assertNull(getDevice().getFileEntry(dir)); + assertThat(getDevice().getFileEntry(dir)).isNull(); } }); } @@ -561,7 +559,7 @@ public class StagedRollbackTest extends BaseHostJUnit4Test { try { getDevice().enableAdbRoot(); getDevice().remountSystemWritable(); - assertTrue(getDevice().pushFile(apex, "/system/apex/" + fileName)); + assertThat(getDevice().pushFile(apex, "/system/apex/" + fileName)).isTrue(); } finally { getDevice().disableAdbRoot(); } @@ -607,8 +605,9 @@ public class StagedRollbackTest extends BaseHostJUnit4Test { try { getDevice().enableAdbRoot(); IFileEntry file = getDevice().getFileEntry(path); - assertTrue("Not a directory: " + path, file.isDirectory()); - assertTrue("Directory not empty: " + path, file.getChildren(false).isEmpty()); + assertWithMessage("Not a directory: " + path).that(file.isDirectory()).isTrue(); + assertWithMessage("Directory not empty: " + path) + .that(file.getChildren(false)).isEmpty(); } catch (DeviceNotAvailableException e) { fail("Can't access directory: " + path); } finally { From 324a5d898dea83a024704db3425c1e1e3a1599f7 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Tue, 19 Jan 2021 14:05:24 +0800 Subject: [PATCH 2/2] Add a Subject to simplify WatchdogEventLogger assertion (2/n) It also gives more descriptive messages when assertion fails. Bug: 177861067 Test: atest StagedRollbackTest Change-Id: Id1340420f7e1f9f263ae3873a22339a0f9971d70 --- .../rollback/host/StagedRollbackTest.java | 34 ++++++++----------- .../rollback/host/WatchdogEventLogger.java | 28 +++++++++++++++ 2 files changed, 42 insertions(+), 20 deletions(-) diff --git a/tests/RollbackTest/StagedRollbackTest/src/com/android/tests/rollback/host/StagedRollbackTest.java b/tests/RollbackTest/StagedRollbackTest/src/com/android/tests/rollback/host/StagedRollbackTest.java index 55c59bb7fd65a..94950dc456cad 100644 --- a/tests/RollbackTest/StagedRollbackTest/src/com/android/tests/rollback/host/StagedRollbackTest.java +++ b/tests/RollbackTest/StagedRollbackTest/src/com/android/tests/rollback/host/StagedRollbackTest.java @@ -16,6 +16,8 @@ package com.android.tests.rollback.host; +import static com.android.tests.rollback.host.WatchdogEventLogger.Subject.assertThat; + import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; @@ -154,11 +156,9 @@ public class StagedRollbackTest extends BaseHostJUnit4Test { runPhase("testBadApkOnly_Phase4"); - assertThat(mLogger.watchdogEventOccurred(ROLLBACK_INITIATE, null, - REASON_APP_CRASH, TESTAPP_A)).isTrue(); - assertThat(mLogger.watchdogEventOccurred(ROLLBACK_BOOT_TRIGGERED, null, - null, null)).isTrue(); - assertThat(mLogger.watchdogEventOccurred(ROLLBACK_SUCCESS, null, null, null)).isTrue(); + assertThat(mLogger).eventOccurred(ROLLBACK_INITIATE, null, REASON_APP_CRASH, TESTAPP_A); + assertThat(mLogger).eventOccurred(ROLLBACK_BOOT_TRIGGERED, null, null, null); + assertThat(mLogger).eventOccurred(ROLLBACK_SUCCESS, null, null, null); } @Test @@ -187,11 +187,9 @@ public class StagedRollbackTest extends BaseHostJUnit4Test { // verify rollback committed runPhase("testNativeWatchdogTriggersRollback_Phase3"); - assertThat(mLogger.watchdogEventOccurred(ROLLBACK_INITIATE, null, - REASON_NATIVE_CRASH, null)).isTrue(); - assertThat(mLogger.watchdogEventOccurred(ROLLBACK_BOOT_TRIGGERED, null, - null, null)).isTrue(); - assertThat(mLogger.watchdogEventOccurred(ROLLBACK_SUCCESS, null, null, null)).isTrue(); + assertThat(mLogger).eventOccurred(ROLLBACK_INITIATE, null, REASON_NATIVE_CRASH, null); + assertThat(mLogger).eventOccurred(ROLLBACK_BOOT_TRIGGERED, null, null, null); + assertThat(mLogger).eventOccurred(ROLLBACK_SUCCESS, null, null, null); } @Test @@ -227,11 +225,9 @@ public class StagedRollbackTest extends BaseHostJUnit4Test { // verify all available rollbacks have been committed runPhase("testNativeWatchdogTriggersRollbackForAll_Phase4"); - assertThat(mLogger.watchdogEventOccurred(ROLLBACK_INITIATE, null, - REASON_NATIVE_CRASH, null)).isTrue(); - assertThat(mLogger.watchdogEventOccurred(ROLLBACK_BOOT_TRIGGERED, null, - null, null)).isTrue(); - assertThat(mLogger.watchdogEventOccurred(ROLLBACK_SUCCESS, null, null, null)).isTrue(); + assertThat(mLogger).eventOccurred(ROLLBACK_INITIATE, null, REASON_NATIVE_CRASH, null); + assertThat(mLogger).eventOccurred(ROLLBACK_BOOT_TRIGGERED, null, null, null); + assertThat(mLogger).eventOccurred(ROLLBACK_SUCCESS, null, null, null); } /** @@ -309,11 +305,9 @@ public class StagedRollbackTest extends BaseHostJUnit4Test { // Verify rollback occurred due to crash of apk-in-apex runPhase("testRollbackApexWithApkCrashing_Phase3"); - assertThat(mLogger.watchdogEventOccurred(ROLLBACK_INITIATE, null, - REASON_APP_CRASH, TESTAPP_A)).isTrue(); - assertThat(mLogger.watchdogEventOccurred(ROLLBACK_BOOT_TRIGGERED, null, - null, null)).isTrue(); - assertThat(mLogger.watchdogEventOccurred(ROLLBACK_SUCCESS, null, null, null)).isTrue(); + assertThat(mLogger).eventOccurred(ROLLBACK_INITIATE, null, REASON_APP_CRASH, TESTAPP_A); + assertThat(mLogger).eventOccurred(ROLLBACK_BOOT_TRIGGERED, null, null, null); + assertThat(mLogger).eventOccurred(ROLLBACK_SUCCESS, null, null, null); } /** diff --git a/tests/RollbackTest/lib/src/com/android/tests/rollback/host/WatchdogEventLogger.java b/tests/RollbackTest/lib/src/com/android/tests/rollback/host/WatchdogEventLogger.java index 6b0d1f8609564..8c16079dca85b 100644 --- a/tests/RollbackTest/lib/src/com/android/tests/rollback/host/WatchdogEventLogger.java +++ b/tests/RollbackTest/lib/src/com/android/tests/rollback/host/WatchdogEventLogger.java @@ -17,6 +17,8 @@ package com.android.tests.rollback.host; import com.android.tradefed.device.ITestDevice; +import com.google.common.truth.FailureMetadata; +import com.google.common.truth.Truth; import static com.google.common.truth.Truth.assertThat; @@ -76,4 +78,30 @@ public class WatchdogEventLogger { && matchProperty(type, "rollbackReason", rollbackReason) && matchProperty(type, "failedPackageName", failedPackageName); } + + static class Subject extends com.google.common.truth.Subject { + private final WatchdogEventLogger mActual; + + private Subject(FailureMetadata failureMetadata, WatchdogEventLogger subject) { + super(failureMetadata, subject); + mActual = subject; + } + + private static com.google.common.truth.Subject.Factory loggers() { + return Subject::new; + } + + static Subject assertThat(WatchdogEventLogger actual) { + return Truth.assertAbout(loggers()).that(actual); + } + + void eventOccurred(String type, String logPackage, String rollbackReason, + String failedPackageName) throws Exception { + check("watchdogEventOccurred(type=%s, logPackage=%s, rollbackReason=%s, " + + "failedPackageName=%s)", type, logPackage, rollbackReason, failedPackageName) + .that(mActual.watchdogEventOccurred(type, logPackage, rollbackReason, + failedPackageName)).isTrue(); + } + } }