diff --git a/espresso/CHANGELOG.md b/espresso/CHANGELOG.md index 89a4f06b1..8d9140570 100644 --- a/espresso/CHANGELOG.md +++ b/espresso/CHANGELOG.md @@ -19,6 +19,7 @@ The following artifacts were released: * Replace now-unnecessary reflection from TestLooperManagerCompat when using Android SDK 36 APIs * Don't suppress AppNotIdleException if dumpThreadStates throws. * Remove Espresso.onIdle tracing +* Fix NullPointerException in UiControllerImpl. **New Features** diff --git a/espresso/core/java/androidx/test/espresso/base/UiControllerImpl.java b/espresso/core/java/androidx/test/espresso/base/UiControllerImpl.java index 88ea0cb94..770647f4f 100644 --- a/espresso/core/java/androidx/test/espresso/base/UiControllerImpl.java +++ b/espresso/core/java/androidx/test/espresso/base/UiControllerImpl.java @@ -35,7 +35,6 @@ import androidx.test.espresso.IdlingPolicies; import androidx.test.espresso.IdlingPolicy; import androidx.test.espresso.InjectEventSecurityException; -import androidx.test.espresso.UiController; import androidx.test.espresso.base.IdlingResourceRegistry.IdleNotificationCallback; import androidx.test.espresso.util.StringJoinerKt; import androidx.test.espresso.util.concurrent.ThreadFactoryBuilder; @@ -77,7 +76,8 @@ enum IdleCondition { COMPAT_TASKS_HAVE_IDLED, KEY_INJECT_HAS_COMPLETED, MOTION_INJECTION_HAS_COMPLETED, - DYNAMIC_TASKS_HAVE_IDLED; + DYNAMIC_TASKS_HAVE_IDLED, + POSSIBLE_RACE_CONDITION_DETECTED; /** Checks whether this condition has been signaled. */ public boolean isSignaled(BitSet conditionSet) { @@ -509,8 +509,9 @@ private IdleNotifier loopUntil( start + masterIdlePolicy.getIdleTimeoutUnit().toMillis(masterIdlePolicy.getIdleTimeout()); interrogation = new MainThreadInterrogation(conditions, conditionSet, end); + Interrogator interrogator = new Interrogator(); InterrogationStatus result = - new Interrogator().loopAndInterrogate(testLooperManager, interrogation); + interrogator.loopAndInterrogate(testLooperManager, interrogation); if (InterrogationStatus.COMPLETED == result) { // did not time out, all conditions happy. return dynamicIdle; @@ -551,12 +552,51 @@ private IdleNotifier loopUntil( } List busyResources = idlingResourceRegistry.getBusyResources(); + + if (busyResources == null) { + Log.w( + TAG, "Possible race condition detected, looping until race buster is complete"); + // Null return value indicates a possible race condition. + // Flush the queue to allow resolution to take place. + controllerHandler.sendMessage( + IdleCondition.POSSIBLE_RACE_CONDITION_DETECTED.createSignal( + controllerHandler, generation)); + + // New interrogation to reset timeout and wait only for our signal. + // Since we are just emptying the queue this should not ever timeout. + start = SystemClock.uptimeMillis(); + end = + start + + masterIdlePolicy + .getIdleTimeoutUnit() + .toMillis(masterIdlePolicy.getIdleTimeout()); + interrogation = + new MainThreadInterrogation( + EnumSet.of(IdleCondition.POSSIBLE_RACE_CONDITION_DETECTED), + conditionSet, + end); + InterrogationStatus raceBusterResult = + interrogator.loopAndInterrogate(testLooperManager, interrogation); + if (raceBusterResult == InterrogationStatus.COMPLETED) { + // Can still return null if resource is buggy. + busyResources = idlingResourceRegistry.getBusyResources(); + } else { + Log.w( + TAG, + String.format( + Locale.ROOT, + "Failed to resolve possible race condition: " + raceBusterResult)); + } + } + conditionName = String.format( Locale.ROOT, "%s(busy resources=%s)", conditionName, - StringJoinerKt.joinToString(busyResources, ",")); + busyResources != null + ? StringJoinerKt.joinToString(busyResources, ",") + : "null"); break; default: break; @@ -588,6 +628,7 @@ private IdleNotifier loopUntil( } return dynamicIdle; } + @Override public void interruptEspressoTasks() { controllerHandler.post( diff --git a/espresso/core/javatests/androidx/test/espresso/base/BUILD b/espresso/core/javatests/androidx/test/espresso/base/BUILD index 9eb122ca5..f43944c30 100644 --- a/espresso/core/javatests/androidx/test/espresso/base/BUILD +++ b/espresso/core/javatests/androidx/test/espresso/base/BUILD @@ -236,7 +236,6 @@ axt_android_library_test( "//espresso/core/java/androidx/test/espresso/base:default_failure_handler", "//espresso/core/java/androidx/test/espresso/base:idling_resource_registry", "//ext/junit", - "//opensource/androidx:annotation", "//runner/android_junit_runner/java/androidx/test:runner", "//testapps/ui_testapp/java/androidx/test/ui/app:lib_exported", "//testapps/ui_testapp/javatests/androidx/test/ui/app:test_resources", diff --git a/espresso/core/javatests/androidx/test/espresso/base/UiControllerImplTest.java b/espresso/core/javatests/androidx/test/espresso/base/UiControllerImplTest.java index 2a2062cab..e450d8e4c 100644 --- a/espresso/core/javatests/androidx/test/espresso/base/UiControllerImplTest.java +++ b/espresso/core/javatests/androidx/test/espresso/base/UiControllerImplTest.java @@ -20,12 +20,17 @@ import static junit.framework.Assert.assertTrue; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; import android.os.Handler; import android.os.Looper; import android.os.Message; +import android.os.SystemClock; import android.util.Log; +import androidx.test.espresso.AppNotIdleException; +import androidx.test.espresso.IdlingPolicies; +import androidx.test.espresso.IdlingPolicy; import androidx.test.espresso.IdlingResourceTimeoutException; import androidx.test.espresso.base.IdlingResourceRegistry.IdleNotificationCallback; import androidx.test.ext.junit.runners.AndroidJUnit4; @@ -371,6 +376,63 @@ public void run() { assertTrue("App should be idle.", latch.await(5, TimeUnit.SECONDS)); } + @Test + public void loopMainThreadUntilIdle_racyIdleResource() throws InterruptedException { + IdlingPolicies.setMasterPolicyTimeout(10, TimeUnit.SECONDS); + OnDemandIdlingResource fakeResource = new OnDemandIdlingResource("FakeResource"); + idlingResourceRegistry.registerResources(singletonList(fakeResource)); + final CountDownLatch startedLatch = new CountDownLatch(1); + final CountDownLatch interruptedLatch = new CountDownLatch(1); + assertTrue( + testThread + .getHandler() + .post( + () -> { + // This is the sequence of events we want: + // - Resource starts busy. + // - Resource becomes idle, event posted to queue to update idle registry. + // - Timeout occurs, no more events will be processed. + // - Espresso detects race condition while checking if the resource is idle. + IdlingPolicy masterIdlingPolicy = IdlingPolicies.getMasterIdlingPolicy(); + long expectedTimeout = + SystemClock.uptimeMillis() + + masterIdlingPolicy + .getIdleTimeoutUnit() + .toMillis(masterIdlingPolicy.getIdleTimeout()); + testThread + .getHandler() + .postAtTime( + () -> { + Log.i(TAG, "Forcing resource to be idle."); + fakeResource.forceIdleNow(); + // Busy wait until after the timeout to ensure race buster cannot run. + try { + Thread.sleep(200); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + assertTrue( + "Sleep should bring us past the timeout.", + SystemClock.uptimeMillis() > expectedTimeout); + }, + expectedTimeout - 100); + + Log.i(TAG, "Hijacking thread and looping it."); + startedLatch.countDown(); + assertThrows( + "Expected for loopMainThreadUntilIdle to be interrupted", + AppNotIdleException.class, + () -> uiController.get().loopMainThreadUntilIdle()); + interruptedLatch.countDown(); + })); + + assertTrue("looper has started.", startedLatch.await(2, TimeUnit.SECONDS)); + assertFalse( + "Should not have stopped looping the main thread yet!", + interruptedLatch.await(2, TimeUnit.SECONDS)); + assertTrue("App should be interrupted.", interruptedLatch.await(20, TimeUnit.SECONDS)); + } + @Test public void loopMainThreadUntilIdle_multipleIdlingResources() throws InterruptedException { OnDemandIdlingResource fakeResource1 = new OnDemandIdlingResource("FakeResource1");