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

Issue 796969 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

"ScreenCaptureNotificationUICocoaTest.ShortTitle" is flaky

Project Member Reported by chromium...@appspot.gserviceaccount.com, Dec 21 2017

Issue description

"ScreenCaptureNotificationUICocoaTest.ShortTitle" is flaky.

This issue was created automatically by the chromium-try-flakes app. Please find the right owner to fix the respective test/step and assign this issue to them. If the step/test is infrastructure-related, please add Infra-Troopers label and change issue status to Untriaged. When done, please remove the issue from Sheriff Bug Queue by removing the Sheriff-Chromium label.

We have detected 3 recent flakes. List of all flakes can be found at https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNyOgsSBUZsYWtlIi9TY3JlZW5DYXB0dXJlTm90aWZpY2F0aW9uVUlDb2NvYVRlc3QuU2hvcnRUaXRsZQw.

Flaky tests should be disabled within 30 minutes unless culprit CL is found and reverted. Please see more details here: https://sites.google.com/a/chromium.org/dev/developers/tree-sheriffs/sheriffing-bug-queues#triaging-auto-filed-flakiness-bugs
 

Comment 1 by osh...@chromium.org, Dec 21 2017

Cc: rogerm@chromium.org nicoh@chromium.org
Owner: sergeyu@chromium.org
Looks like this has been flaky for a while. I'll disable the test.

sergeyu@, can you either triage this or find the right owner?

+nico@ who reviewed the CL.

Comment 2 by osh...@chromium.org, Dec 21 2017

Maybe this depends on some environment? I can relax the condition to 460 or 461 if that's desirable. lmk.  

[ RUN      ] ScreenCaptureNotificationUICocoaTest.ShortTitle
../../chrome/browser/ui/cocoa/screen_capture_notification_ui_cocoa_unittest.mm:89: Failure
Expected equality of these values:
  460
  NSWidth([[controller() window] frame])
    Which is: 461

Project Member

Comment 3 by bugdroid1@chromium.org, Dec 21 2017

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

commit 31be251e38a6ad9d987c96e79095c5c1dbdfaaaf
Author: Mitsuru Oshima <oshima@chromium.org>
Date: Thu Dec 21 18:24:23 2017

Disable flaky ScreenCaptureNotificationUICocoaTest.ShortTitle test

BUG= 796969 
TEST=None
TBR=rogerm@chromium.org,sergeyu@chromium.org

Change-Id: I92bbee2ce723b5adc3cba9fcd91936c6fd35f2d7
Reviewed-on: https://chromium-review.googlesource.com/840440
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525745}
[modify] https://crrev.com/31be251e38a6ad9d987c96e79095c5c1dbdfaaaf/chrome/browser/ui/cocoa/screen_capture_notification_ui_cocoa_unittest.mm

Relaxing that check seems reasonable. LongTitle test contains this comment:

  // The elided label sometimes is a few pixels longer than the max width. So
  // allow a 5px off from the 1000px maximium.
  EXPECT_LE(NSWidth([[controller() window] frame]), 1005);

So we should do the same in ShortTitle.
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 21 2017

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

commit 2c029431c2db4119ff921b82dc1247790b0b684f
Author: Sergey Ulanov <sergeyu@chromium.org>
Date: Thu Dec 21 21:45:41 2017

Fix flake in ScreenCaptureNotificationUICocoaTest.ShortTitle

ScreenCaptureNotificationUICocoaTest.ShortTitle was checking that window width is set
to 460. But ScreenCaptureNotificationUICocoa doesn't set this size directly. Instead
it sets dimensions for controls inside the window. The test was flaky because
sometimes window width was set to 461.
Relaxed window dimensions check to make it less flaky.

Bug:  796969 
Change-Id: I82a7930efcd1ce8543e0723d43981eb1348e17d7
Reviewed-on: https://chromium-review.googlesource.com/840448
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525817}
[modify] https://crrev.com/2c029431c2db4119ff921b82dc1247790b0b684f/chrome/browser/ui/cocoa/screen_capture_notification_ui_cocoa_unittest.mm

Status: Fixed (was: Untriaged)
Project Member

Comment 7 by bugdroid1@chromium.org, Dec 21 2017

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

commit c9713a9f1cb27e76e1280d56d48083d0399f3dfc
Author: Sergey Ulanov <sergeyu@chromium.org>
Date: Thu Dec 21 23:48:51 2017

Revert LongTitle test changes from crrev.com/525817

crrev.com/525817 Fixed an issue in
ScreenCaptureNotificationUICocoaTest.ShortTitle and also updated
LongTitle test for consistency. That change in LongTitle test
inadvertently broke that test. Reverting changes in LongTitle test
to unbreak it.

TBR=oshima@chromium.org

Bug:  796969 
Change-Id: I595ecf30bb9ba0d2e041515abc7cd08a064bd527
Reviewed-on: https://chromium-review.googlesource.com/841524
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525857}
[modify] https://crrev.com/c9713a9f1cb27e76e1280d56d48083d0399f3dfc/chrome/browser/ui/cocoa/screen_capture_notification_ui_cocoa_unittest.mm

Comment 8 by awdf@chromium.org, Jan 8 2018

 Issue 794836  has been merged into this issue.

Sign in to add a comment