My favorites | Sign in
Logo
             
New issue | Search
for
| Advanced search | Search tips
Issue 16447: FindInPageControllerTest is so flaky
2 people starred this issue and may be notified of changes. Back to list
Status:  Fixed
Owner:  finnur@chromium.org
Closed:  Feb 01
Type-Bug
Pri-1
OS-All
Tests
Mstone-5
Area-BuildTools


Sign in to add a comment
 
Reported by phajdan...@chromium.org, Jul 10, 2009
Note: Google Test filter = FindInPageControllerTest.FindDisappearOnNavigate
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from FindInPageControllerTest
[ RUN      ] FindInPageControllerTest.FindDisappearOnNavigate
C:\b\slave\chromium-dbg-builder\build\src\chrome\browser\views\find_bar_win_browsertest.cc(449):
error: Value of: fully_visible
  Actual: true
Expected: false
[  FAILED  ] FindInPageControllerTest.FindDisappearOnNavigate (3312 ms)
[----------] 1 test from FindInPageControllerTest (3312 ms total)

  // Navigate and make sure the Find window goes away.
  ui_test_utils::NavigateToURL(browser(), url2);

  GetFindBarWindowInfo(&position, &fully_visible);
  EXPECT_FALSE(fully_visible);  // Fails here.

I see two possibilities:

1. Definition of "navigation finished" should also include maintaining
proper state of the find bar.
2. We should set up notifications for find bar state changes and explicitly
wait here.
Comment 1 by phajdan...@chromium.org, Jul 13, 2009
Actually the whole test is flaky. Another example:

[----------] 1 test from FindInPageControllerTest
[ RUN      ] FindInPageControllerTest.FindMovesWhenObscuring
C:\b\slave\chromium-dbg-
builder\build\src\chrome\browser\views\find_bar_win_browsertest.cc(522): error: 
Expected: (start_position.x()) != (position.x()), actual: 643 vs 643
[  FAILED  ] FindInPageControllerTest.FindMovesWhenObscuring (3779 ms)
[----------] 1 test from FindInPageControllerTest (3779 ms total)

  // Search for 'dream' which the Find box is obscuring.
  int ordinal = 0;
  EXPECT_EQ(1, FindInPage(L"dream", FWD, IGNORE_CASE, &ordinal));
  EXPECT_EQ(1, ordinal);

  // Make sure Find box has moved.
  GetFindBarWindowInfo(&position, &fully_visible);
  EXPECT_EQ(start_position.y(), position.y());
  EXPECT_NE(start_position.x(), position.x());  // Fails here.
  EXPECT_TRUE(fully_visible);
Summary: FindInPageControllerTest is so flaky
Comment 2 by phajdan...@chromium.org, Jul 16, 2009
(No comment was entered for this change.)
Labels: FlakyTest
Comment 3 by bugdroid1@chromium.org, Aug 03, 2009
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=22322 

------------------------------------------------------------------------
r22322 | phajdan.jr@chromium.org | 2009-08-03 14:57:30 -0700 (Mon, 03 Aug 2009) | 6 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/find_bar_win_browsertest.cc?r1=22322&r2=22321

Disable flaky parts of FindInPageControllerTest.

TEST=none
http://crbug.com/16447

Review URL: http://codereview.chromium.org/159805
------------------------------------------------------------------------

Comment 4 by nsylvain@chromium.org, Oct 24, 2009
(No comment was entered for this change.)
Labels: -Pri-2 Pri-1
Comment 5 by phajdan...@chromium.org, Oct 25, 2009
Finnur, could you take a look at this? Recently the priority has been bumped to P1, and I 
don't have free cycles for that.
Status: Untriaged
Owner: fin...@chromium.org
Cc: -fin...@chromium.org
Labels: -Size-Medium
Comment 6 by mal.chromium, Nov 04, 2009
(No comment was entered for this change.)
Status: Assigned
Labels: Mstone-4
Comment 7 by bugdroid1@chromium.org, Nov 06, 2009
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=31325 

------------------------------------------------------------------------
r31325 | finnur@chromium.org | 2009-11-06 15:41:11 -0800 (Fri, 06 Nov 2009) | 9 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/find_bar_host_browsertest.cc?r1=31325&r2=31324

Adding traces to a test when it fails in the hope of at
least ruling some things out. These traces will be 
either be removed (once I get more data).

TBR=jcampan
BUG=16447
TEST=This is changing a test

Review URL: http://codereview.chromium.org/373019
------------------------------------------------------------------------

Comment 8 by finnur@chromium.org, Nov 06, 2009
Hey Phajdan,

The flakiness dashboard lists the test that I just modified, but not the other test 
you marked flaky (FindInPageControllerTest.FLAKY_FindDisappearOnNavigate).

You limit the number shown on the dashboard to what... 25 or so? Does it have 
additional data that you are not showing or is that discarded?

In other words, I'd like to look at the logs for the failures, but don't want to sort 
through the build logs for a failure that last happened a week ago or something. Is 
that easily available somewhere?
Comment 9 by phajdan...@chromium.org, Nov 07, 2009
Yes, the flakiness dashboard is limited to 25 entries. I can tweak it a little to display 
more flaky tests (because people need this info to see if the flakiness has been 
fixed), and the same number of flaky groups (just to limit the noise).

For now, I grepped the dashboard logs manually, and the only failure looks like this:

C:\b\slave\chromium-rel-
builder\build\src\chrome\browser\views\find_bar_host_browsertest.cc(528): error: 
Expected: (start_position.x()) != (position.x()), actual: 686 vs 686

Failed this way 9 time(s). First failure was on 2009-10-26, and last on 2009-11-04.

It's FindInPageControllerTest.FindMovesWhenObscuring I think (the line numbers 
seem to have shifted a little).

So, I think you can safely mark FindDisappearOnNavigate as not flaky (as well as 
other tests in this group). It had no flaky flips in the last 3 weeks. It's still possible 
that the test was just lucky, but then the flakiness in it is tiny.
Comment 10 by bugdroid1@chromium.org, Nov 07, 2009
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=31399 

------------------------------------------------------------------------
r31399 | finnur@chromium.org | 2009-11-07 19:08:17 -0800 (Sat, 07 Nov 2009) | 10 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/find_bar_host_browsertest.cc?r1=31399&r2=31398

Remove flaky label for FindInPageControllerTest.FindDisappearOnNavigate

When I started looking into the test flakiness, it
turns out it hasn't failed in 3 weeks, according to
Phajdan's logs, so I am removing the Flaky label.

BUG=16447
TEST=This is a test change.

Review URL: http://codereview.chromium.org/371054
------------------------------------------------------------------------

Comment 11 by bugdroid1@chromium.org, Nov 09, 2009
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=31446 

------------------------------------------------------------------------
r31446 | finnur@chromium.org | 2009-11-09 10:15:52 -0800 (Mon, 09 Nov 2009) | 7 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/find_bar_host_browsertest.cc?r1=31446&r2=31445

Try one other thing temporarily to try to determine cause for a flaky test.

TBR=jcampan
BUG=16447
TEST=This is changing a test

Review URL: http://codereview.chromium.org/376024
------------------------------------------------------------------------

Comment 12 by bugdroid1@chromium.org, Nov 18, 2009
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=31325 

------------------------------------------------------------------------
r31325 | finnur@chromium.org | 2009-11-06 15:41:11 -0800 (Fri, 06 Nov 2009) | 9 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/find_bar_host_browsertest.cc?r1=31325&r2=31324

Adding traces to a test when it fails in the hope of at
least ruling some things out. These traces will be 
either be removed (once I get more data).

TBR=jcampan
BUG=16447
TEST=This is changing a test

Review URL: http://codereview.chromium.org/373019
------------------------------------------------------------------------

Comment 13 by bugdroid1@chromium.org, Nov 18, 2009
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=31399 

------------------------------------------------------------------------
r31399 | finnur@chromium.org | 2009-11-07 19:08:17 -0800 (Sat, 07 Nov 2009) | 10 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/find_bar_host_browsertest.cc?r1=31399&r2=31398

Remove flaky label for FindInPageControllerTest.FindDisappearOnNavigate

When I started looking into the test flakiness, it
turns out it hasn't failed in 3 weeks, according to
Phajdan's logs, so I am removing the Flaky label.

BUG=16447
TEST=This is a test change.

Review URL: http://codereview.chromium.org/371054
------------------------------------------------------------------------

Comment 14 by bugdroid1@chromium.org, Nov 19, 2009
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=31446 

------------------------------------------------------------------------
r31446 | finnur@chromium.org | 2009-11-09 10:15:52 -0800 (Mon, 09 Nov 2009) | 7 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/find_bar_host_browsertest.cc?r1=31446&r2=31445

Try one other thing temporarily to try to determine cause for a flaky test.

TBR=jcampan
BUG=16447
TEST=This is changing a test

Review URL: http://codereview.chromium.org/376024
------------------------------------------------------------------------

Comment 15 by m...@google.com, Nov 19, 2009
I see a lot of changes, but it's not clear what the status is now. It looks like the 
Flaky label has been removed... but then Jay has a change after that.

Is this still a flaky test?
Labels: -Mstone-4 Mstone-5
Comment 16 by finnur@chromium.org, Nov 19, 2009
This bug was tracking two flaky tests. One has been marked as not flaky. The other I 
added debugging info to. That issue is still open and the traces have ruled out some 
things, but not pinpointed the problem. I was in process of adding more traces (it 
takes a couple of days to get the results each time I make a change because it doesn't 
fail that often) and then I got pulled into something else...
Comment 17 by mal.chromium, Dec 18, 2009
(No comment was entered for this change.)
Labels: -Area-Infrastructure Area-BuildTools
Comment 18 by phajdan...@chromium.org, Jan 20, 2010
Finnur, here's the most recent sample:

C:\b\slave\chromium-dbg-builder\build\src\chrome\browser\views\find_bar_host_browsertest.cc(602): error: Value of: 
debug_msg.c_str()
  Actual: "Position check failed once. Position check failed again. "
Expected: ""
C:\b\slave\chromium-dbg-builder\build\src\chrome\browser\views\find_bar_host_browsertest.cc(604): error: Expected: 
(start_position.x()) != (position.x()), actual: 643 vs 643

Comment 19 by pkasting@chromium.org, Jan 21, 2010
FindDisappearOnNavigate is failing 100% consistently on linux_views, it is not flaky.

This test needs to either be disabled or fixed.  False flakiness makes life hard on the 
sheriffs.

Finnur, please take some kind of action.
Labels: -FlakyTest Tests
Comment 20 by bugdroid1@chromium.org, Jan 22, 2010
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=36781 

------------------------------------------------------------------------
r36781 | finnur@chromium.org | 2010-01-21 12:53:53 -0800 (Thu, 21 Jan 2010) | 6 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/find_bar_host_browsertest.cc?r1=36781&r2=36780
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/test/data/find_in_page/move_if_obscuring.html?r1=36781&r2=36780

Try to fix flaky test (FindMovesIfObscuring). I've run this through the try bots three times and they seemed to like the patch.

BUG=16447
TEST=The change is to a test. 

Review URL: http://codereview.chromium.org/553042
------------------------------------------------------------------------

Comment 21 by bugdroid1@chromium.org, Jan 22, 2010
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=36795 

------------------------------------------------------------------------
r36795 | finnur@chromium.org | 2010-01-21 14:01:37 -0800 (Thu, 21 Jan 2010) | 9 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/find_bar_host_browsertest.cc?r1=36795&r2=36794

Disable a test on Linux Views. It was marked as flaky, but it is failing all the time.

See http://crbug.com/28629 for details.

TBR=pkasting
BUG=http://crbug.com/28629, http://crbug.com/16447
TEST=None

Review URL: http://codereview.chromium.org/552086
------------------------------------------------------------------------

Comment 22 by bugdroid1@chromium.org, Jan 22, 2010
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=36797 

------------------------------------------------------------------------
r36797 | finnur@chromium.org | 2010-01-21 14:06:21 -0800 (Thu, 21 Jan 2010) | 7 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/find_bar_host_browsertest.cc?r1=36797&r2=36796

Take 2: Disable the test (I accidentally modified the file before checking in in my last checkin and added things I was working on but didn't want checked in yet). :/

TBR=oshima
BUG=http://crbug.com/28629, http://crbug.com/16447
TEST=None

Review URL: http://codereview.chromium.org/554041
------------------------------------------------------------------------

Comment 23 by bugdroid1@chromium.org, Jan 25, 2010
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=37018 

------------------------------------------------------------------------
r37018 | finnur@chromium.org | 2010-01-25 09:38:16 -0800 (Mon, 25 Jan 2010) | 8 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/find_bar_host_browsertest.cc?r1=37018&r2=37017

Remove flaky label for a test. My fix seems to have worked (hasn't
been failing for a few days now -- used to fail regularly).

BUG=16447
TEST=None (making a change to a test)
TBR=jcampan

Review URL: http://codereview.chromium.org/553070
------------------------------------------------------------------------

Comment 24 by finnur@chromium.org, Jan 25, 2010
These two tests haven't failed for a few days now after I committed my two changes 
(used to fail regularly), so I am calling this bug fixed.

FindMovesIfObscuring
FindDisappearOnNavigate

The latter test is disabled on Linux with Views toolkit, but there is another bug 
tracking that.
Status: Fixed
Comment 25 by ananta@chromium.org, Jan 26, 2010
Reopening as this is still failing

Status: Assigned
Comment 26 by bugdroid1@chromium.org, Jan 26, 2010
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=37226 

------------------------------------------------------------------------
r37226 | ananta@chromium.org | 2010-01-26 19:26:02 -0800 (Tue, 26 Jan 2010) | 6 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/find_bar_host_browsertest.cc?r1=37226&r2=37225

The FindMovesWhenObscuring test is still flaky on the builders. Marking it accordingly.

Bug=16447
TBR=finnur

Review URL: http://codereview.chromium.org/543223
------------------------------------------------------------------------

Comment 27 by bugdroid1@chromium.org, Jan 26, 2010
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=37239 

------------------------------------------------------------------------
r37239 | finnur@chromium.org | 2010-01-26 22:29:25 -0800 (Tue, 26 Jan 2010) | 6 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/find_bar_host_browsertest.cc?r1=37239&r2=37238
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/test/data/find_in_page/move_if_obscuring.html?r1=37239&r2=37238

Fix remaining flakiness in FindMoveIfObscuring browser test. 

BUG=http://crbug.com/16447
TEST=Covered by automated tests.

Review URL: http://codereview.chromium.org/549168
------------------------------------------------------------------------

Comment 28 by finnur@chromium.org, Feb 01, 2010
Hasn't failed since January 26th, according to the flakiness dashboard. Marked as non-
flaky and closing this bug.
Status: Fixed
Comment 29 by bugdroid1@chromium.org, Feb 02, 2010
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=37770 

------------------------------------------------------------------------
r37770 | finnur@chromium.org | 2010-02-01 16:34:15 -0800 (Mon, 01 Feb 2010) | 8 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/views/find_bar_host_browsertest.cc?r1=37770&r2=37769

Looks like the fix I did last time did work. According to the
flakiness dashboard this hasn't failed since the day I checked
in the fix (Jan 26th).

BUG=http://crbug.com/16447
TEST=None (making a change to a test)

Review URL: http://codereview.chromium.org/565006
------------------------------------------------------------------------

Sign in to add a comment

Powered by Google Project Hosting