Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(251)

Issue 1413383007: Revert of [Contextual Search] Improve testing & add regressions tests (Closed)

Created:
5 years, 1 month ago by jam
Modified:
5 years, 1 month ago
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of [Contextual Search] Improve testing & add regressions tests (patchset #4 id:50001 of https://codereview.chromium.org/1426683005/ ) Reason for revert: Adds a very flaky test. BUG=549805 Original issue's description: > [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 > > Committed: https://crrev.com/7e0c3b88ed5970327b2aa7962a1bc336b71bc427 > Cr-Commit-Position: refs/heads/master@{#357143} TBR=aurimas@chromium.org,donnd@chromium.org,dtrainor@chromium.org,dfalcantara@chromium.org,pedrosimonetti@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=543319 Committed: https://crrev.com/d6d3bcb62ad798ee56ffef7429f990d4a1e23ef4 Cr-Commit-Position: refs/heads/master@{#357278}

Patch Set 1 #

Messages

Total messages: 4 (0 generated)
jam
Created Revert of [Contextual Search] Improve testing & add regressions tests
5 years, 1 month ago (2015-10-31 18:22:27 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1413383007/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1413383007/1
5 years, 1 month ago (2015-10-31 18:22:32 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 1 month ago (2015-10-31 18:23:16 UTC) #3
commit-bot: I haz the power
5 years, 1 month ago (2015-10-31 18:24:23 UTC) #4
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/d6d3bcb62ad798ee56ffef7429f990d4a1e23ef4
Cr-Commit-Position: refs/heads/master@{#357278}

Powered by Google App Engine
This is Rietveld 408576698