Commit 7e0c3b88 authored by pedrosimonetti's avatar pedrosimonetti Committed by Commit bot

[Contextual Search] Improve testing & add regressions tests

This CL adds tests for regressions that have been fixed
recently, some of them involving ContentViewCore.

In order to be able to properly instrument the ContentViewCore,
changes in the testing framework have been made. There are now
a couple of wrapper classes: OverlayPanelContentWrapper and
ContentViewCoreWrapper, which allow us to test for example
if the onShow() method was ever called, or if URLs have been
properly removed from local history when not seen.

This CL also introduces a few more helpers to simulate searches
caused by gestures (long press and tap) in a way that it
encapsulates the simulation logic and prevent flakiness. We
were manually doing a search term resolution by calling a
fakeResponse() method. Sometimes we are calling this when
not necessary (on long press) and tests don't call it in the
same order. We are also assuming it completed when the Bar
peeks (which works but under the wrong assumption). There are
now simulateLongPressSearch(id) and simulateTapSearch(id)
methods that simulate the search and handle the resolution
automatically when the system requests one.

There are several helper methods in ContextualSearchManagerTest
that are similar but do slightly different things, and some
assumptions are not quite correct which can and may be causing
falkiness in some tests. A couple of examples are: tests rely
on the change of state of the Panel in order to determine that
animations/actions are done. The closed state was being changed
before destroying the Contents. This is now fixed.

Another example is the tapBasePageToClosePanel() helper method
that wasn't properly working when the Panel was peeking while
a long press selection is present. I believe this is the cause
of the flakiness of 3 of our tests.

The new search simulation method should help making our tests
more robust and a followup CL is required to change existing
tests to use the new methods, once it has been confirmed that
the new methods work properly on existing test bots.

BUG=543319

Review URL: https://codereview.chromium.org/1426683005

Cr-Commit-Position: refs/heads/master@{#357143}
parent 8142249a
......@@ -155,6 +155,15 @@ public class OverlayPanelContent {
// ContentViewCore related
// ============================================================================================
/**
* Creates a ContentViewCore. This method will be overridden by tests.
* @param activity The ChromeActivity.
* @return The newly created ContentViewCore.
*/
protected ContentViewCore createContentViewCore(ChromeActivity activity) {
return new ContentViewCore(activity);
}
/**
* Create a new ContentViewCore that will be managed by this panel.
*/
......@@ -167,7 +176,7 @@ public class OverlayPanelContent {
destroyContentView();
}
mContentViewCore = new ContentViewCore(mActivity);
mContentViewCore = createContentViewCore(mActivity);
if (mContentViewClient == null) {
mContentViewClient = new ContentViewClient();
......@@ -247,14 +256,6 @@ public class OverlayPanelContent {
}
}
/**
* @return Whether the ContentViewCore was created.
*/
@VisibleForTesting
public boolean didCreateContentView() {
return mContentViewCore != null;
}
/**
* Load a URL, this will trigger creation of a new ContentViewCore.
* @param url The URL that should be loaded.
......
......@@ -810,8 +810,6 @@ abstract class ContextualSearchPanelBase implements ContextualSearchPromoHost {
* @param reason The reason for a change in the panel's state.
*/
public void setPanelState(PanelState state, StateChangeReason reason) {
mPanelState = state;
if (state == PanelState.CLOSED) {
mIsShowing = false;
onClosed(reason);
......@@ -819,6 +817,12 @@ abstract class ContextualSearchPanelBase implements ContextualSearchPromoHost {
|| (state == PanelState.MAXIMIZED && !isFullscreenSizePanel())) {
showPromoViewAtYPosition(getPromoYPx());
}
// We should only set the state at the end of this method, in oder to make sure that
// all callbacks will be fired before changing the state of the Panel. This prevents
// some flakiness on tests since they rely on changes of state to determine when a
// particular action has been completed.
mPanelState = state;
}
/**
......
......@@ -22,6 +22,9 @@
<div>United States <span id="intelligence">Intelligence</span></div>
<div>United <span id="states">States</span> Intelligence</div>
<div>United <span id="states-near">StatesNear</span> Intelligence</div>
<!-- These three spans should be close to each other so that taps from one to
to the next are within our "near" threshold. -->
<div><span id="search">Search</span> <span id="term">Term</span> <span id="resolution">Resolution</span></div>
<form action="demo_form.asp">
<label for="male">Male</label>
<input type="radio" name="sex" id="male" value="male"><br>
......
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