Add bookmarks search UI tests #3161

jotaemepereira · 2024-08-27T15:32:34Z

Task/Issue URL: https://app.asana.com/0/1204006570077678/1208027756712863/f
Tech Design URL:
CC:

Description

Adds the following UI tests:

Test 'No results' empty state is shown when searching and no results (both panel and manager)
Test that searching filter the correct results (both panel and manager)
Test 'Show in Folder' menu item action is shown on items when searching (both panel and manager)
Test search actions should be hidden when the user has no bookmarks (panel)

Steps to test

Selec the UI Tests target
Run the tests inside BookmarkSearchTests

Definition of Done:

Does this PR satisfy our Definition of Done?

—

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

github-actions · 2024-08-27T15:34:09Z

	Warnings
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS against 33357fe

jotaemepereira · 2024-08-27T15:34:14Z

DuckDuckGo/Menus/MainMenu.swift

-            bookmarksMenuToggleBookmarksBarMenuItem
-            NSMenuItem.separator()
+        NSMenuItem(title: UserText.bookmarks)
+            .withAccessibilityIdentifier("MainMenu.bookmarks")


ℹ️ This the only change to this class. I added an accessibility identifier, but I had to fix the indentation.

jotaemepereira · 2024-08-27T15:46:08Z

DuckDuckGo/Menus/MainMenuActions.swift

@@ -797,6 +797,10 @@ extension MainViewController {
    @objc func resetBookmarks(_ sender: Any?) {
        LocalBookmarkManager.shared.resetBookmarks()
        UserDefaults.standard.set(false, forKey: UserDefaultsWrapper<Bool>.Key.homePageContinueSetUpImport.rawValue)
+
+
+        UserDefaults.standard.set(false, forKey: UserDefaultsWrapper<Bool>.Key.bookmarksBarPromptShown.rawValue)


ℹ️ I had to to add this to be consistent between new tests. When we reset bookmarks, I want to reset the show bookmarks bar prompt dialog, so it’s shown.

That makes perfect sense 👍 but why are you adjusting onboardingFinished in the line below? It feels to me like a side effect (one wouldn't normally expect onboarding flag to be adjusted when you reset bookmarks, and especially not to have that flag set to true).

I thought it was needed for the show bookmarks bar dialog to appear, but that is not the case. I removed it

jotaemepereira · 2024-08-27T19:57:31Z

UITests/Common/BookmarksUtilites.swift

+
+import XCTest
+
+class BookmarkUtilities {


💡 'I’m having second thoughts about this class. I think another approach is to have an extension of XCUIApplication and define those methods there. Thoughts?

Then it’s easier to read in the tests:

app.openBookmarksManager(..) app.openSiteToBookmark(…) app.dismissPopover(…) // etc ...

Example here: https://github.com/duckduckgo/macos-browser/pull/3162/files#diff-3b229a7e4f5bfeae7c04d751aeb24bc7c35282ce32266902ab2e24a4663002cc

I second your idea 👍 would much better prefer to have an extension on XCUIApplication than standalone utility classes.

Great. I’ve done that ✅

ayoy · 2024-08-29T11:13:30Z

UITests/Common/BookmarksUtilites.swift

+    /// Open the initial site to be bookmarked, bookmarking it and/or escaping out of the dialog only if needed
+    /// - Parameter url: The URL we will use to load the bookmark
+    /// - Parameter pageTitle: The page title that would become the bookmark name
+    /// - Parameter bookmarkingViaDialog: open bookmark dialog, adding bookmark
+    /// - Parameter escapingDialog: `esc` key to leave dialog
+    /// - Parameter folderName: The name of the folder where you want to save the bookmark. If the folder does not exist, it fails.


Thanks a lot for taking time to document the code, especially for these utility APIs 🙏

ayoy

This looks great @jotaemepereira, thanks!
I've run UI Tests in CI for your branch and it's green: https://github.com/duckduckgo/macos-browser/actions/runs/10613949826

Left a few comments. I'm mostly interested about adjusting onboardingFinished flag from "Reset Bookmarks" menu option.

BookmarksUtilities can be rewritten as XCUIApplication extension now, or after you merge this 👍

jotaemepereira · 2024-09-02T19:22:33Z

The feedback was addressed, but I’m waiting for #3188 to be merged, given that a bug introduced by #3138 makes this test fail.

* main: (28 commits) Fix bookmark sort UI tests failure (#3219) Add tests for DuckPlayer pixel calculation (#3216) Implement checkbox VPN exclusions UI (#3207) fix zoom updated (#3140) Bump version to 1.105.0 (255) Add DuckPlayer enrollment pixels (#3190) Update PeopleFinders to address broker changes (#3208) Bump BSK with C-S-S to 6.14.0 (#3209) Zoom PDF controls (#3204) Fix exception on Copy in Save dialog (#3205) Remove DuckPlayer onboarding animation (#3198) Fix wrong URL displayed for auth dialog (#3191) Bump version to 1.105.0 (254) UI Ship review feedback for Duck Player onboarding (#3186) Implement bookmarks sort UI tests (#3162) Add bookmarks search UI tests (#3161) Bug: Disable boomark reordering when searching (#3188) Bump version to 1.105.0 (253) Remote feature flag for New Tab Page Improvements (#3176) Fix bookmarks bar issues (#3187) ...

jotaemepereira commented Aug 27, 2024

View reviewed changes

jotaemepereira force-pushed the juan/ui-testing-bookmarks-search branch from f8834ba to 131b5dd Compare August 27, 2024 15:44

jotaemepereira commented Aug 27, 2024

View reviewed changes

jotaemepereira requested review from ayoy and SabrinaTardio August 27, 2024 15:46

jotaemepereira force-pushed the juan/ui-testing-bookmarks-search branch 3 times, most recently from 74ef78a to fd58733 Compare August 27, 2024 15:50

jotaemepereira commented Aug 27, 2024

View reviewed changes

ayoy reviewed Aug 29, 2024

View reviewed changes

ayoy approved these changes Aug 29, 2024

View reviewed changes

ayoy assigned jotaemepereira Aug 29, 2024

jotaemepereira force-pushed the juan/ui-testing-bookmarks-search branch from 805c2d1 to 7f19b10 Compare September 2, 2024 19:21

jotaemepereira and others added 3 commits September 3, 2024 10:26

Add bookmarks search UI tests

21f405d

Fix a typo

fa18f03

Address feedback

33357fe

jotaemepereira force-pushed the juan/ui-testing-bookmarks-search branch from 7f19b10 to 33357fe Compare September 3, 2024 13:37

jotaemepereira merged commit 8561bd2 into main Sep 3, 2024
18 checks passed

jotaemepereira deleted the juan/ui-testing-bookmarks-search branch September 3, 2024 13:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add bookmarks search UI tests #3161

Add bookmarks search UI tests #3161

jotaemepereira commented Aug 27, 2024 •

edited

Loading

github-actions bot commented Aug 27, 2024 •

edited

Loading

jotaemepereira Aug 27, 2024 •

edited

Loading

jotaemepereira Aug 27, 2024

ayoy Aug 29, 2024

jotaemepereira Sep 2, 2024

jotaemepereira Aug 27, 2024

jotaemepereira Aug 27, 2024

ayoy Aug 29, 2024

jotaemepereira Sep 2, 2024

ayoy Aug 29, 2024

ayoy left a comment

jotaemepereira commented Sep 2, 2024


		import XCTest

		class BookmarkUtilities {

Add bookmarks search UI tests #3161

Add bookmarks search UI tests #3161

Conversation

jotaemepereira commented Aug 27, 2024 • edited Loading

Description

Steps to test

Internal references:

github-actions bot commented Aug 27, 2024 • edited Loading

jotaemepereira Aug 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ayoy left a comment

Choose a reason for hiding this comment

jotaemepereira commented Sep 2, 2024

jotaemepereira commented Aug 27, 2024 •

edited

Loading

github-actions bot commented Aug 27, 2024 •

edited

Loading

jotaemepereira Aug 27, 2024 •

edited

Loading