Issue metadata
Sign in to add a comment
|
ContextualSearchManagerTest failing on chromium.android/Marshmallow Tablet Tester |
||||||||||||||||||||||
Issue descriptionchrome_public_test_apk failing on chromium.android/Marshmallow Tablet Tester Type: build-failure Builders failed on: - Marshmallow Tablet Tester: https://build.chromium.org/p/chromium.android/builders/Marshmallow%20Tablet%20Tester Initial failing build: https://uberchromegw.corp.google.com/i/chromium.android/builders/Marshmallow%20Tablet%20Tester/builds/7135 Suspect change: https://codereview.chromium.org/2201853002
,
Jan 19 2017
I can't get the Contextual Search panel to show up on a ToT build of ChromePublic.apk. Do I need to build clank to see it?
,
Jan 19 2017
Looks like there's something strange going on with Pedro's phone based on our f2f tests. Looks like these two tests fail on all tablets, so I can lend you a tablet to work on. These two tests are pretty important, so I'd rather not just disable them.
,
Jan 19 2017
,
Jan 19 2017
,
Jan 19 2017
Note that this is happening on all three of the tablet bots, not just M tablet.
,
Jan 19 2017
Pedro, I noticed both tests fail to close the panel when they call tapBasePageToClosePanel() near the end of the test. It's likely that just changing those calls to the newer closePanel() call will fix the problem. Give that a try, and let me know if there's any way I can help!
,
Jan 19 2017
I think we need some tests that check base page taps work as expected on tablets, so I'm okay with that plan as long as we have coverage somewhere else.
,
Jan 19 2017
Accidentally added the RBS label. I tested chained searches and base page taps on my Nexus 9 and they appear to be working correctly so I think it's just broken tests and nothing that needs to block the release.
,
Jan 20 2017
I tried the changes donnd@ suggested and they fix the tests. There are several other tests that use |tapBasePageToClosePanel()|. Do you think that is enough coverage?
,
Jan 20 2017
I checked our other tests and there are still quite a few that will still use tapBasePageToClosePanel after your change, so I think that's enough coverage.
,
Jan 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ac2bc433990822272814e5eaa3f98ebf8c5338c4 commit ac2bc433990822272814e5eaa3f98ebf8c5338c4 Author: amaralp <amaralp@chromium.org> Date: Fri Jan 20 03:19:49 2017 ContextualSearchManagerTest fails because of tapBasePageToClosePanel() A regression was introduced as crrbug.com/682119 describes. The fix for it was to change the failing |tapBasePageToClosePanel()| to |closePanel()| BUG= 682119 Review-Url: https://codereview.chromium.org/2637403009 Cr-Commit-Position: refs/heads/master@{#444966} [modify] https://crrev.com/ac2bc433990822272814e5eaa3f98ebf8c5338c4/chrome/android/javatests/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchManagerTest.java
,
Jan 20 2017
Re #11 - Most of our tests that call #tapBasePageToClosePanel() are restricted to phone only, so I think the only place we have coverage for this on tablets now is #testTapCloseRemovedFromHistory() and #testTapCount() (which is marked as flaky).
,
Jan 20 2017
You're quite right -- I was sloppy checking on the phone restriction and just saw that there were _some_ tests that do run on tablet. But it would be easy to upgrade them too and lose this check. I'll file a bug to add another test to explicitly check that we can close the panel on tablet with various gestures.
,
Jan 31 2017
Pedro, I think you can close this now.
,
Jan 31 2017
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by tedc...@chromium.org
, Jan 18 2017