New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 791066 link

Starred by 0 users
Status: Available
Owner:
kmilka@chromium.org
Cc:
█ sfiera@chromium.org
█ ramyan@chromium.org
treib@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Make LocalNTPUITest use the real local NTP, rather than local_ntp_browsertest.html

Project Member Reported by treib@chromium.org, Dec 1 2017 
LocalNTPUITest.FakeboxRedirectsToOmnibox should use the real local NTP rather than local_ntp_browsertest.html, which is basically a third-party NTP that implements similar functionality. (Which is also worth testing, but separately.)

I had a first attempt at this in https://crrev.com/c/789884 which promptly got reverted (https://crrev.com/c/803534) for causing massive flakiness on Windows (Mac and Linux were fine).

Dashboard link for future reference:
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=interactive_ui_tests&tests=LocalNTPUITest

There seem to be two possible failure modes:

../../chrome/browser/ui/search/local_ntp_uitest.cc(98): error:       Expected: OMNIBOX_FOCUS_INVISIBLE
      Which is: 2
To be equal to: omnibox()->model()->focus_state()
      Which is: 0
../../chrome/browser/ui/search/local_ntp_uitest.cc(104): error: Value of: result
  Actual: false
Expected: true

and

../../chrome/browser/ui/search/local_ntp_uitest.cc(58): error:       Expected: OMNIBOX_FOCUS_NONE
      Which is: 0
To be equal to: omnibox()->model()->focus_state()
      Which is: 2
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 6 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d3607c81973cd30028c61d24d5b5716d113403b7

commit d3607c81973cd30028c61d24d5b5716d113403b7
Author: Marc Treib <treib@chromium.org>
Date: Wed Dec 06 12:35:45 2017

Local NTP tests: Move NavigateToNTPAndWaitUntilLoaded into utils

in preparation for using it in more tests (UI tests in particular).
Also contains some other minor cleanups.

Bug: 791066
Change-Id: I5bd2c7e3e99655e3ff453fc447c69f1484ce3d5b
Reviewed-on: https://chromium-review.googlesource.com/808764
Reviewed-by: Chris Pickel <sfiera@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522072}
[modify] https://crrev.com/d3607c81973cd30028c61d24d5b5716d113403b7/chrome/browser/ui/search/local_ntp_browsertest.cc
[modify] https://crrev.com/d3607c81973cd30028c61d24d5b5716d113403b7/chrome/browser/ui/search/local_ntp_test_utils.cc
[modify] https://crrev.com/d3607c81973cd30028c61d24d5b5716d113403b7/chrome/browser/ui/search/local_ntp_test_utils.h

Comment 2 by mvanouwe...@chromium.org, Dec 6 2017

Labels: zine-triaged

Comment 3 by treib@chromium.org, Jan 18 2018 

New attempt is at https://crrev.com/c/817565, but still getting the same errors.

Interestingly, the flakiness dashboard currently shows a few instances of the same failure on Linux and Windows, *without* that CL. So it looks like the CL isn't actually the cause of the problems, it just makes them worse/more visible somehow.

Comment 4 by treib@chromium.org, Mar 1 2018

Cc: treib@chromium.org
Owner: ----
Status: Available (was: Started)

Comment 5 by ramyan@chromium.org, Sep 24

Cc: ramyan@chromium.org
Owner: kmilka@chromium.org
Kyle - we should review this as part of the Local NTP work.

Comment 6 by kmilka@chromium.org, Sep 25 

I think we should hold off on this until we finally experiment with removing the fakebox. There are a number of open bugs that are obsolete once the fakebox-omnibox interaction no longer exists.
►

Sign in to add a comment