Commit 820293d1 authored by Clemens Arbesser's avatar Clemens Arbesser Committed by Commit Bot

[Autofill Assistant] Fix flaky/failing UI tests.

This CL fixes two bugs that caused flaky AA tests involving the bottom sheet:
- First, the tests were waiting for |isDisplayed()| and then immediately calling |perform(click())|. This is flaky, because |isDisplayed()| succeeds with widets that are >= 50% on screen, but |perform(click))| requires >= 90%. During the initial bottom sheet animation, it was possible to click the button before it was completely visible.
- Second, and more importantly, |waitUntilViewMatchesCondition| did not correctly wait for the full duration. Rather, if a matching view exists that did not fulfill the specified condition, it would erroneously return, because the thrown exception in that case was an |AssertionError| rather than a |NoMatchingViewException|.

b/142317318

Bug: 990118
Change-Id: I0c47e92ac8e77ae804eff5f05ac2c80ce9d77082
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1848372Reviewed-by: default avatarStephane Zermatten <szermatt@chromium.org>
Commit-Queue: Clemens Arbesser <arbesser@google.com>
Cr-Commit-Position: refs/heads/master@{#704122}
parent 5c86959c
...@@ -8,6 +8,7 @@ import static android.support.test.espresso.Espresso.onView; ...@@ -8,6 +8,7 @@ import static android.support.test.espresso.Espresso.onView;
import static android.support.test.espresso.action.ViewActions.click; import static android.support.test.espresso.action.ViewActions.click;
import static android.support.test.espresso.assertion.ViewAssertions.matches; import static android.support.test.espresso.assertion.ViewAssertions.matches;
import static android.support.test.espresso.matcher.ViewMatchers.assertThat; import static android.support.test.espresso.matcher.ViewMatchers.assertThat;
import static android.support.test.espresso.matcher.ViewMatchers.isCompletelyDisplayed;
import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed; import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed;
import static android.support.test.espresso.matcher.ViewMatchers.withId; import static android.support.test.espresso.matcher.ViewMatchers.withId;
...@@ -34,7 +35,6 @@ import org.mockito.junit.MockitoJUnit; ...@@ -34,7 +35,6 @@ import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule; import org.mockito.junit.MockitoRule;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.base.ThreadUtils;
import org.chromium.base.test.util.CommandLineFlags; import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.DisableIf; import org.chromium.base.test.util.DisableIf;
import org.chromium.chrome.autofill_assistant.R; import org.chromium.chrome.autofill_assistant.R;
...@@ -47,6 +47,7 @@ import org.chromium.chrome.browser.customtabs.CustomTabActivityTestRule; ...@@ -47,6 +47,7 @@ import org.chromium.chrome.browser.customtabs.CustomTabActivityTestRule;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController; import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner; import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
...@@ -75,7 +76,7 @@ public class AssistantOnboardingCoordinatorTest { ...@@ -75,7 +76,7 @@ public class AssistantOnboardingCoordinatorTest {
public void setUp() throws Exception { public void setUp() throws Exception {
AutofillAssistantUiTestUtil.startOnBlankPage(mCustomTabActivityTestRule); AutofillAssistantUiTestUtil.startOnBlankPage(mCustomTabActivityTestRule);
mActivity = mCustomTabActivityTestRule.getActivity(); mActivity = mCustomTabActivityTestRule.getActivity();
mBottomSheetController = ThreadUtils.runOnUiThreadBlocking( mBottomSheetController = TestThreadUtils.runOnUiThreadBlocking(
() -> AutofillAssistantUiTestUtil.createBottomSheetController(mActivity)); () -> AutofillAssistantUiTestUtil.createBottomSheetController(mActivity));
mTab = mActivity.getTabModelSelector().getCurrentTab(); mTab = mActivity.getTabModelSelector().getCurrentTab();
} }
...@@ -107,13 +108,13 @@ public class AssistantOnboardingCoordinatorTest { ...@@ -107,13 +108,13 @@ public class AssistantOnboardingCoordinatorTest {
AssistantOnboardingCoordinator coordinator = createCoordinator(mTab); AssistantOnboardingCoordinator coordinator = createCoordinator(mTab);
showOnboardingAndWait(coordinator, mCallback); showOnboardingAndWait(coordinator, mCallback);
assertTrue(ThreadUtils.runOnUiThreadBlocking(coordinator::isInProgress)); assertTrue(TestThreadUtils.runOnUiThreadBlocking(coordinator::isInProgress));
onView(is(mActivity.getScrim())).check(matches(isDisplayed())); onView(is(mActivity.getScrim())).check(matches(isDisplayed()));
onView(withId(buttonToClick)).perform(click()); onView(withId(buttonToClick)).perform(click());
verify(mCallback).onResult(expectAccept); verify(mCallback).onResult(expectAccept);
assertFalse(ThreadUtils.runOnUiThreadBlocking(coordinator::isInProgress)); assertFalse(TestThreadUtils.runOnUiThreadBlocking(coordinator::isInProgress));
assertEquals(expectAccept, AutofillAssistantPreferencesUtil.isAutofillOnboardingAccepted()); assertEquals(expectAccept, AutofillAssistantPreferencesUtil.isAutofillOnboardingAccepted());
} }
...@@ -132,7 +133,7 @@ public class AssistantOnboardingCoordinatorTest { ...@@ -132,7 +133,7 @@ public class AssistantOnboardingCoordinatorTest {
@Test @Test
@MediumTest @MediumTest
@DisableIf.Build(sdk_is_greater_than = 22) // TODO(crbug/990118): re-enable @DisableIf.Build(sdk_is_greater_than = 22) // TODO(crbug/990118): re-enable
public void testTransfertControls() throws Exception { public void testTransferControls() throws Exception {
AssistantOnboardingCoordinator coordinator = createCoordinator(mTab); AssistantOnboardingCoordinator coordinator = createCoordinator(mTab);
List<AssistantOverlayCoordinator> capturedOverlays = List<AssistantOverlayCoordinator> capturedOverlays =
...@@ -141,7 +142,7 @@ public class AssistantOnboardingCoordinatorTest { ...@@ -141,7 +142,7 @@ public class AssistantOnboardingCoordinatorTest {
(accepted) -> { capturedOverlays.add(coordinator.transferControls()); }); (accepted) -> { capturedOverlays.add(coordinator.transferControls()); });
onView(withId(R.id.button_init_ok)).perform(click()); onView(withId(R.id.button_init_ok)).perform(click());
assertFalse(ThreadUtils.runOnUiThreadBlocking(coordinator::isInProgress)); assertFalse(TestThreadUtils.runOnUiThreadBlocking(coordinator::isInProgress));
// An overlay was captured, and it is still shown. // An overlay was captured, and it is still shown.
onView(is(mActivity.getScrim())).check(matches(isDisplayed())); onView(is(mActivity.getScrim())).check(matches(isDisplayed()));
...@@ -159,7 +160,7 @@ public class AssistantOnboardingCoordinatorTest { ...@@ -159,7 +160,7 @@ public class AssistantOnboardingCoordinatorTest {
/** Trigger onboarding and wait until it is fully displayed. */ /** Trigger onboarding and wait until it is fully displayed. */
private void showOnboardingAndWait( private void showOnboardingAndWait(
AssistantOnboardingCoordinator coordinator, Callback<Boolean> callback) { AssistantOnboardingCoordinator coordinator, Callback<Boolean> callback) {
ThreadUtils.runOnUiThreadBlocking(() -> coordinator.show(callback)); TestThreadUtils.runOnUiThreadBlocking(() -> coordinator.show(callback));
waitUntilViewMatchesCondition(withId(R.id.button_init_ok), isDisplayed()); waitUntilViewMatchesCondition(withId(R.id.button_init_ok), isCompletelyDisplayed());
} }
} }
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
package org.chromium.chrome.browser.autofill_assistant; package org.chromium.chrome.browser.autofill_assistant;
import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed; import static android.support.test.espresso.matcher.ViewMatchers.isCompletelyDisplayed;
import static android.support.test.espresso.matcher.ViewMatchers.withText; import static android.support.test.espresso.matcher.ViewMatchers.withText;
import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiTestUtil.startAutofillAssistant; import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiTestUtil.startAutofillAssistant;
...@@ -76,6 +76,6 @@ public class AutofillAssistantAutostartTest { ...@@ -76,6 +76,6 @@ public class AutofillAssistantAutostartTest {
new AutofillAssistantTestService(Collections.singletonList(script)); new AutofillAssistantTestService(Collections.singletonList(script));
startAutofillAssistant(mTestRule.getActivity(), testService); startAutofillAssistant(mTestRule.getActivity(), testService);
waitUntilViewMatchesCondition(withText("Hello World!"), isDisplayed()); waitUntilViewMatchesCondition(withText("Hello World!"), isCompletelyDisplayed());
} }
} }
...@@ -6,7 +6,7 @@ package org.chromium.chrome.browser.autofill_assistant; ...@@ -6,7 +6,7 @@ package org.chromium.chrome.browser.autofill_assistant;
import static android.support.test.espresso.Espresso.onView; import static android.support.test.espresso.Espresso.onView;
import static android.support.test.espresso.action.ViewActions.click; import static android.support.test.espresso.action.ViewActions.click;
import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed; import static android.support.test.espresso.matcher.ViewMatchers.isCompletelyDisplayed;
import static android.support.test.espresso.matcher.ViewMatchers.withId; import static android.support.test.espresso.matcher.ViewMatchers.withId;
import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.contains;
...@@ -29,7 +29,6 @@ import org.junit.runner.RunWith; ...@@ -29,7 +29,6 @@ import org.junit.runner.RunWith;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.base.ThreadUtils;
import org.chromium.base.test.util.CallbackHelper; import org.chromium.base.test.util.CallbackHelper;
import org.chromium.base.test.util.CommandLineFlags; import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.DisableIf; import org.chromium.base.test.util.DisableIf;
...@@ -43,6 +42,7 @@ import org.chromium.chrome.browser.directactions.DirectActionReporter.Type; ...@@ -43,6 +42,7 @@ import org.chromium.chrome.browser.directactions.DirectActionReporter.Type;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController; import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
import org.chromium.chrome.test.ChromeActivityTestRule; import org.chromium.chrome.test.ChromeActivityTestRule;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner; import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
...@@ -66,7 +66,7 @@ public class AutofillAssistantDirectActionHandlerTest { ...@@ -66,7 +66,7 @@ public class AutofillAssistantDirectActionHandlerTest {
mActivityTestRule.startMainActivityOnBlankPage(); mActivityTestRule.startMainActivityOnBlankPage();
mActivity = mActivityTestRule.getActivity(); mActivity = mActivityTestRule.getActivity();
mBottomSheetController = ThreadUtils.runOnUiThreadBlocking( mBottomSheetController = TestThreadUtils.runOnUiThreadBlocking(
() -> AutofillAssistantUiTestUtil.createBottomSheetController(mActivity)); () -> AutofillAssistantUiTestUtil.createBottomSheetController(mActivity));
mModuleEntryProvider = new TestingAutofillAssistantModuleEntryProvider(); mModuleEntryProvider = new TestingAutofillAssistantModuleEntryProvider();
mModuleEntryProvider.setCannotInstall(); mModuleEntryProvider.setCannotInstall();
...@@ -161,7 +161,7 @@ public class AutofillAssistantDirectActionHandlerTest { ...@@ -161,7 +161,7 @@ public class AutofillAssistantDirectActionHandlerTest {
WaitingCallback<Boolean> onboardingCallback = WaitingCallback<Boolean> onboardingCallback =
performActionAsync("onboarding", Bundle.EMPTY); performActionAsync("onboarding", Bundle.EMPTY);
waitUntilViewMatchesCondition(withId(R.id.button_init_ok), isDisplayed()); waitUntilViewMatchesCondition(withId(R.id.button_init_ok), isCompletelyDisplayed());
assertFalse(onboardingCallback.hasResult()); assertFalse(onboardingCallback.hasResult());
onView(withId(R.id.button_init_ok)).perform(click()); onView(withId(R.id.button_init_ok)).perform(click());
...@@ -174,7 +174,7 @@ public class AutofillAssistantDirectActionHandlerTest { ...@@ -174,7 +174,7 @@ public class AutofillAssistantDirectActionHandlerTest {
/** Calls list_assistant_actions and returns the result. */ /** Calls list_assistant_actions and returns the result. */
private List<String> listActions() throws Exception { private List<String> listActions() throws Exception {
WaitingCallback<Bundle> callback = new WaitingCallback<Bundle>(); WaitingCallback<Bundle> callback = new WaitingCallback<Bundle>();
assertTrue(ThreadUtils.runOnUiThreadBlocking( assertTrue(TestThreadUtils.runOnUiThreadBlocking(
() ()
-> mHandler.performDirectAction( -> mHandler.performDirectAction(
"list_assistant_actions", Bundle.EMPTY, callback))); "list_assistant_actions", Bundle.EMPTY, callback)));
...@@ -196,7 +196,7 @@ public class AutofillAssistantDirectActionHandlerTest { ...@@ -196,7 +196,7 @@ public class AutofillAssistantDirectActionHandlerTest {
WaitingCallback<Boolean> callback = new WaitingCallback<Boolean>(); WaitingCallback<Boolean> callback = new WaitingCallback<Boolean>();
Bundle allArguments = new Bundle(arguments); Bundle allArguments = new Bundle(arguments);
if (!name.isEmpty()) allArguments.putString("name", name); if (!name.isEmpty()) allArguments.putString("name", name);
ThreadUtils.runOnUiThreadBlocking( TestThreadUtils.runOnUiThreadBlocking(
() ()
-> mHandler.performDirectAction("perform_assistant_action", allArguments, -> mHandler.performDirectAction("perform_assistant_action", allArguments,
(bundle) -> callback.onResult(bundle.getBoolean("success")))); (bundle) -> callback.onResult(bundle.getBoolean("success"))));
......
...@@ -149,7 +149,7 @@ class AutofillAssistantUiTestUtil { ...@@ -149,7 +149,7 @@ class AutofillAssistantUiTestUtil {
try { try {
onView(matcher).check(matches(condition)); onView(matcher).check(matches(condition));
return true; return true;
} catch (NoMatchingViewException e) { } catch (NoMatchingViewException | AssertionError e) {
// Note: all other exceptions are let through, in particular // Note: all other exceptions are let through, in particular
// AmbiguousViewMatcherException. // AmbiguousViewMatcherException.
return false; return false;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment