Skip to content

Commit

Permalink
Revert "Reland "[Fixit] TabGridAccessibilityHelperTest""
Browse files Browse the repository at this point in the history 
This reverts commit e042779.

Reason for revert: https://crbug.com/1454747#c5

Original change's description:
> Reland "[Fixit] TabGridAccessibilityHelperTest"
>
> This reverts commit cb6674c.
>
> Reason for revert: Another attempt to fix and wait for layout.
>
> The issue only seems to be arm64 "real" devices so if this also
> fails I might just DisableIf to abi is not arm.
>
> Original change's description:
> > Revert "[Fixit] TabGridAccessibilityHelperTest"
> >
> > This reverts commit 4c91398.
> >
> > Reason for revert: failing tests on a couple bots, see crbug.com/1454747
> >
> > Original change's description:
> > > [Fixit] TabGridAccessibilityHelperTest
> > >
> > > Fix failing tests which seem to stem from the following:
> > > 1) Start surface vs Start Surface Refactor have different onView
> > >    parents. (Set one consistent flag set).
> > > 2) If scrolled out of view getting the RecyclerView ViewHolder is
> > >    flaky.
> > > 3) Small phones don't get span count = 3. Skip those sections.
> > >
> > > Additionally:
> > > * Batch tests
> > > * Throw ViewMatchExceptions to aid in debugging in future
> > >
> > > Fixed: 1368279, 1332995, 1318394, 1371231, 1332934, 1318376, 1306823, 1146575
> > > Change-Id: Ica6d65fd4a53a7270aa5e066f0307ccb7772fe4d
> > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4604877
> > > Auto-Submit: Calder Kitagawa <ckitagawa@chromium.org>
> > > Reviewed-by: Fred Mello <fredmello@chromium.org>
> > > Commit-Queue: Fred Mello <fredmello@chromium.org>
> > > Cr-Commit-Position: refs/heads/main@{#1156890}
> >
> > Change-Id: I7535c81be60389627d64dde372380a18c24d2da0
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4615045
> > Commit-Queue: Theresa Sullivan <twellington@chromium.org>
> > Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> > Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1157608}
>
> Cq-Include-Trybots: luci.chromium.try:android-arm64-rel,android-pie-arm64-dbg
> Change-Id: I86ae894c2e227f6dbc3886a81b8258996b4fd1be
> Bug: 1454747
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4617508
> Reviewed-by: Fred Mello <fredmello@chromium.org>
> Commit-Queue: Fred Mello <fredmello@chromium.org>
> Auto-Submit: Calder Kitagawa <ckitagawa@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1158179}

Bug: 1454747
Change-Id: If6d68754da811d19c2d23690f6a7593c0db98cd2
Cq-Include-Trybots: luci.chromium.try:android-arm64-rel,android-pie-arm64-dbg
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4626513
Commit-Queue: Samar Chehade-Lepleux <samarchehade@google.com>
Owners-Override: Samar Chehade-Lepleux <samarchehade@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1159517}
  • Loading branch information
Samar Chehade-Lepleux authored and Chromium LUCI CQ committed Jun 19, 2023
1 parent b531b31 commit 144f241
Showing 1 changed file with 22 additions and 96 deletions.
118 changes: 22 additions & 96 deletions .../src/org/chromium/chrome/browser/tasks/tab_management/TabGridAccessibilityHelperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,46 +13,38 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;

import static org.chromium.base.test.util.Restriction.RESTRICTION_TYPE_NON_LOW_END_DEVICE;
import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.createTabs;
import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.enterTabSwitcher;
import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.leaveTabSwitcher;
import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.verifyTabSwitcherCardCount;

import android.content.Context;
import android.content.res.Configuration;
import android.util.Pair;
import android.view.View;
import android.view.View.OnLayoutChangeListener;
import android.view.accessibility.AccessibilityNodeInfo.AccessibilityAction;

import androidx.annotation.IntDef;
import androidx.recyclerview.widget.GridLayoutManager;
import androidx.recyclerview.widget.LinearLayoutManager;
import androidx.recyclerview.widget.RecyclerView;
import androidx.test.filters.MediumTest;

import org.junit.After;
import org.junit.Before;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;

import org.chromium.base.test.util.Batch;
import org.chromium.base.test.util.CallbackHelper;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.CriteriaHelper;
import org.chromium.base.test.util.DisabledTest;
import org.chromium.base.test.util.Restriction;
import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.compositor.layouts.Layout;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.layouts.LayoutType;
import org.chromium.chrome.features.start_surface.TabSwitcherAndStartSurfaceLayout;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
import org.chromium.chrome.test.R;
import org.chromium.chrome.test.batch.BlankCTATabInitialStateRule;
import org.chromium.chrome.test.util.ActivityTestUtils;
import org.chromium.chrome.test.util.browser.Features;
import org.chromium.ui.test.util.UiRestriction;
Expand All @@ -68,11 +60,7 @@
// clang-format off
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
// START_SURFACE_REFACTOR is required to have stable parent id logic.
@Features.EnableFeatures({ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID,
ChromeFeatureList.START_SURFACE_REFACTOR,
ChromeFeatureList.TAB_GROUPS_CONTINUATION_ANDROID})
@Batch(Batch.PER_CLASS)
@Features.EnableFeatures({ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID})
public class TabGridAccessibilityHelperTest {
// clang-format on
@IntDef({TabMovementDirection.LEFT, TabMovementDirection.RIGHT, TabMovementDirection.UP,
Expand All @@ -86,58 +74,43 @@ public class TabGridAccessibilityHelperTest {
int NUM_ENTRIES = 4;
}

@ClassRule
public static ChromeTabbedActivityTestRule sActivityTestRule =
new ChromeTabbedActivityTestRule();

@Rule
public BlankCTATabInitialStateRule mBlankCTATabInitialStateRule =
new BlankCTATabInitialStateRule(sActivityTestRule, false);
public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();

@Before
public void setUp() {
Layout layout =
sActivityTestRule.getActivity().getLayoutManager().getTabSwitcherLayoutForTesting();
assertTrue(layout instanceof TabSwitcherLayout);
mActivityTestRule.startMainActivityFromLauncher();
Layout layout = mActivityTestRule.getActivity().getLayoutManager().getOverviewLayout();
assertTrue(layout instanceof TabSwitcherAndStartSurfaceLayout);
CriteriaHelper.pollUiThread(
sActivityTestRule.getActivity().getTabModelSelector()::isTabStateInitialized);
mActivityTestRule.getActivity().getTabModelSelector()::isTabStateInitialized);
}

@After
public void tearDown() {
ActivityTestUtils.clearActivityOrientation(sActivityTestRule.getActivity());
final ChromeTabbedActivity cta = sActivityTestRule.getActivity();
if (cta != null && cta.getLayoutManager().isLayoutVisible(LayoutType.TAB_SWITCHER)) {
leaveTabSwitcher(cta);
}
ActivityTestUtils.clearActivityOrientation(mActivityTestRule.getActivity());
}

@Test
@MediumTest
// Low-end uses list mode.
@Restriction(RESTRICTION_TYPE_NON_LOW_END_DEVICE)
public void testGetPotentialActionsForView() throws Exception {
final ChromeTabbedActivity cta = sActivityTestRule.getActivity();
@DisabledTest(message = "https://crbug.com/1318376")
public void testGetPotentialActionsForView() {
// clang-format on
final ChromeTabbedActivity cta = mActivityTestRule.getActivity();
final AccessibilityActionChecker checker = new AccessibilityActionChecker(cta);
createTabs(cta, false, 5);
enterTabSwitcher(cta);
verifyTabSwitcherCardCount(cta, 5);

View view = cta.findViewById(R.id.tab_list_view);
assertTrue(view instanceof TabListMediator.TabGridAccessibilityHelper);
TabListMediator.TabGridAccessibilityHelper helper =
(TabListMediator.TabGridAccessibilityHelper) view;
assertTrue(cta.findViewById(R.id.tab_list_view)
instanceof TabListMediator.TabGridAccessibilityHelper);
TabListMediator.TabGridAccessibilityHelper helper = cta.findViewById(R.id.tab_list_view);

// Verify action list in portrait mode with span count = 2.
onView(allOf(withParent(withId(R.id.compositor_view_holder)), withId(R.id.tab_list_view)))
.check((v, noMatchingViewException) -> {
if (noMatchingViewException != null) {
throw noMatchingViewException;
}
assertTrue(v instanceof RecyclerView);
RecyclerView recyclerView = (RecyclerView) v;
assertEquals(2,
((GridLayoutManager) recyclerView.getLayoutManager()).getSpanCount());

View item1 = getItemViewForPosition(recyclerView, 0);
checker.verifyListOfAccessibilityAction(
Expand Down Expand Up @@ -169,30 +142,13 @@ public void testGetPotentialActionsForView() throws Exception {
new ArrayList<>(Arrays.asList(TabMovementDirection.UP)));
});

assertTrue(view instanceof TabListRecyclerView);
TabListRecyclerView tabListRecyclerView = (TabListRecyclerView) view;
CallbackHelper callbackHelper = new CallbackHelper();
OnLayoutChangeListener listener =
(rv, left, top, right, bottom, oldLeft, oldTop, oldRight, oldBottom) -> {
callbackHelper.notifyCalled();
};
tabListRecyclerView.addOnLayoutChangeListener(listener);
final int callCount = callbackHelper.getCallCount();
ActivityTestUtils.rotateActivityToOrientation(cta, Configuration.ORIENTATION_LANDSCAPE);
callbackHelper.waitForCallback(callCount);

// Verify action list in landscape mode with span count = 3.
onView(allOf(withParent(withId(R.id.compositor_view_holder)), withId(R.id.tab_list_view)))
.check((v, noMatchingViewException) -> {
if (noMatchingViewException != null) {
throw noMatchingViewException;
}
assertTrue(v instanceof RecyclerView);
RecyclerView recyclerView = (RecyclerView) v;
// This case only applies for a span of 3.
if (((GridLayoutManager) recyclerView.getLayoutManager()).getSpanCount() != 3) {
return;
}

View item1 = getItemViewForPosition(recyclerView, 0);
checker.verifyListOfAccessibilityAction(
Expand Down Expand Up @@ -227,10 +183,9 @@ public void testGetPotentialActionsForView() throws Exception {

@Test
@MediumTest
// Low-end uses list mode.
@Restriction(RESTRICTION_TYPE_NON_LOW_END_DEVICE)
public void testGetPositionsOfReorderAction() throws Exception {
final ChromeTabbedActivity cta = sActivityTestRule.getActivity();
@DisabledTest(message = "https://crbug.com/1318394")
public void testGetPositionsOfReorderAction() {
final ChromeTabbedActivity cta = mActivityTestRule.getActivity();
int leftActionId = R.id.move_tab_left;
int rightActionId = R.id.move_tab_right;
int upActionId = R.id.move_tab_up;
Expand All @@ -239,22 +194,14 @@ public void testGetPositionsOfReorderAction() throws Exception {
enterTabSwitcher(cta);
verifyTabSwitcherCardCount(cta, 5);

View view = cta.findViewById(R.id.tab_list_view);
assertTrue(view instanceof TabListMediator.TabGridAccessibilityHelper);
TabListMediator.TabGridAccessibilityHelper helper =
(TabListMediator.TabGridAccessibilityHelper) view;
assertTrue(cta.findViewById(R.id.tab_list_view)
instanceof TabListMediator.TabGridAccessibilityHelper);
TabListMediator.TabGridAccessibilityHelper helper = cta.findViewById(R.id.tab_list_view);

// Span count 2.
onView(allOf(withParent(withId(R.id.compositor_view_holder)), withId(R.id.tab_list_view)))
.check((v, noMatchingViewException) -> {
if (noMatchingViewException != null) {
throw noMatchingViewException;
}
assertTrue(v instanceof RecyclerView);
RecyclerView recyclerView = (RecyclerView) v;
assertEquals(2,
((GridLayoutManager) recyclerView.getLayoutManager()).getSpanCount());

Pair<Integer, Integer> positions;

View item1 = getItemViewForPosition(recyclerView, 0);
Expand All @@ -276,31 +223,12 @@ public void testGetPositionsOfReorderAction() throws Exception {
assertEquals(1, (int) positions.second);
});

assertTrue(view instanceof TabListRecyclerView);
TabListRecyclerView tabListRecyclerView = (TabListRecyclerView) view;
CallbackHelper callbackHelper = new CallbackHelper();
OnLayoutChangeListener listener =
(rv, left, top, right, bottom, oldLeft, oldTop, oldRight, oldBottom) -> {
callbackHelper.notifyCalled();
};
tabListRecyclerView.addOnLayoutChangeListener(listener);
final int callCount = callbackHelper.getCallCount();
ActivityTestUtils.rotateActivityToOrientation(cta, Configuration.ORIENTATION_LANDSCAPE);
callbackHelper.waitForCallback(callCount);

// Span count 3.
onView(allOf(withParent(withId(R.id.compositor_view_holder)), withId(R.id.tab_list_view)))
.check((v, noMatchingViewException) -> {
if (noMatchingViewException != null) {
throw noMatchingViewException;
}
assertTrue(v instanceof RecyclerView);
RecyclerView recyclerView = (RecyclerView) v;
// This case only applies for a span of 3.
if (((GridLayoutManager) recyclerView.getLayoutManager()).getSpanCount() != 3) {
return;
}

Pair<Integer, Integer> positions;

View item2 = getItemViewForPosition(recyclerView, 1);
Expand Down Expand Up @@ -328,8 +256,6 @@ public void testGetPositionsOfReorderAction() throws Exception {
}

private View getItemViewForPosition(RecyclerView recyclerView, int position) {
// Scroll to position to ensure the ViewHolder is not recycled.
((LinearLayoutManager) recyclerView.getLayoutManager()).scrollToPosition(position);
RecyclerView.ViewHolder viewHolder =
recyclerView.findViewHolderForAdapterPosition(position);
assertNotNull(viewHolder);
Expand Down

0 comments on commit 144f241

Please sign in to comment.