New issue
Advanced search Search tips

Issue 810479 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug

Blocking:
issue 802257
issue 817425



Sign in to add a comment

polychrome: fix unit_tests

Project Member Reported by ellyjo...@chromium.org, Feb 8 2018

Issue description

We need to make sure unit_tests passes in polychrome builds. This is a prereq for flipping the flag on trunk.
 
Cc: -ellyjo...@chromium.org
Owner: ellyjo...@chromium.org
Status: Assigned (was: Available)
17 tests failed:
    BookmarkBarViewTest.AddNodesWhenBarAlreadySized (../../chrome/browser/ui/views/bookmarks/bookmark_bar_view_unittest.cc:228)
    BookmarkBarViewTest.AppsShortcutVisibility (../../chrome/browser/ui/views/bookmarks/bookmark_bar_view_unittest.cc:142)
    BookmarkBarViewTest.ButtonsDynamicallyAdded (../../chrome/browser/ui/views/bookmarks/bookmark_bar_view_unittest.cc:213)
    BookmarkBarViewTest.ButtonsDynamicallyAddedAfterModelHasNodes (../../chrome/browser/ui/views/bookmarks/bookmark_bar_view_unittest.cc:194)
    BookmarkBarViewTest.ChangeTitle (../../chrome/browser/ui/views/bookmarks/bookmark_bar_view_unittest.cc:289)
    BookmarkBarViewTest.ManagedShowAppsShortcutInBookmarksBar (../../chrome/browser/ui/views/bookmarks/bookmark_bar_view_unittest.cc:336)
    BookmarkBarViewTest.MoveNode (../../chrome/browser/ui/views/bookmarks/bookmark_bar_view_unittest.cc:257)
    BookmarkBarViewTest.OverflowVisibility (../../chrome/browser/ui/views/bookmarks/bookmark_bar_view_unittest.cc:165)
    BookmarkBarViewTest.RemoveNode (../../chrome/browser/ui/views/bookmarks/bookmark_bar_view_unittest.cc:238)
    BookmarkBarViewTest.UpdateTooltipText (../../chrome/browser/ui/views/bookmarks/bookmark_bar_view_unittest.cc:357)
    ChooserDialogViewTest.CancelButtonFocusedWhenReScanIsPressed (../../chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc:95)
    ReloadButtonTest.Basic (../../chrome/browser/ui/views/toolbar/reload_button_unittest.cc:60)
    ReloadButtonTest.DisableOnHover (../../chrome/browser/ui/views/toolbar/reload_button_unittest.cc:112)
    ReloadButtonTest.DoubleClickTimer (../../chrome/browser/ui/views/toolbar/reload_button_unittest.cc:85)
    ReloadButtonTest.ResetOnClick (../../chrome/browser/ui/views/toolbar/reload_button_unittest.cc:135)
    ReloadButtonTest.ResetOnTimer (../../chrome/browser/ui/views/toolbar/reload_button_unittest.cc:150)
    TabTest.HitTestTopPixel (../../chrome/browser/ui/views/tabs/tab_unittest.cc:265)
2 tests failed on exit:
    ChooserDialogViewTest.Cancel (../../chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc:126)
    ChooserDialogViewTest.Close (../../chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc:131)
29 tests crashed:
    AlertIndicatorButtonTest.ButtonUpdateOnAudioStateAnimation (../../chrome/browser/ui/views/tabs/alert_indicator_button_unittest.cc:76)
    AutofillPopupViewNativeViewsForEveryTypeTest.ShowClickTest/0 (../../chrome/browser/ui/views/autofill/autofill_popup_view_native_views_unittest.cc:171)
    AutofillPopupViewNativeViewsForEveryTypeTest.ShowClickTest/1 (../../chrome/browser/ui/views/autofill/autofill_popup_view_native_views_unittest.cc:171)
    AutofillPopupViewNativeViewsForEveryTypeTest.ShowClickTest/10 (../../chrome/browser/ui/views/autofill/autofill_popup_view_native_views_unittest.cc:171)
    AutofillPopupViewNativeViewsForEveryTypeTest.ShowClickTest/11 (../../chrome/browser/ui/views/autofill/autofill_popup_view_native_views_unittest.cc:171)
    AutofillPopupViewNativeViewsForEveryTypeTest.ShowClickTest/12 (../../chrome/browser/ui/views/autofill/autofill_popup_view_native_views_unittest.cc:171)
    AutofillPopupViewNativeViewsForEveryTypeTest.ShowClickTest/13 (../../chrome/browser/ui/views/autofill/autofill_popup_view_native_views_unittest.cc:171)
    AutofillPopupViewNativeViewsForEveryTypeTest.ShowClickTest/14 (../../chrome/browser/ui/views/autofill/autofill_popup_view_native_views_unittest.cc:171)
    AutofillPopupViewNativeViewsForEveryTypeTest.ShowClickTest/2 (../../chrome/browser/ui/views/autofill/autofill_popup_view_native_views_unittest.cc:171)
    AutofillPopupViewNativeViewsForEveryTypeTest.ShowClickTest/3 (../../chrome/browser/ui/views/autofill/autofill_popup_view_native_views_unittest.cc:171)
    AutofillPopupViewNativeViewsForEveryTypeTest.ShowClickTest/4 (../../chrome/browser/ui/views/autofill/autofill_popup_view_native_views_unittest.cc:171)
    AutofillPopupViewNativeViewsForEveryTypeTest.ShowClickTest/5 (../../chrome/browser/ui/views/autofill/autofill_popup_view_native_views_unittest.cc:171)
    AutofillPopupViewNativeViewsForEveryTypeTest.ShowClickTest/6 (../../chrome/browser/ui/views/autofill/autofill_popup_view_native_views_unittest.cc:171)
    AutofillPopupViewNativeViewsForEveryTypeTest.ShowClickTest/7 (../../chrome/browser/ui/views/autofill/autofill_popup_view_native_views_unittest.cc:171)
    AutofillPopupViewNativeViewsForEveryTypeTest.ShowClickTest/8 (../../chrome/browser/ui/views/autofill/autofill_popup_view_native_views_unittest.cc:171)
    AutofillPopupViewNativeViewsForEveryTypeTest.ShowClickTest/9 (../../chrome/browser/ui/views/autofill/autofill_popup_view_native_views_unittest.cc:171)
    AutofillPopupViewNativeViewsTest.ShowHideTest (../../chrome/browser/ui/views/autofill/autofill_popup_view_native_views_unittest.cc:164)
    BrowserNonClientFrameViewPopupTest.HitTestPopupTopChrome (../../chrome/browser/ui/views/frame/browser_non_client_frame_view_unittest.cc:47)
    BrowserNonClientFrameViewTabbedTest.HitTestTabstrip (../../chrome/browser/ui/views/frame/browser_non_client_frame_view_unittest.cc:63)
    BrowserViewHostedAppTest.Layout (../../chrome/browser/ui/views/frame/browser_view_unittest.cc:276)
    BrowserViewTest.AccessibleWindowTitle (../../chrome/browser/ui/views/frame/browser_view_unittest.cc:220)
    BrowserViewTest.BookmarkBarInvisibleOnShutdown (../../chrome/browser/ui/views/frame/browser_view_unittest.cc:199)
    BrowserViewTest.BrowserView (../../chrome/browser/ui/views/frame/browser_view_unittest.cc:59)
    BrowserViewTest.BrowserViewLayout (../../chrome/browser/ui/views/frame/browser_view_unittest.cc:76)
    BrowserViewTest.RepeatedAccelerators (../../chrome/browser/ui/views/frame/browser_view_unittest.cc:182)
    ToolbarActionsBarBubbleViewsTest.TestClickActionButton (../../chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views_unittest.cc:240)
    ToolbarActionsBarBubbleViewsTest.TestClickDismissButton (../../chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views_unittest.cc:256)
    ToolbarActionsBarBubbleViewsTest.TestClickLearnMoreLink (../../chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views_unittest.cc:271)
    UserManagerMacTest.ShowUserManager (../../chrome/browser/ui/cocoa/profiles/user_manager_mac_unittest.mm:46)

The very last crash is known (issue 810139), the rest are new to polychrome. In I go :)
Status: Started (was: Assigned)
After <https://chromium-review.googlesource.com/#/c/chromium/src/+/911710>, the first set of fixes:

5 tests failed:
    ChooserDialogViewTest.CancelButtonFocusedWhenReScanIsPressed (../../chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc:95)
    ColorPanelCocoaTest.ClearTargetAndDelegateOnEnd (../../chrome/browser/ui/cocoa/color_panel_cocoa_unittest.mm:31)
    ColorPanelCocoaTest.ClearTargetOnEnd (../../chrome/browser/ui/cocoa/color_panel_cocoa_unittest.mm:52)
    SadTabViewBehaviorTest.ClickOnLinks (../../chrome/browser/ui/cocoa/tab_contents/sad_tab_mac_unittest.mm:39)
    TabTest.HitTestTopPixel (../../chrome/browser/ui/views/tabs/tab_unittest.cc:265)
2 tests failed on exit:
    ChooserDialogViewTest.Cancel (../../chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc:126)
    ChooserDialogViewTest.Close (../../chrome/browser/ui/views/extensions/chooser_dialog_view_unittest.cc:131)
13 tests crashed:
    AlertIndicatorButtonTest.ButtonUpdateOnAudioStateAnimation (../../chrome/browser/ui/views/tabs/alert_indicator_button_unittest.cc:76)
    BrowserNonClientFrameViewPopupTest.HitTestPopupTopChrome (../../chrome/browser/ui/views/frame/browser_non_client_frame_view_unittest.cc:47)
    BrowserNonClientFrameViewTabbedTest.HitTestTabstrip (../../chrome/browser/ui/views/frame/browser_non_client_frame_view_unittest.cc:63)
    BrowserViewHostedAppTest.Layout (../../chrome/browser/ui/views/frame/browser_view_unittest.cc:276)
    BrowserViewTest.AccessibleWindowTitle (../../chrome/browser/ui/views/frame/browser_view_unittest.cc:220)
    BrowserViewTest.BookmarkBarInvisibleOnShutdown (../../chrome/browser/ui/views/frame/browser_view_unittest.cc:199)
    BrowserViewTest.BrowserView (../../chrome/browser/ui/views/frame/browser_view_unittest.cc:59)
    BrowserViewTest.BrowserViewLayout (../../chrome/browser/ui/views/frame/browser_view_unittest.cc:76)
    BrowserViewTest.RepeatedAccelerators (../../chrome/browser/ui/views/frame/browser_view_unittest.cc:182)
    ToolbarActionsBarBubbleViewsTest.TestClickActionButton (../../chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views_unittest.cc:240)
    ToolbarActionsBarBubbleViewsTest.TestClickDismissButton (../../chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views_unittest.cc:256)
    ToolbarActionsBarBubbleViewsTest.TestClickLearnMoreLink (../../chrome/browser/ui/views/toolbar/toolbar_actions_bar_bubble_views_unittest.cc:271)
    UserManagerMacTest.ShowUserManager (../../chrome/browser/ui/cocoa/profiles/user_manager_mac_unittest.mm:46)
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 12 2018

Project Member

Comment 4 by bugdroid1@chromium.org, Feb 13 2018

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

commit 23abb3f5a5388283021d8411e0f8436166d61bec
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Tue Feb 13 20:17:08 2018

polychrome: use Views browser window for some Views browser tests

Otherwise, browser()->window() is a Cocoa window, which causes chaos.

Bug:  810479 
Change-Id: Idd91922b2cdf732732fe962207c7f49989fb9b19
Reviewed-on: https://chromium-review.googlesource.com/912011
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536440}
[modify] https://crrev.com/23abb3f5a5388283021d8411e0f8436166d61bec/chrome/browser/ui/views/frame/test_with_browser_view.cc
[modify] https://crrev.com/23abb3f5a5388283021d8411e0f8436166d61bec/chrome/browser/ui/views/frame/test_with_browser_view.h

Issue 817420 has been merged into this issue.
Cc: ellyjo...@chromium.org
Owner: lgrey@chromium.org
Blocking: 817425
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 9 2018

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

commit 914e0d56f6020d170f55149fcb3bb2fb176a7413
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Fri Mar 09 16:50:14 2018

polychrome: virtualize BrowserActionTestUtil

This change:
1) Makes BrowserActionTestUtil into an interface instead of a concrete class;
2) Adds two implementations: BrowserActionTestUtilCocoa (for Cocoa windows)
   and BrowserActionTestUtilViews (for Views windows);
3) Adds BrowserActionTestUtil::Create as a factory function to create an
   instance of the appropriate concrete class depending on which browser
   window type is in use;
4) Migrates all uses of BrowserActionTestUtil's constructors to that factory
   function;
5) Links both implementations of BrowserActionTestUtil into polychrome builds

Bug:  810479 , 817408
Change-Id: I68eb74c6dd4fa58d9d064b3b942a620fd0f8a81b
Reviewed-on: https://chromium-review.googlesource.com/956026
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542140}
[modify] https://crrev.com/914e0d56f6020d170f55149fcb3bb2fb176a7413/chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc
[modify] https://crrev.com/914e0d56f6020d170f55149fcb3bb2fb176a7413/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc
[modify] https://crrev.com/914e0d56f6020d170f55149fcb3bb2fb176a7413/chrome/browser/extensions/api/extension_action/browser_action_interactive_test.cc
[modify] https://crrev.com/914e0d56f6020d170f55149fcb3bb2fb176a7413/chrome/browser/extensions/extension_incognito_apitest.cc
[modify] https://crrev.com/914e0d56f6020d170f55149fcb3bb2fb176a7413/chrome/browser/extensions/extension_keybinding_apitest.cc
[modify] https://crrev.com/914e0d56f6020d170f55149fcb3bb2fb176a7413/chrome/browser/extensions/lazy_background_page_apitest.cc
[modify] https://crrev.com/914e0d56f6020d170f55149fcb3bb2fb176a7413/chrome/browser/extensions/process_manager_browsertest.cc
[modify] https://crrev.com/914e0d56f6020d170f55149fcb3bb2fb176a7413/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/914e0d56f6020d170f55149fcb3bb2fb176a7413/chrome/browser/ui/cocoa/extensions/browser_action_test_util_views_cocoa.mm
[modify] https://crrev.com/914e0d56f6020d170f55149fcb3bb2fb176a7413/chrome/browser/ui/extensions/browser_action_test_util.h
[modify] https://crrev.com/914e0d56f6020d170f55149fcb3bb2fb176a7413/chrome/browser/ui/toolbar/browser_actions_bar_browsertest.cc
[modify] https://crrev.com/914e0d56f6020d170f55149fcb3bb2fb176a7413/chrome/browser/ui/toolbar/component_toolbar_actions_browsertest.cc
[modify] https://crrev.com/914e0d56f6020d170f55149fcb3bb2fb176a7413/chrome/browser/ui/toolbar/media_router_action_unittest.cc
[modify] https://crrev.com/914e0d56f6020d170f55149fcb3bb2fb176a7413/chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc
[modify] https://crrev.com/914e0d56f6020d170f55149fcb3bb2fb176a7413/chrome/browser/ui/toolbar/toolbar_actions_bar_unittest.cc
[modify] https://crrev.com/914e0d56f6020d170f55149fcb3bb2fb176a7413/chrome/browser/ui/views/toolbar/browser_action_test_util_views.cc
[add] https://crrev.com/914e0d56f6020d170f55149fcb3bb2fb176a7413/chrome/browser/ui/views/toolbar/browser_action_test_util_views.h

Project Member

Comment 9 by bugdroid1@chromium.org, Mar 20 2018

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

commit 45616255b1c1a9754a445f5174be04f6b5fd9659
Author: Leonard Grey <lgrey@chromium.org>
Date: Tue Mar 20 16:32:00 2018

Disable menu-related translate bubble tests on Mac

NSMenus on Mac block in a nested run-loop, so any tests that open a menu
and try to do things to it or sense things on it either:
- Succeed by accident because of a race condition with app activations
- Block until the menu or another window is interacted with

Disabling these until we've either been able to make NSMenus asynchronous
to match views expectations, or found a way to test this functionality a
different way.

|AlwaysTranslateLanguageMenuItem| has an additional problem: pressing a
button halfway through closes the bubble, but on non-Mac platforms, the
test can continue due to asynchronous window closing. On Mac, the window's
content view is removed as soon as closing begins. We'll probably need
to address this by breaking it up into multiple tests and creating a way
to sense the state without relying on an object owned by the closing
bubble.

Bug:  810479 , 823735 
Change-Id: I9d9d9ca49a3436b91d3cc99fb43ecef13d2e77c0
Reviewed-on: https://chromium-review.googlesource.com/969653
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544400}
[modify] https://crrev.com/45616255b1c1a9754a445f5174be04f6b5fd9659/chrome/browser/ui/views/translate/translate_bubble_view_unittest.cc

Comment 10 by lgrey@chromium.org, Mar 20 2018

Status: Fixed (was: Started)
Project Member

Comment 11 by bugdroid1@chromium.org, Mar 22 2018

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

commit cc076cf046cc7f62c7b826ca1fc7afad51efba93
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Thu Mar 22 19:09:41 2018

polychrome: mark AcceleratorTableTest views-only

It compares the Views accelerator table to the keyboard shortcut table, which
only makes sense to do in Views browsers.

TBR=pkasting@chromium.org

Bug:  810479 
Change-Id: I4bde07a0cc9a80894772e9e6aa0b275e30d04671
Reviewed-on: https://chromium-review.googlesource.com/976372
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545185}
[modify] https://crrev.com/cc076cf046cc7f62c7b826ca1fc7afad51efba93/chrome/browser/ui/views/accelerator_table_unittest_mac.mm

Sign in to add a comment