From 110ea4cec0e3612afb0568e57546aee050340630 Mon Sep 17 00:00:00 2001 From: Lais Andrade Date: Mon, 8 Feb 2021 14:47:47 +0000 Subject: [PATCH] Fix flaky VibratorServiceTest Fix test to use wait on predicate instead of fixed sleep periods to avoid flakiness. Fix: 179487131 Test: VibratorServiceTest Change-Id: Ibbca594af9adfa0c9d106401824aa89a0050c60a --- .../android/server/VibratorServiceTest.java | 89 ++++++++----------- .../server/vibrator/VibrationThreadTest.java | 14 ++- 2 files changed, 42 insertions(+), 61 deletions(-) diff --git a/services/tests/servicestests/src/com/android/server/VibratorServiceTest.java b/services/tests/servicestests/src/com/android/server/VibratorServiceTest.java index 2a7905a451b98..633957a8b13a6 100644 --- a/services/tests/servicestests/src/com/android/server/VibratorServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/VibratorServiceTest.java @@ -50,6 +50,7 @@ import android.os.Looper; import android.os.PowerManagerInternal; import android.os.PowerSaveState; import android.os.Process; +import android.os.SystemClock; import android.os.UserHandle; import android.os.VibrationAttributes; import android.os.VibrationEffect; @@ -81,6 +82,7 @@ import java.util.Arrays; import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.function.Predicate; import java.util.stream.Collectors; /** @@ -92,6 +94,7 @@ import java.util.stream.Collectors; @Presubmit public class VibratorServiceTest { + private static final int TEST_TIMEOUT_MILLIS = 1_000; private static final int UID = Process.ROOT_UID; private static final int VIBRATOR_ID = 1; private static final String PACKAGE_NAME = "package"; @@ -342,8 +345,8 @@ public class VibratorServiceTest { verify(mIInputManagerMock).vibrate(eq(1), eq(effect), any()); // VibrationThread will start this vibration async, so wait before checking it never played. - Thread.sleep(10); - assertTrue(mVibratorProvider.getEffects().isEmpty()); + assertFalse(waitUntil(s -> !mVibratorProvider.getEffects().isEmpty(), service, + /* timeout= */ 20)); } @Test @@ -399,8 +402,8 @@ public class VibratorServiceTest { verify(mIInputManagerMock).vibrate(eq(1), any(), any()); // VibrationThread will start this vibration async, so wait before checking it never played. - Thread.sleep(10); - assertTrue(mVibratorProvider.getEffects().isEmpty()); + assertFalse(waitUntil(s -> !mVibratorProvider.getEffects().isEmpty(), service, + /* timeout= */ 20)); } @Test @@ -409,16 +412,10 @@ public class VibratorServiceTest { mRegisteredPowerModeListener.onLowPowerModeChanged(NORMAL_POWER_STATE); vibrate(service, VibrationEffect.createOneShot(1000, 100), HAPTIC_FEEDBACK_ATTRS); - - // VibrationThread will start this vibration async, so wait before triggering callbacks. - Thread.sleep(10); - assertTrue(service.isVibrating()); + assertTrue(waitUntil(s -> s.isVibrating(), service, TEST_TIMEOUT_MILLIS)); mRegisteredPowerModeListener.onLowPowerModeChanged(LOW_POWER_STATE); - - // Wait for callback to cancel vibration. - Thread.sleep(10); - assertFalse(service.isVibrating()); + assertTrue(waitUntil(s -> !s.isVibrating(), service, TEST_TIMEOUT_MILLIS)); } @Test @@ -427,26 +424,18 @@ public class VibratorServiceTest { mRegisteredPowerModeListener.onLowPowerModeChanged(NORMAL_POWER_STATE); vibrate(service, VibrationEffect.createOneShot(1000, 100), RINGTONE_ATTRS); - - // VibrationThread will start this vibration async, so wait before triggering callbacks. - Thread.sleep(10); - assertTrue(service.isVibrating()); + assertTrue(waitUntil(s -> s.isVibrating(), service, TEST_TIMEOUT_MILLIS)); mRegisteredPowerModeListener.onLowPowerModeChanged(LOW_POWER_STATE); - - // Wait for callback to cancel vibration. - Thread.sleep(10); - assertTrue(service.isVibrating()); + // Settings callback is async, so wait before checking it never got cancelled. + assertFalse(waitUntil(s -> !s.isVibrating(), service, /* timeout= */ 20)); } @Test public void vibrate_withSettingsChanged_doNotCancelVibration() throws Exception { VibratorService service = createService(); vibrate(service, VibrationEffect.createOneShot(1000, 100), HAPTIC_FEEDBACK_ATTRS); - - // VibrationThread will start this vibration async, so wait before triggering callbacks. - Thread.sleep(10); - assertTrue(service.isVibrating()); + assertTrue(waitUntil(s -> s.isVibrating(), service, TEST_TIMEOUT_MILLIS)); setUserSetting(Settings.System.NOTIFICATION_VIBRATION_INTENSITY, Vibrator.VIBRATION_INTENSITY_MEDIUM); @@ -454,9 +443,8 @@ public class VibratorServiceTest { // FakeSettingsProvider don't support testing triggering ContentObserver yet. service.updateVibrators(); - // Wait for callback to cancel vibration. - Thread.sleep(10); - assertTrue(service.isVibrating()); + // Settings callback is async, so wait before checking it never got cancelled. + assertFalse(waitUntil(s -> !s.isVibrating(), service, /* timeout= */ 20)); } @Test @@ -488,8 +476,8 @@ public class VibratorServiceTest { inOrderVerifier.verify(mIInputManagerMock).vibrate(eq(2), eq(effect), any()); // VibrationThread will start this vibration async, so wait before checking it never played. - Thread.sleep(10); - assertTrue(mVibratorProvider.getEffects().isEmpty()); + assertFalse(waitUntil(s -> !mVibratorProvider.getEffects().isEmpty(), service, + /* timeout= */ 20)); } @Test @@ -521,8 +509,8 @@ public class VibratorServiceTest { verify(mIInputManagerMock).vibrate(eq(1), eq(effect), any()); // VibrationThread will start this vibration async, so wait before checking it never played. - Thread.sleep(10); - assertTrue(mVibratorProvider.getEffects().isEmpty()); + assertFalse(waitUntil(s -> !mVibratorProvider.getEffects().isEmpty(), service, + /* timeout= */ 20)); } @Test @@ -531,18 +519,12 @@ public class VibratorServiceTest { VibratorService service = createService(); vibrate(service, VibrationEffect.get(VibrationEffect.EFFECT_CLICK), ALARM_ATTRS); - - // VibrationThread will start this vibration async, so wait before triggering callbacks. - Thread.sleep(10); - assertTrue(service.isVibrating()); + assertTrue(waitUntil(s -> s.isVibrating(), service, TEST_TIMEOUT_MILLIS)); // Trigger callbacks from controller. mTestLooper.moveTimeForward(50); mTestLooper.dispatchAll(); - - // VibrationThread needs some time to react to native callbacks and stop the vibrator. - Thread.sleep(10); - assertFalse(service.isVibrating()); + assertTrue(waitUntil(s -> !s.isVibrating(), service, TEST_TIMEOUT_MILLIS)); } @Test @@ -550,16 +532,10 @@ public class VibratorServiceTest { VibratorService service = createService(); vibrate(service, VibrationEffect.createOneShot(100, 100), ALARM_ATTRS); - - // VibrationThread will start this vibration async, so wait before checking. - Thread.sleep(10); - assertTrue(service.isVibrating()); + assertTrue(waitUntil(s -> s.isVibrating(), service, TEST_TIMEOUT_MILLIS)); service.cancelVibrate(service); - - // VibrationThread will stop this vibration async, so wait before checking. - Thread.sleep(10); - assertFalse(service.isVibrating()); + assertTrue(waitUntil(s -> !s.isVibrating(), service, TEST_TIMEOUT_MILLIS)); } @Test @@ -584,17 +560,13 @@ public class VibratorServiceTest { service.registerVibratorStateListener(mVibratorStateListenerMock); vibrate(service, VibrationEffect.createOneShot(30, 100), ALARM_ATTRS); - - // VibrationThread will start this vibration async, so wait before triggering callbacks. - Thread.sleep(10); - assertTrue(service.isVibrating()); + assertTrue(waitUntil(s -> s.isVibrating(), service, TEST_TIMEOUT_MILLIS)); service.unregisterVibratorStateListener(mVibratorStateListenerMock); // Trigger callbacks from controller. mTestLooper.moveTimeForward(50); mTestLooper.dispatchAll(); - Thread.sleep(20); - assertFalse(service.isVibrating()); + assertTrue(waitUntil(s -> !s.isVibrating(), service, TEST_TIMEOUT_MILLIS)); InOrder inOrderVerifier = inOrder(mVibratorStateListenerMock); // First notification done when listener is registered. @@ -771,4 +743,15 @@ public class VibratorServiceTest { private void setGlobalSetting(String settingName, int value) { Settings.Global.putInt(mContextSpy.getContentResolver(), settingName, value); } + + private boolean waitUntil(Predicate predicate, + VibratorService service, long timeout) throws InterruptedException { + long timeoutTimestamp = SystemClock.uptimeMillis() + timeout; + boolean predicateResult = false; + while (!predicateResult && SystemClock.uptimeMillis() < timeoutTimestamp) { + Thread.sleep(10); + predicateResult = predicate.test(service); + } + return predicateResult; + } } diff --git a/services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java b/services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java index d876b05af3d56..7d208799ee88d 100644 --- a/services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java +++ b/services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java @@ -229,9 +229,9 @@ public class VibrationThreadTest { .compose(); VibrationThread vibrationThread = startThreadAndDispatcher(vibrationId, effect); - Thread.sleep(20); + assertTrue(waitUntil(t -> t.getVibrators().get(VIBRATOR_ID).isVibrating(), vibrationThread, + TEST_TIMEOUT_MILLIS)); assertTrue(vibrationThread.isAlive()); - assertTrue(vibrationThread.getVibrators().get(VIBRATOR_ID).isVibrating()); // Run cancel in a separate thread so if VibrationThread.cancel blocks then this test should // fail at waitForCompletion(vibrationThread) if the vibration not cancelled immediately. @@ -254,9 +254,9 @@ public class VibrationThreadTest { VibrationEffect effect = VibrationEffect.createWaveform(new long[]{100}, new int[]{100}, 0); VibrationThread vibrationThread = startThreadAndDispatcher(vibrationId, effect); - Thread.sleep(20); + assertTrue(waitUntil(t -> t.getVibrators().get(VIBRATOR_ID).isVibrating(), vibrationThread, + TEST_TIMEOUT_MILLIS)); assertTrue(vibrationThread.isAlive()); - assertTrue(vibrationThread.getVibrators().get(VIBRATOR_ID).isVibrating()); // Run cancel in a separate thread so if VibrationThread.cancel blocks then this test should // fail at waitForCompletion(vibrationThread) if the vibration not cancelled immediately. @@ -621,10 +621,8 @@ public class VibrationThreadTest { .combine(); VibrationThread vibrationThread = startThreadAndDispatcher(vibrationId, effect); - Thread.sleep(10); - assertTrue(vibrationThread.isAlive()); - assertTrue(vibrationThread.getVibrators().get(1).isVibrating()); - assertTrue(vibrationThread.getVibrators().get(2).isVibrating()); + assertTrue(waitUntil(t -> t.getVibrators().get(2).isVibrating(), vibrationThread, + TEST_TIMEOUT_MILLIS)); // Run cancel in a separate thread so if VibrationThread.cancel blocks then this test should // fail at waitForCompletion(vibrationThread) if the vibration not cancelled immediately.