Commit aeb0de46 authored by Pavel Yatsuk's avatar Pavel Yatsuk Committed by Commit Bot

Fix some ChromeTabModalPresenterTest flaky tests

When test scenario involves tab switching, the toolbar is animated into
view. ChromeTabModalPresenter has a logic that prevents it from running
enter animation when toolbar animation is not finished (browser controls
are not fully visible). Because of this logic modal dialog view is not
added to container view immediately in ModalDialogManager.showDialog
call.

On the other hand Espresso's onView(...).check() call verifies condition
immediately and asserts if the condition is not met (descendant with a
particular text is not present in container view). This causes
occasional test failures. Semantically test should verify that a dialog
with particular text is shown at some point.

Container view visibility it closest approximation to a condition that
modal dialog view is added to the container. In this CL I add helper
that polls UI thread waiting for dialog container to become visible.

BUG=1030683,1030903,1031092
R=twellington@chromium.org

Change-Id: Id1a42aefa45ee50d26171ecbff662c44226ad056
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2134401Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Commit-Queue: Pavel Yatsuk <pavely@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756468}
parent 8d22839b
...@@ -38,7 +38,6 @@ import org.junit.runner.RunWith; ...@@ -38,7 +38,6 @@ import org.junit.runner.RunWith;
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.DisabledTest;
import org.chromium.base.test.util.Feature; import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.Restriction; import org.chromium.base.test.util.Restriction;
import org.chromium.chrome.R; import org.chromium.chrome.R;
...@@ -127,14 +126,11 @@ public class ChromeTabModalPresenterTest { ...@@ -127,14 +126,11 @@ public class ChromeTabModalPresenterTest {
createDialog(mActivity, mActivity.getModalDialogManager(), "1", null); createDialog(mActivity, mActivity.getModalDialogManager(), "1", null);
showDialog(mManager, dialog1, ModalDialogType.TAB); showDialog(mManager, dialog1, ModalDialogType.TAB);
ChromeTabModalPresenter presenter = final View dialogContainer = mTabModalPresenter.getDialogContainerForTest();
(ChromeTabModalPresenter) mManager.getPresenterForTest(ModalDialogType.TAB);
final View dialogContainer = presenter.getDialogContainerForTest();
final View controlContainer = mActivity.findViewById(R.id.control_container); final View controlContainer = mActivity.findViewById(R.id.control_container);
final ViewGroup containerParent = presenter.getContainerParentForTest(); final ViewGroup containerParent = mTabModalPresenter.getContainerParentForTest();
CriteriaHelper.pollUiThread( ensureDialogContainerVisible();
Criteria.equals(View.VISIBLE, () -> dialogContainer.getVisibility()));
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
Assert.assertThat(containerParent.indexOfChild(dialogContainer), Assert.assertThat(containerParent.indexOfChild(dialogContainer),
...@@ -220,6 +216,7 @@ public class ChromeTabModalPresenterTest { ...@@ -220,6 +216,7 @@ public class ChromeTabModalPresenterTest {
mTestObserver.onTabInteractabilityChangedCallback.waitForCallback(callCount); mTestObserver.onTabInteractabilityChangedCallback.waitForCallback(callCount);
checkPendingSize(mManager, ModalDialogType.TAB, 1); checkPendingSize(mManager, ModalDialogType.TAB, 1);
ensureDialogContainerVisible();
onView(withId(R.id.tab_modal_dialog_container)) onView(withId(R.id.tab_modal_dialog_container))
.check(matches( .check(matches(
allOf(hasDescendant(withText("1")), not(hasDescendant(withText("2")))))); allOf(hasDescendant(withText("1")), not(hasDescendant(withText("2"))))));
...@@ -260,6 +257,7 @@ public class ChromeTabModalPresenterTest { ...@@ -260,6 +257,7 @@ public class ChromeTabModalPresenterTest {
// Add a tab modal dialog available for showing. // Add a tab modal dialog available for showing.
showDialog(mManager, dialog1, ModalDialogType.TAB); showDialog(mManager, dialog1, ModalDialogType.TAB);
checkPendingSize(mManager, ModalDialogType.TAB, 0); checkPendingSize(mManager, ModalDialogType.TAB, 0);
ensureDialogContainerVisible();
onView(withId(R.id.tab_modal_dialog_container)) onView(withId(R.id.tab_modal_dialog_container))
.check(matches(hasDescendant(withText("1")))); .check(matches(hasDescendant(withText("1"))));
checkBrowserControls(true); checkBrowserControls(true);
...@@ -283,7 +281,6 @@ public class ChromeTabModalPresenterTest { ...@@ -283,7 +281,6 @@ public class ChromeTabModalPresenterTest {
@SmallTest @SmallTest
@Feature({"ModalDialog"}) @Feature({"ModalDialog"})
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE) @Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
@DisabledTest(message = "Flaky. crbug.com/1030683")
public void testSuspend_TabClosed() throws Exception { public void testSuspend_TabClosed() throws Exception {
PropertyModel dialog1 = PropertyModel dialog1 =
createDialog(mActivity, mActivity.getModalDialogManager(), "1", null); createDialog(mActivity, mActivity.getModalDialogManager(), "1", null);
...@@ -303,6 +300,7 @@ public class ChromeTabModalPresenterTest { ...@@ -303,6 +300,7 @@ public class ChromeTabModalPresenterTest {
showDialog(mManager, dialog1, ModalDialogType.TAB); showDialog(mManager, dialog1, ModalDialogType.TAB);
checkPendingSize(mManager, ModalDialogType.APP, 0); checkPendingSize(mManager, ModalDialogType.APP, 0);
checkPendingSize(mManager, ModalDialogType.TAB, 0); checkPendingSize(mManager, ModalDialogType.TAB, 0);
ensureDialogContainerVisible();
onView(withId(R.id.tab_modal_dialog_container)) onView(withId(R.id.tab_modal_dialog_container))
.check(matches(hasDescendant(withText("1")))); .check(matches(hasDescendant(withText("1"))));
checkBrowserControls(true); checkBrowserControls(true);
...@@ -340,7 +338,6 @@ public class ChromeTabModalPresenterTest { ...@@ -340,7 +338,6 @@ public class ChromeTabModalPresenterTest {
@Test @Test
@SmallTest @SmallTest
@Feature({"ModalDialog"}) @Feature({"ModalDialog"})
@DisabledTest(message = "Flaky. crbug.com/1030903")
public void testDismiss_SwitchTab() throws Exception { public void testDismiss_SwitchTab() throws Exception {
PropertyModel dialog1 = PropertyModel dialog1 =
createDialog(mActivity, mActivity.getModalDialogManager(), "1", null); createDialog(mActivity, mActivity.getModalDialogManager(), "1", null);
...@@ -359,6 +356,7 @@ public class ChromeTabModalPresenterTest { ...@@ -359,6 +356,7 @@ public class ChromeTabModalPresenterTest {
// Add a tab modal dialog available for showing. // Add a tab modal dialog available for showing.
showDialog(mManager, dialog1, ModalDialogType.TAB); showDialog(mManager, dialog1, ModalDialogType.TAB);
checkPendingSize(mManager, ModalDialogType.TAB, 0); checkPendingSize(mManager, ModalDialogType.TAB, 0);
ensureDialogContainerVisible();
onView(withId(R.id.tab_modal_dialog_container)) onView(withId(R.id.tab_modal_dialog_container))
.check(matches(hasDescendant(withText("1")))); .check(matches(hasDescendant(withText("1"))));
checkBrowserControls(true); checkBrowserControls(true);
...@@ -373,6 +371,7 @@ public class ChromeTabModalPresenterTest { ...@@ -373,6 +371,7 @@ public class ChromeTabModalPresenterTest {
// Open a tab modal dialog in the current tab. The dialog should be shown. // Open a tab modal dialog in the current tab. The dialog should be shown.
showDialog(mManager, dialog2, ModalDialogType.TAB); showDialog(mManager, dialog2, ModalDialogType.TAB);
checkPendingSize(mManager, ModalDialogType.TAB, 0); checkPendingSize(mManager, ModalDialogType.TAB, 0);
ensureDialogContainerVisible();
onView(withId(R.id.tab_modal_dialog_container)) onView(withId(R.id.tab_modal_dialog_container))
.check(matches(hasDescendant(withText("2")))); .check(matches(hasDescendant(withText("2"))));
checkBrowserControls(true); checkBrowserControls(true);
...@@ -567,4 +566,10 @@ public class ChromeTabModalPresenterTest { ...@@ -567,4 +566,10 @@ public class ChromeTabModalPresenterTest {
assertTrue("Menu is incorrectly disabled.", isMenuEnabled); assertTrue("Menu is incorrectly disabled.", isMenuEnabled);
} }
} }
private void ensureDialogContainerVisible() {
final View dialogContainer = mTabModalPresenter.getDialogContainerForTest();
CriteriaHelper.pollUiThread(
Criteria.equals(View.VISIBLE, () -> dialogContainer.getVisibility()));
}
} }
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