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

Issue 782551 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug



Sign in to add a comment

Unittests and egtests need to be added/modified for the new iOS bookmark ui.

Project Member Reported by martiw@chromium.org, Nov 8 2017

Issue description

Some unittests are broken when the new bookmark flag is ON.
We need to rewrite them.
 
Summary: 2 unittests need to be rewritten for the new iOS bookmark ui. (was: A few unittests need to be rewritten for the new iOS bookmark ui.)
The 2 unittests needs rewrite:
BookmarkHomeViewControllerTest -> LoadBookmarks
NewTabPageControllerTest -> SelectBookmarkPanel

Comment 2 by martiw@chromium.org, Nov 12 2017

Summary: Unittests and egtests need to be modified for the new iOS bookmark ui. (was: 2 unittests need to be rewritten for the new iOS bookmark ui.)
Summary: Unittests and egtests need to be added/modified for the new iOS bookmark ui. (was: Unittests and egtests need to be modified for the new iOS bookmark ui.)
Working on crrev.com/c/807410 to add 2 egtests: testWhenCurrentFolderDeletedInBackground and testBookmarksURLDisabled.
Cc: linds...@chromium.org
Project Member

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

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

commit 5ab0c368f8ae52fdb29b337e889f1728897526ac
Author: Marti Wong <martiw@chromium.org>
Date: Wed Dec 06 00:45:16 2017

Add 2 egtests for the new iOS bookmarks

2 egtest2 are added:
1. testWhenCurrentFolderDeletedInBackground: test when the navigating
   folder of Bookmarks is deleted in background, empty background is
   shown with context bar buttons disabled.
2. testBookmarksURLDisabled: test chrome://bookmarks is disabled.

Bug:  782551 
Change-Id: I1e0f4cc4960aca3094e673e812d040ec231d497c
Reviewed-on: https://chromium-review.googlesource.com/807410
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Marti Wong <martiw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521921}
[modify] https://crrev.com/5ab0c368f8ae52fdb29b337e889f1728897526ac/ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller.mm
[modify] https://crrev.com/5ab0c368f8ae52fdb29b337e889f1728897526ac/ios/chrome/browser/ui/bookmarks/bookmark_table_view.mm
[modify] https://crrev.com/5ab0c368f8ae52fdb29b337e889f1728897526ac/ios/chrome/browser/ui/bookmarks/bookmarks_new_generation_egtest.mm

Comment 7 by jlebel@chromium.org, Dec 20 2017

Blocking: 796085
Project Member

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

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

commit 9fc43c0b446b01f8346ecf94281f12e678d5d4a2
Author: Jérôme Lebel <jlebel@chromium.org>
Date: Thu Dec 21 08:46:52 2017

[iOS][Signin] Fixing SigninInteractionControllerTestCase on first run

Converting SigninInteractionControllerTestCase to use the new UI for
recent tabs.
This way the RecentTabsTableViewController created with
RecentTabsTableTestCase tests will be always deallocated.
See crbug.com/796506
The deallocated issue only appears with EarlGrey tests. It is not an
issue with Chrome app itself.

Bug: 796085,  782551 
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: If4ea1ddf7255a1f6cc8ea58007257468b9cf247f
Reviewed-on: https://chromium-review.googlesource.com/836897
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525649}
[modify] https://crrev.com/9fc43c0b446b01f8346ecf94281f12e678d5d4a2/ios/chrome/browser/ui/ntp/recent_tabs/recent_tabs_table_egtest.mm
[modify] https://crrev.com/9fc43c0b446b01f8346ecf94281f12e678d5d4a2/ios/chrome/browser/ui/ntp/recent_tabs/recent_tabs_table_view_controller.h
[modify] https://crrev.com/9fc43c0b446b01f8346ecf94281f12e678d5d4a2/ios/chrome/browser/ui/ntp/recent_tabs/recent_tabs_table_view_controller.mm

Comment 9 by jlebel@chromium.org, Dec 21 2017

Blocking: -796085
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 29 2017

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

commit 2c5e14473b2c8888d4a11f120273e286d916add6
Author: Marti Wong <martiw@chromium.org>
Date: Fri Dec 29 13:57:29 2017

Modify egtests/unittests for the new iOS bookmarks.

- Modify the following tests, make them pass when the new iOS
   bookmarks flag is ON:
   1. BookmarkHomeViewControllerTest: LoadBookmarks
   2. keyboard_commands_egtest: testKeyboard...BookmarksPresented
   3. signin_interaction_controller_egtest: testSignIn...Bookmarks

- Add some TODOs to cleanup old bookmarks' properties, functions &
  following NTP Bookmarks tests:
   1. NewTabPageControllerTest: SelectBookmarkPanel
   2. new_tab_page_egtest: testAccessibilityOnBookmarks

Bug:  782551 
Change-Id: I4d60ad016d252643e5a04563671cf4f2d20df499
Reviewed-on: https://chromium-review.googlesource.com/846679
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Marti Wong <martiw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526373}
[modify] https://crrev.com/2c5e14473b2c8888d4a11f120273e286d916add6/ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller.mm
[modify] https://crrev.com/2c5e14473b2c8888d4a11f120273e286d916add6/ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller_protected.h
[modify] https://crrev.com/2c5e14473b2c8888d4a11f120273e286d916add6/ios/chrome/browser/ui/bookmarks/bookmark_home_view_controller_unittest.mm
[modify] https://crrev.com/2c5e14473b2c8888d4a11f120273e286d916add6/ios/chrome/browser/ui/keyboard_commands_egtest.mm
[modify] https://crrev.com/2c5e14473b2c8888d4a11f120273e286d916add6/ios/chrome/browser/ui/ntp/new_tab_page_controller_unittest.mm
[modify] https://crrev.com/2c5e14473b2c8888d4a11f120273e286d916add6/ios/chrome/browser/ui/ntp/new_tab_page_egtest.mm
[modify] https://crrev.com/2c5e14473b2c8888d4a11f120273e286d916add6/ios/chrome/browser/ui/signin_interaction/signin_interaction_controller_egtest.mm

Project Member

Comment 11 by bugdroid1@chromium.org, Jan 10 2018

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

commit 32ae123074234d140ead8754c2fc852772ca87e4
Author: Marti Wong <martiw@chromium.org>
Date: Wed Jan 10 00:01:50 2018

Copy egtests from the old bookmarks to the new bookmarks.

Before cleanup the old bookmarks, copy (and modify) the following from
bookmarks_egtest to bookmarks_new_generation_egtest:
testAddRemoveBookmark
testBookmarkMultipleTabs
testAddBookmarkInNewFolder
testStickyDefaultFolder
testMoveDoesSaveOnSave
testCreateNewFolderWhileMovingBookmark
testKeyboardCommandsRegistered_AddBookmark
testKeyboardCommandsNotRegistered_EditBookmark

The following tests in the bookmarks_egtest are already covered (or no
longer relevant) in bookmarks_new_generation_egtest and no need to
copy:
testTapBookmark
testSelectBookmark
testUndoDeleteBookmark
testUndoDeleteBookmarkFromEditScreen
testUndoMoveBookmark
testLabelUpdatedUponMove
testChangeFolderTitle
testSingleBookmarkEdit
testSingleBookmarkCancelEdit
testLongPressBookmark
testEditFolder
testDeleteFolder
testDeleteCurrentSubfolder
testDeleteParentFolder
testBrowseNestedFolders
testDeleteRootFolder
testPromoNoThanksMakeItDisappear
testSignInPromoWithColdStateUsingPrimaryButton
testSignInPromoWithWarmStateUsingPrimaryButton
testSignInPromoWithWarmStateUsingSecondaryButton
testAutomaticSigninPromoDismiss
testAccessibilityOnBookmarksLandingPage
testAccessibilityOnBookmarksEditPage
testAccessibilityOnBookmarksMovePage
testAccessibilityOnBookmarksMoveToNewFolderPage
testAccessibilityOnBookmarksDeleteUndo
testAccessibilityOnBookmarksSelect

Bug:  782551 
Change-Id: I0b8b65ad9992c749261b52d426a8b2b093c5461a
Reviewed-on: https://chromium-review.googlesource.com/853738
Commit-Queue: Marti Wong <martiw@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528173}
[modify] https://crrev.com/32ae123074234d140ead8754c2fc852772ca87e4/ios/chrome/browser/ui/bookmarks/bookmarks_egtest.mm
[modify] https://crrev.com/32ae123074234d140ead8754c2fc852772ca87e4/ios/chrome/browser/ui/bookmarks/bookmarks_new_generation_egtest.mm

Status: Fixed (was: Assigned)

Sign in to add a comment