From 773d073bf9734c7c8389a7bbc6ebadab563b5faa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariusz=20=C5=9Apiewak?= Date: Tue, 9 Apr 2024 15:23:46 +0200 Subject: [PATCH 1/4] Keep windows miniaturized upon relaunching (#2569) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task/Issue URL: https://app.asana.com/0/276630244458377/1206774148393444/f Tech Design URL: CC: **Description**: Stores `isMiniaturized` property in `WindowRestorationItem` and applies to respective window during launch. **Steps to test this PR**: 1. Open a bunch of windows (preferably with a loaded URL). 2. Minimize some of them. 3. Restart the app. 4. Previously miniaturized windows should reopen in the same state (visible only in dock). — ###### Internal references: [Pull Request Review Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f) [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) [Pull Request Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f) --- .../StateRestoration/WindowManager+StateRestoration.swift | 7 ++++++- DuckDuckGo/Windows/View/WindowsManager.swift | 5 ++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/DuckDuckGo/StateRestoration/WindowManager+StateRestoration.swift b/DuckDuckGo/StateRestoration/WindowManager+StateRestoration.swift index f9c84e5055..aaa38da89c 100644 --- a/DuckDuckGo/StateRestoration/WindowManager+StateRestoration.swift +++ b/DuckDuckGo/StateRestoration/WindowManager+StateRestoration.swift @@ -52,7 +52,7 @@ extension WindowsManager { } private class func setUpWindow(from item: WindowRestorationItem) { - guard let window = openNewWindow(with: item.model, showWindow: true) else { return } + guard let window = openNewWindow(with: item.model, showWindow: !item.isMiniaturized, isMiniaturized: item.isMiniaturized) else { return } window.setContentSize(item.frame.size) window.setFrameOrigin(item.frame.origin) } @@ -135,11 +135,13 @@ final class WindowRestorationItem: NSObject, NSSecureCoding { private enum NSSecureCodingKeys { static let frame = "frame" static let model = "model" + static let isMiniaturized = "isMiniaturized" } let model: TabCollectionViewModel let frame: NSRect + let isMiniaturized: Bool @MainActor init?(windowController: MainWindowController) { @@ -150,6 +152,7 @@ final class WindowRestorationItem: NSObject, NSSecureCoding { self.frame = windowController.window!.frame self.model = windowController.mainViewController.tabCollectionViewModel + self.isMiniaturized = windowController.window!.isMiniaturized } static var supportsSecureCoding: Bool { true } @@ -161,10 +164,12 @@ final class WindowRestorationItem: NSObject, NSSecureCoding { } self.model = model self.frame = coder.decodeRect(forKey: NSSecureCodingKeys.frame) + self.isMiniaturized = coder.decodeBool(forKey: NSSecureCodingKeys.isMiniaturized) } func encode(with coder: NSCoder) { coder.encode(frame, forKey: NSSecureCodingKeys.frame) coder.encode(model, forKey: NSSecureCodingKeys.model) + coder.encode(isMiniaturized, forKey: NSSecureCodingKeys.isMiniaturized) } } diff --git a/DuckDuckGo/Windows/View/WindowsManager.swift b/DuckDuckGo/Windows/View/WindowsManager.swift index 245b36ed0c..007f850990 100644 --- a/DuckDuckGo/Windows/View/WindowsManager.swift +++ b/DuckDuckGo/Windows/View/WindowsManager.swift @@ -61,7 +61,8 @@ final class WindowsManager { contentSize: NSSize? = nil, showWindow: Bool = true, popUp: Bool = false, - lazyLoadTabs: Bool = false) -> MainWindow? { + lazyLoadTabs: Bool = false, + isMiniaturized: Bool = false) -> MainWindow? { let mainWindowController = makeNewWindow(tabCollectionViewModel: tabCollectionViewModel, popUp: popUp, burnerMode: burnerMode, @@ -71,6 +72,8 @@ final class WindowsManager { mainWindowController.window?.setContentSize(contentSize) } + mainWindowController.window?.setIsMiniaturized(isMiniaturized) + if let droppingPoint { mainWindowController.window?.setFrameOrigin(droppingPoint: droppingPoint) From 5be1c5af53c9b01a27099170c08d82fdd016f04b Mon Sep 17 00:00:00 2001 From: Halle <378795+Halle@users.noreply.github.com> Date: Tue, 9 Apr 2024 06:25:10 -0700 Subject: [PATCH 2/4] Adds a series of UI tests for Bookmarks and Favorites Task/Issue URL: https://app.asana.com/0/1199230911884351/1205717021705367/f Tech Design URL: CC: **Description**: Adds a series of UI tests for Bookmarks and Favorites **Steps to test this PR**: 1. Open the scheme **UI Tests** 2. Navigate to the test pane 3. Run BookmarksAndFavoritesTests **Note**: If this builds a `review` build that is treated as a first run on your system during every test (for instance, asking you where to put the application), please **build and run** the scheme once instead of running its tests, and go through the new app install first run steps once before running the tests, and answer any first install questions. **UI Tests general guidelines for everyone**: it unfortunately isn't possible to multitask while running UI tests on your local machine, because you will change or intercept screen interactions that the tests depend on. If you experience failures in your local environment, please always send the `xcresult` bundle so it is possible to view what happened differently on your machine, even if it seems like the same failure as a previous failure (it may be subtly different). Since the `xcresult` bundle will include a screen recording, for your privacy, please consider things like your chats, calls, and open documents that you don't want to send in a screen recording when you are running the tests. Thank you very much for your help! --- DuckDuckGo.xcodeproj/project.pbxproj | 4 + .../Bookmarks/Services/ContextualMenu.swift | 5 + ...kmarkManagementSidebarViewController.swift | 1 + .../View/BookmarkOutlineCellView.swift | 1 + .../View/BookmarkTableCellView.swift | 4 + .../Extensions/NSMenuItemExtension.swift | 5 + DuckDuckGo/HomePage/View/FavoritesView.swift | 16 +- DuckDuckGo/Menus/MainMenu.swift | 3 +- .../AddressBarButtonsViewController.swift | 7 +- .../NavigationBar/View/MoreOptionsMenu.swift | 3 +- .../View/PreferencesAppearanceView.swift | 2 +- .../TabExtensions/ContextMenuManager.swift | 2 +- UITests/AutocompleteTests.swift | 1 - UITests/BookmarksAndFavoritesTests.swift | 724 ++++++++++++++++++ UITests/BookmarksBarTests.swift | 1 - UITests/Common/UITests.swift | 2 +- UITests/Common/XCUIElementExtension.swift | 16 + 17 files changed, 780 insertions(+), 17 deletions(-) create mode 100644 UITests/BookmarksAndFavoritesTests.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 2bbc99a10b..dfa32b6122 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -3291,6 +3291,7 @@ EE339228291BDEFD009F62C1 /* JSAlertController.swift in Sources */ = {isa = PBXBuildFile; fileRef = EE339227291BDEFD009F62C1 /* JSAlertController.swift */; }; EE3424602BA0853900173B1B /* VPNUninstaller.swift in Sources */ = {isa = PBXBuildFile; fileRef = EE34245D2BA0853900173B1B /* VPNUninstaller.swift */; }; EE3424612BA0853900173B1B /* VPNUninstaller.swift in Sources */ = {isa = PBXBuildFile; fileRef = EE34245D2BA0853900173B1B /* VPNUninstaller.swift */; }; + EE54F7B32BBFEA49006218DB /* BookmarksAndFavoritesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EE54F7B22BBFEA48006218DB /* BookmarksAndFavoritesTests.swift */; }; EE66418C2B9B1981005BCD17 /* NetworkProtectionTokenStore+SubscriptionTokenKeychainStorage.swift in Sources */ = {isa = PBXBuildFile; fileRef = EE66418B2B9B1981005BCD17 /* NetworkProtectionTokenStore+SubscriptionTokenKeychainStorage.swift */; }; EE66418D2B9B1981005BCD17 /* NetworkProtectionTokenStore+SubscriptionTokenKeychainStorage.swift in Sources */ = {isa = PBXBuildFile; fileRef = EE66418B2B9B1981005BCD17 /* NetworkProtectionTokenStore+SubscriptionTokenKeychainStorage.swift */; }; EE66666F2B56EDE4001D898D /* VPNLocationsHostingViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = EE66666E2B56EDE4001D898D /* VPNLocationsHostingViewController.swift */; }; @@ -4772,6 +4773,7 @@ EE0429DF2BA31D2F009EB20F /* FindInPageTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FindInPageTests.swift; sourceTree = ""; }; EE339227291BDEFD009F62C1 /* JSAlertController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = JSAlertController.swift; sourceTree = ""; }; EE34245D2BA0853900173B1B /* VPNUninstaller.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VPNUninstaller.swift; sourceTree = ""; }; + EE54F7B22BBFEA48006218DB /* BookmarksAndFavoritesTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BookmarksAndFavoritesTests.swift; sourceTree = ""; }; EE66418B2B9B1981005BCD17 /* NetworkProtectionTokenStore+SubscriptionTokenKeychainStorage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "NetworkProtectionTokenStore+SubscriptionTokenKeychainStorage.swift"; sourceTree = ""; }; EE66666E2B56EDE4001D898D /* VPNLocationsHostingViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VPNLocationsHostingViewController.swift; sourceTree = ""; }; EE7F74902BB5D76600CD9456 /* BookmarksBarTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BookmarksBarTests.swift; sourceTree = ""; }; @@ -6556,6 +6558,7 @@ children = ( EEBCE6802BA444FA00B9DF00 /* Common */, EED735352BB46B6000F173D6 /* AutocompleteTests.swift */, + EE54F7B22BBFEA48006218DB /* BookmarksAndFavoritesTests.swift */, EE7F74902BB5D76600CD9456 /* BookmarksBarTests.swift */, EE02D41B2BB460A600DBE6B3 /* BrowsingHistoryTests.swift */, EE0429DF2BA31D2F009EB20F /* FindInPageTests.swift */, @@ -12279,6 +12282,7 @@ EE02D41A2BB4609900DBE6B3 /* UITests.swift in Sources */, EE0429E02BA31D2F009EB20F /* FindInPageTests.swift in Sources */, EE02D4212BB460FE00DBE6B3 /* StringExtension.swift in Sources */, + EE54F7B32BBFEA49006218DB /* BookmarksAndFavoritesTests.swift in Sources */, EE02D4222BB4611A00DBE6B3 /* TestsURLExtension.swift in Sources */, 7B4CE8E726F02135009134B1 /* TabBarTests.swift in Sources */, EEBCE6832BA463DD00B9DF00 /* NSImageExtensions.swift in Sources */, diff --git a/DuckDuckGo/Bookmarks/Services/ContextualMenu.swift b/DuckDuckGo/Bookmarks/Services/ContextualMenu.swift index f79e265d96..eb77fd956f 100644 --- a/DuckDuckGo/Bookmarks/Services/ContextualMenu.swift +++ b/DuckDuckGo/Bookmarks/Services/ContextualMenu.swift @@ -163,11 +163,15 @@ private extension ContextualMenu { static func addBookmarkToFavoritesMenuItem(isFavorite: Bool, bookmark: Bookmark?) -> NSMenuItem { let title = isFavorite ? UserText.removeFromFavorites : UserText.addToFavorites return menuItem(title, #selector(BookmarkMenuItemSelectors.toggleBookmarkAsFavorite(_:)), bookmark) + .withAccessibilityIdentifier(isFavorite == false ? "ContextualMenu.addBookmarkToFavoritesMenuItem" : + "ContextualMenu.removeBookmarkFromFavoritesMenuItem") } static func addBookmarksToFavoritesMenuItem(bookmarks: [Bookmark], allFavorites: Bool) -> NSMenuItem { let title = allFavorites ? UserText.removeFromFavorites : UserText.addToFavorites + let accessibilityValue = allFavorites ? "Favorited" : "Unfavorited" return menuItem(title, #selector(BookmarkMenuItemSelectors.toggleBookmarkAsFavorite(_:)), bookmarks) + .withAccessibilityIdentifier("ContextualMenu.addBookmarksToFavoritesMenuItem").withAccessibilityValue(accessibilityValue) } static func editBookmarkMenuItem(bookmark: Bookmark?) -> NSMenuItem { @@ -180,6 +184,7 @@ private extension ContextualMenu { static func deleteBookmarkMenuItem(bookmark: Bookmark?) -> NSMenuItem { menuItem(UserText.bookmarksBarContextMenuDelete, #selector(BookmarkMenuItemSelectors.deleteBookmark(_:)), bookmark) + .withAccessibilityIdentifier("ContextualMenu.deleteBookmark") } static func moveToEndMenuItem(entity: BaseBookmarkEntity?, parent: BookmarkFolder?) -> NSMenuItem { diff --git a/DuckDuckGo/Bookmarks/View/BookmarkManagementSidebarViewController.swift b/DuckDuckGo/Bookmarks/View/BookmarkManagementSidebarViewController.swift index 53e502383b..f567a9f124 100644 --- a/DuckDuckGo/Bookmarks/View/BookmarkManagementSidebarViewController.swift +++ b/DuckDuckGo/Bookmarks/View/BookmarkManagementSidebarViewController.swift @@ -89,6 +89,7 @@ final class BookmarkManagementSidebarViewController: NSViewController { tabSwitcherButton.menu = NSMenu { for content in Tab.TabContent.displayableTabTypes { NSMenuItem(title: content.title!, representedObject: content) + .withAccessibilityIdentifier("BookmarkManagementSidebarViewController.\(content.title!)") } } diff --git a/DuckDuckGo/Bookmarks/View/BookmarkOutlineCellView.swift b/DuckDuckGo/Bookmarks/View/BookmarkOutlineCellView.swift index 603849bbbf..06b868c826 100644 --- a/DuckDuckGo/Bookmarks/View/BookmarkOutlineCellView.swift +++ b/DuckDuckGo/Bookmarks/View/BookmarkOutlineCellView.swift @@ -82,6 +82,7 @@ final class BookmarkOutlineCellView: NSTableCellView { faviconImageView.imageScaling = .scaleProportionallyDown faviconImageView.wantsLayer = true faviconImageView.layer?.cornerRadius = 2.0 + faviconImageView.setAccessibilityIdentifier("BookmarkOutlineCellView.favIconImageView") titleLabel.translatesAutoresizingMaskIntoConstraints = false titleLabel.isEditable = false diff --git a/DuckDuckGo/Bookmarks/View/BookmarkTableCellView.swift b/DuckDuckGo/Bookmarks/View/BookmarkTableCellView.swift index 8bfdc3c4fb..3a3d3ea0d9 100644 --- a/DuckDuckGo/Bookmarks/View/BookmarkTableCellView.swift +++ b/DuckDuckGo/Bookmarks/View/BookmarkTableCellView.swift @@ -129,6 +129,7 @@ final class BookmarkTableCellView: NSTableCellView { menuButton.translatesAutoresizingMaskIntoConstraints = false menuButton.isBordered = false menuButton.isHidden = true + menuButton.setAccessibilityIdentifier("BookmarkTableCellView.menuButton") } private func setupLayout() { @@ -209,11 +210,14 @@ final class BookmarkTableCellView: NSTableCellView { faviconImageView.image = bookmark.favicon(.small) ?? .bookmarkDefaultFavicon + faviconImageView.setAccessibilityIdentifier("BookmarkTableCellView.favIconImageView") if bookmark.isFavorite { accessoryImageView.isHidden = false } accessoryImageView.image = bookmark.isFavorite ? .favoriteFilledBorder : nil + accessoryImageView.setAccessibilityIdentifier("BookmarkTableCellView.accessoryImageView") + accessoryImageView.setAccessibilityValue(bookmark.isFavorite ? "Favorited" : "Unfavorited") titleLabel.stringValue = bookmark.title primaryTitleLabelValue = bookmark.title tertiaryTitleLabelValue = bookmark.url diff --git a/DuckDuckGo/Common/Extensions/NSMenuItemExtension.swift b/DuckDuckGo/Common/Extensions/NSMenuItemExtension.swift index b3f0870252..dfe76b5092 100644 --- a/DuckDuckGo/Common/Extensions/NSMenuItemExtension.swift +++ b/DuckDuckGo/Common/Extensions/NSMenuItemExtension.swift @@ -110,6 +110,11 @@ extension NSMenuItem { return self } + func withAccessibilityValue(_ accessibilityValue: String) -> NSMenuItem { + self.setAccessibilityValue(accessibilityValue) + return self + } + @discardableResult func withImage(_ image: NSImage?) -> NSMenuItem { self.image = image diff --git a/DuckDuckGo/HomePage/View/FavoritesView.swift b/DuckDuckGo/HomePage/View/FavoritesView.swift index 52e7f585d1..39f74b58cf 100644 --- a/DuckDuckGo/HomePage/View/FavoritesView.swift +++ b/DuckDuckGo/HomePage/View/FavoritesView.swift @@ -324,12 +324,12 @@ struct Favorite: View { .link { model.open(bookmark) }.contextMenu(ContextMenu(menuItems: { - Button(UserText.openInNewTab, action: { model.openInNewTab(bookmark) }) - Button(UserText.openInNewWindow, action: { model.openInNewWindow(bookmark) }) + Button(UserText.openInNewTab, action: { model.openInNewTab(bookmark) }).accessibilityIdentifier("HomePage.Views.openInNewTab") + Button(UserText.openInNewWindow, action: { model.openInNewWindow(bookmark) }).accessibilityIdentifier("HomePage.Views.openInNewWindow") Divider() - Button(UserText.edit, action: { model.edit(bookmark) }) - Button(UserText.removeFavorite, action: { model.removeFavorite(bookmark) }) - Button(UserText.deleteBookmark, action: { model.deleteBookmark(bookmark) }) + Button(UserText.edit, action: { model.edit(bookmark) }).accessibilityIdentifier("HomePage.Views.editBookmark") + Button(UserText.removeFavorite, action: { model.removeFavorite(bookmark) }).accessibilityIdentifier("HomePage.Views.removeFavorite") + Button(UserText.deleteBookmark, action: { model.deleteBookmark(bookmark) }).accessibilityIdentifier("HomePage.Views.deleteBookmark") })) } @@ -344,13 +344,13 @@ extension HomePage.Models.FavoriteModel { var favoriteView: some View { switch favoriteType { case .bookmark(let bookmark): - HomePage.Views.Favorite(bookmark: bookmark) + HomePage.Views.Favorite(bookmark: bookmark)?.accessibilityIdentifier("HomePage.Models.FavoriteModel.\(bookmark.title)") case .addButton: - HomePage.Views.FavoritesGridAddButton() + HomePage.Views.FavoritesGridAddButton().accessibilityIdentifier("HomePage.Models.FavoriteModel.addButton") case .ghostButton: - HomePage.Views.FavoritesGridGhostButton() + HomePage.Views.FavoritesGridGhostButton().accessibilityIdentifier("HomePage.Models.FavoriteModel.ghostButton") } } } diff --git a/DuckDuckGo/Menus/MainMenu.swift b/DuckDuckGo/Menus/MainMenu.swift index 31d25d3d31..d5bdd2d495 100644 --- a/DuckDuckGo/Menus/MainMenu.swift +++ b/DuckDuckGo/Menus/MainMenu.swift @@ -69,7 +69,7 @@ import SubscriptionUI var forwardMenuItem: NSMenuItem { historyMenu.forwardMenuItem } // MARK: Bookmarks - let manageBookmarksMenuItem = NSMenuItem(title: UserText.mainMenuHistoryManageBookmarks, action: #selector(MainViewController.showManageBookmarks)) + let manageBookmarksMenuItem = NSMenuItem(title: UserText.mainMenuHistoryManageBookmarks, action: #selector(MainViewController.showManageBookmarks)).withAccessibilityIdentifier("MainMenu.manageBookmarksMenuItem") var bookmarksMenuToggleBookmarksBarMenuItem = NSMenuItem(title: "BookmarksBarMenuPlaceholder", action: #selector(MainViewController.toggleBookmarksBarFromMenu), keyEquivalent: "B") let importBookmarksMenuItem = NSMenuItem(title: UserText.importBookmarks, action: #selector(AppDelegate.openImportBrowserDataWindow)) let bookmarksMenu = NSMenu(title: UserText.bookmarks) @@ -306,6 +306,7 @@ import SubscriptionUI .submenu(favoritesMenu.buildItems { NSMenuItem(title: UserText.mainMenuHistoryFavoriteThisPage, action: #selector(MainViewController.favoriteThisPage)) .withImage(.favorite) + .withAccessibilityIdentifier("MainMenu.favoriteThisPage") NSMenuItem.separator() }) .withImage(.favorite) diff --git a/DuckDuckGo/NavigationBar/View/AddressBarButtonsViewController.swift b/DuckDuckGo/NavigationBar/View/AddressBarButtonsViewController.swift index 7afc044118..7bb06c097f 100644 --- a/DuckDuckGo/NavigationBar/View/AddressBarButtonsViewController.swift +++ b/DuckDuckGo/NavigationBar/View/AddressBarButtonsViewController.swift @@ -272,7 +272,7 @@ final class AddressBarButtonsViewController: NSViewController { private func updateBookmarkButtonVisibility() { guard view.window?.isPopUpWindow == false else { return } - bookmarkButton.setAccessibilityIdentifier("Bookmarks Button") + bookmarkButton.setAccessibilityIdentifier("AddressBarButtonsViewController.bookmarkButton") let hasEmptyAddressBar = textFieldValue?.isEmpty ?? true var showBookmarkButton: Bool { guard let tabViewModel, tabViewModel.canBeBookmarked else { return false } @@ -727,15 +727,18 @@ final class AddressBarButtonsViewController: NSViewController { private func updateBookmarkButtonImage(isUrlBookmarked: Bool = false) { if let url = tabViewModel?.tab.content.url, - isUrlBookmarked || bookmarkManager.isUrlBookmarked(url: url) { + isUrlBookmarked || bookmarkManager.isUrlBookmarked(url: url) + { bookmarkButton.image = .bookmarkFilled bookmarkButton.mouseOverTintColor = NSColor.bookmarkFilledTint bookmarkButton.toolTip = UserText.editBookmarkTooltip + bookmarkButton.setAccessibilityValue("Bookmarked") } else { bookmarkButton.mouseOverTintColor = nil bookmarkButton.image = .bookmark bookmarkButton.contentTintColor = nil bookmarkButton.toolTip = UserText.addBookmarkTooltip + bookmarkButton.setAccessibilityValue("Unbookmarked") } } diff --git a/DuckDuckGo/NavigationBar/View/MoreOptionsMenu.swift b/DuckDuckGo/NavigationBar/View/MoreOptionsMenu.swift index ed8a2e2b76..e784fccf14 100644 --- a/DuckDuckGo/NavigationBar/View/MoreOptionsMenu.swift +++ b/DuckDuckGo/NavigationBar/View/MoreOptionsMenu.swift @@ -270,7 +270,7 @@ final class MoreOptionsMenu: NSMenu { .targetting(self) .withImage(.bookmarks) .withSubmenu(bookmarksSubMenu) - + .withAccessibilityIdentifier("MoreOptionsMenu.openBookmarks") addItem(withTitle: UserText.downloads, action: #selector(openDownloads), keyEquivalent: "j") .targetting(self) .withImage(.downloads) @@ -641,6 +641,7 @@ final class BookmarksSubMenu: NSMenu { let bookmarkPageItem = addItem(withTitle: UserText.bookmarkThisPage, action: #selector(MoreOptionsMenu.bookmarkPage(_:)), keyEquivalent: "d") .withModifierMask([.command]) .targetting(target) + .withAccessibilityIdentifier("MoreOptionsMenu.bookmarkPage") bookmarkPageItem.isEnabled = tabCollectionViewModel.selectedTabViewModel?.canBeBookmarked == true diff --git a/DuckDuckGo/Preferences/View/PreferencesAppearanceView.swift b/DuckDuckGo/Preferences/View/PreferencesAppearanceView.swift index a7b30dae90..c44ff7c3bd 100644 --- a/DuckDuckGo/Preferences/View/PreferencesAppearanceView.swift +++ b/DuckDuckGo/Preferences/View/PreferencesAppearanceView.swift @@ -106,7 +106,7 @@ extension Preferences { if model.isContinueSetUpAvailable { ToggleMenuItem(UserText.newTabSetUpSectionTitle, isOn: $model.isContinueSetUpVisible) } - ToggleMenuItem(UserText.newTabFavoriteSectionTitle, isOn: $model.isFavoriteVisible) + ToggleMenuItem(UserText.newTabFavoriteSectionTitle, isOn: $model.isFavoriteVisible).accessibilityIdentifier("Preferences.AppearanceView.showFavoritesToggle") ToggleMenuItem(UserText.newTabRecentActivitySectionTitle, isOn: $model.isRecentActivityVisible) } diff --git a/DuckDuckGo/Tab/TabExtensions/ContextMenuManager.swift b/DuckDuckGo/Tab/TabExtensions/ContextMenuManager.swift index 787ac89300..e966c8b7a0 100644 --- a/DuckDuckGo/Tab/TabExtensions/ContextMenuManager.swift +++ b/DuckDuckGo/Tab/TabExtensions/ContextMenuManager.swift @@ -234,7 +234,7 @@ private extension ContextMenuManager { } func bookmarkPageMenuItem() -> NSMenuItem { - NSMenuItem(title: UserText.bookmarkPage, action: #selector(MainViewController.bookmarkThisPage), target: nil, keyEquivalent: "") + NSMenuItem(title: UserText.bookmarkPage, action: #selector(MainViewController.bookmarkThisPage), target: nil, keyEquivalent: "").withAccessibilityIdentifier("ContextMenuManager.bookmarkPageMenuItem") } func openLinkInNewWindowMenuItem(from item: NSMenuItem) -> NSMenuItem { diff --git a/UITests/AutocompleteTests.swift b/UITests/AutocompleteTests.swift index 2ef08b7e8c..4c730f0f80 100644 --- a/UITests/AutocompleteTests.swift +++ b/UITests/AutocompleteTests.swift @@ -16,7 +16,6 @@ // limitations under the License. // -import Common import XCTest class AutocompleteTests: XCTestCase { diff --git a/UITests/BookmarksAndFavoritesTests.swift b/UITests/BookmarksAndFavoritesTests.swift new file mode 100644 index 0000000000..bf06b4aac6 --- /dev/null +++ b/UITests/BookmarksAndFavoritesTests.swift @@ -0,0 +1,724 @@ +// +// BookmarksAndFavoritesTests.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import XCTest + +class BookmarksAndFavoritesTests: XCTestCase { + private var app: XCUIApplication! + private var pageTitle: String! + private var urlForBookmarksBar: URL! + private let titleStringLength = 12 + + private var addressBarBookmarkButton: XCUIElement! + private var addressBarTextField: XCUIElement! + private var bookmarkDialogBookmarkFolderDropdown: XCUIElement! + private var bookmarkPageContextMenuItem: XCUIElement! + private var bookmarkPageMenuItem: XCUIElement! + private var bookmarksBarCollectionView: XCUIElement! + private var bookmarksDialogAddToFavoritesCheckbox: XCUIElement! + private var bookmarksManagementAccessoryImageView: XCUIElement! + private var bookmarksMenu: XCUIElement! + private var bookmarksTabPopup: XCUIElement! + private var bookmarkTableCellViewFavIconImageView: XCUIElement! + private var bookmarkTableCellViewMenuButton: XCUIElement! + private var contextualMenuAddBookmarkToFavoritesMenuItem: XCUIElement! + private var contextualMenuDeleteBookmarkMenuItem: XCUIElement! + private var contextualMenuRemoveBookmarkFromFavoritesMenuItem: XCUIElement! + private var defaultBookmarkDialogButton: XCUIElement! + private var defaultBookmarkOtherButton: XCUIElement! + private var favoriteGridAddFavoriteButton: XCUIElement! + private var favoriteThisPageMenuItem: XCUIElement! + private var manageBookmarksMenuItem: XCUIElement! + private var openBookmarksMenuItem: XCUIElement! + private var optionsButton: XCUIElement! + private var removeFavoritesContextMenuItem: XCUIElement! + private var resetBookMarksMenuItem: XCUIElement! + private var settingsAppearanceButton: XCUIElement! + private var showBookmarksBarPreferenceToggle: XCUIElement! + private var showBookmarksBarAlways: XCUIElement! + private var showBookmarksBarPopup: XCUIElement! + private var showFavoritesPreferenceToggle: XCUIElement! + + override func setUpWithError() throws { + continueAfterFailure = false + app = XCUIApplication() + app.launchEnvironment["UITEST_MODE"] = "1" + pageTitle = UITests.randomPageTitle(length: titleStringLength) + urlForBookmarksBar = UITests.simpleServedPage(titled: pageTitle) + addressBarBookmarkButton = app.buttons["AddressBarButtonsViewController.bookmarkButton"] + addressBarTextField = app.windows.textFields["AddressBarViewController.addressBarTextField"] + bookmarkDialogBookmarkFolderDropdown = app.popUpButtons["bookmark.add.folder.dropdown"] + bookmarkPageContextMenuItem = app.menuItems["ContextMenuManager.bookmarkPageMenuItem"] + bookmarkPageMenuItem = app.menuItems["MoreOptionsMenu.bookmarkPage"] + bookmarksBarCollectionView = app.collectionViews["BookmarksBarViewController.bookmarksBarCollectionView"] + bookmarksDialogAddToFavoritesCheckbox = app.checkBoxes["bookmark.add.add.to.favorites.button"] + bookmarksManagementAccessoryImageView = app.images["BookmarkTableCellView.accessoryImageView"] + bookmarksMenu = app.menuBarItems["Bookmarks"] + bookmarksTabPopup = app.popUpButtons["Bookmarks"] + bookmarkTableCellViewFavIconImageView = app.images["BookmarkTableCellView.favIconImageView"] + bookmarkTableCellViewMenuButton = app.buttons["BookmarkTableCellView.menuButton"] + contextualMenuAddBookmarkToFavoritesMenuItem = app.menuItems["ContextualMenu.addBookmarkToFavoritesMenuItem"] + contextualMenuDeleteBookmarkMenuItem = app.menuItems["ContextualMenu.deleteBookmark"] + contextualMenuRemoveBookmarkFromFavoritesMenuItem = app.menuItems["ContextualMenu.removeBookmarkFromFavoritesMenuItem"] + defaultBookmarkDialogButton = app.buttons["BookmarkDialogButtonsView.defaultButton"] + defaultBookmarkOtherButton = app.buttons["BookmarkDialogButtonsView.otherButton"] + favoriteGridAddFavoriteButton = app.staticTexts["HomePage.Models.FavoriteModel.addButton"] + favoriteThisPageMenuItem = app.menuItems["MainMenu.favoriteThisPage"] + manageBookmarksMenuItem = app.menuItems["MainMenu.manageBookmarksMenuItem"] + openBookmarksMenuItem = app.menuItems["MoreOptionsMenu.openBookmarks"] + optionsButton = app.buttons["NavigationBarViewController.optionsButton"] + removeFavoritesContextMenuItem = app.menuItems["HomePage.Views.removeFavorite"] + resetBookMarksMenuItem = app.menuItems["MainMenu.resetBookmarks"] + settingsAppearanceButton = app.buttons["PreferencesSidebar.appearanceButton"] + showBookmarksBarAlways = app.menuItems["Preferences.AppearanceView.showBookmarksBarAlways"] + showBookmarksBarPopup = app.popUpButtons["Preferences.AppearanceView.showBookmarksBarPopUp"] + showBookmarksBarPreferenceToggle = app.checkBoxes["Preferences.AppearanceView.showBookmarksBarPreferenceToggle"] + showFavoritesPreferenceToggle = app.checkBoxes["Preferences.AppearanceView.showFavoritesToggle"] + + app.launch() + resetBookmarks() + app.typeKey("w", modifierFlags: [.command, .option, .shift]) // Let's enforce a single window + app.typeKey("n", modifierFlags: .command) + } + + func test_bookmarks_canBeAddedTo_withContextClickBookmarkThisPage() { + openSiteToBookmark(bookmarkingViaDialog: false, escapingDialog: false) + app.windows.webViews[pageTitle].rightClick() + bookmarkPageContextMenuItem.clickAfterExistenceTestSucceeds() + XCTAssertTrue( // Check Add Bookmark dialog for existence but don't click on it + defaultBookmarkDialogButton.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The \"Add bookmark\" dialog option button didn't appear with the expected title in a reasonable timeframe." + ) + XCTAssertTrue( + addressBarBookmarkButton.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The address bar bookmark button didn't appear with the expected title in a reasonable timeframe." + ) + XCTAssertTrue( + bookmarkDialogBookmarkFolderDropdown.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The \"Add bookmark\" dialog's bookmark folder dropdown didn't appear with the expected title in a reasonable timeframe." + ) + let bookmarkDialogBookmarkFolderDropdownValue = try? XCTUnwrap( // Bookmark dialog must default to "Bookmarks" folder + bookmarkDialogBookmarkFolderDropdown.value as? String, + "It wasn't possible to get the value of the \"Add bookmark\" dialog's bookmark folder dropdown as String" + ) + XCTAssertEqual( + bookmarkDialogBookmarkFolderDropdownValue, + "Bookmarks", + "The accessibility value of the \"Add bookmark\" dialog's bookmark folder dropdown must be \"Bookmarks\"." + ) + let addressBarBookmarkButtonValue = try? XCTUnwrap( + addressBarBookmarkButton.value as? String, + "It wasn't possible to get the value of the address bar bookmark button as String" + ) + + XCTAssertEqual( // The bookmark icon is already in a filled state and it isn't necessary to click the add button + addressBarBookmarkButtonValue, + "Bookmarked", + "The accessibility value of the address bar bookmark button must be \"Bookmarked\", which indicates the icon in the filled state." + ) + + bookmarksMenu.clickAfterExistenceTestSucceeds() + XCTAssertTrue( // And the bookmark is found in the Bookmarks menu + app.menuItems[pageTitle].waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The bookmark in the \"Bookmarks\" menu with the title of the test page didn't appear with the expected title in a reasonable timeframe." + ) + } + + func test_bookmarks_canBeAddedTo_withSettingsItemBookmarkThisPage() { + openSiteToBookmark(bookmarkingViaDialog: false, escapingDialog: false) + optionsButton.clickAfterExistenceTestSucceeds() + openBookmarksMenuItem.hoverAfterExistenceTestSucceeds() + bookmarkPageMenuItem.clickAfterExistenceTestSucceeds() + XCTAssertTrue( // Check Add Bookmark dialog for existence but don't click on it + defaultBookmarkDialogButton.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The \"Add bookmark\" dialog option button didn't appear with the expected title in a reasonable timeframe." + ) + XCTAssertTrue( + addressBarBookmarkButton.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The address bar bookmark button didn't appear with the expected title in a reasonable timeframe." + ) + XCTAssertTrue( + bookmarkDialogBookmarkFolderDropdown.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The \"Add bookmark\" dialog's bookmark folder dropdown didn't appear with the expected title in a reasonable timeframe." + ) + let bookmarkDialogBookmarkFolderDropdownValue = try? XCTUnwrap( + bookmarkDialogBookmarkFolderDropdown.value as? String, + "It wasn't possible to get the value of the \"Add bookmark\" dialog's bookmark folder dropdown as String" + ) + XCTAssertEqual( // Bookmark dialog must default to "Bookmarks" folder + bookmarkDialogBookmarkFolderDropdownValue, + "Bookmarks", + "The accessibility value of the \"Add bookmark\" dialog's bookmark folder dropdown must be \"Bookmarks\"." + ) + + let addressBarBookmarkButtonValue = try? XCTUnwrap( + addressBarBookmarkButton.value as? String, + "It wasn't possible to get the value of the address bar bookmark button as String" + ) + XCTAssertEqual( // The bookmark icon is already in a filled state and it isn't necessary to click the add button + addressBarBookmarkButtonValue, + "Bookmarked", + "The accessibility value of the address bar bookmark button must be \"Bookmarked\", which indicates the icon in the filled state." + ) + + bookmarksMenu.clickAfterExistenceTestSucceeds() + XCTAssertTrue( // And the bookmark is found in the Bookmarks menu + app.menuItems[pageTitle].waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The bookmark in the \"Bookmarks\" menu with the title of the test page didn't appear with the expected title in a reasonable timeframe." + ) + } + + func test_bookmarks_canBeAddedTo_byClickingBookmarksButtonInAddressBar() { + openSiteToBookmark(bookmarkingViaDialog: false, escapingDialog: false) + // In order to directly click the bookmark button in the address bar, we need to hover over something in the bar area + optionsButton.hoverAfterExistenceTestSucceeds() + addressBarBookmarkButton.clickAfterExistenceTestSucceeds() + XCTAssertTrue( // Check Add Bookmark dialog for existence but don't click on it + defaultBookmarkDialogButton.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The \"Add bookmark\" dialog option button didn't appear with the expected title in a reasonable timeframe." + ) + XCTAssertTrue( + bookmarkDialogBookmarkFolderDropdown.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The \"Add bookmark\" dialog's bookmark folder dropdown didn't appear with the expected title in a reasonable timeframe." + ) + let bookmarkDialogBookmarkFolderDropdownValue = try? XCTUnwrap( + bookmarkDialogBookmarkFolderDropdown.value as? String, + "It wasn't possible to get the value of the \"Add bookmark\" dialog's bookmark folder dropdown as String" + ) + + XCTAssertEqual( // Bookmark dialog must default to "Bookmarks" folder + bookmarkDialogBookmarkFolderDropdownValue, + "Bookmarks", + "The accessibility value of the \"Add bookmark\" dialog's bookmark folder dropdown must be \"Bookmarks\"." + ) + let addressBarBookmarkButtonValue = try? XCTUnwrap( + addressBarBookmarkButton.value as? String, + "It wasn't possible to get the value of the address bar bookmark button as String" + ) + XCTAssertEqual( // The bookmark icon is already in a filled state and it isn't necessary to click the add button + addressBarBookmarkButtonValue, + "Bookmarked", + "The accessibility value of the address bar bookmark button must be \"Bookmarked\", which indicates the icon in the filled state." + ) + + bookmarksMenu.clickAfterExistenceTestSucceeds() + XCTAssertTrue( // And the bookmark is found in the Bookmarks menu + app.menuItems[pageTitle].waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The bookmark in the \"Bookmarks\" menu with the title of the test page didn't appear with the expected title in a reasonable timeframe." + ) + } + + func test_favorites_canBeAddedTo_byClickingFavoriteThisPageMenuBarItem() { + openSiteToBookmark(bookmarkingViaDialog: false, escapingDialog: false) + bookmarksMenu.clickAfterExistenceTestSucceeds() + favoriteThisPageMenuItem.clickAfterExistenceTestSucceeds() + + XCTAssertTrue( // Check Add Bookmark dialog for existence but don't click on it + defaultBookmarkDialogButton.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The \"Add bookmark\" dialog option button didn't appear with the expected title in a reasonable timeframe." + ) + let addressBarBookmarkButtonValue = try? XCTUnwrap( + addressBarBookmarkButton.value as? String, + "It wasn't possible to get the value of the address bar bookmark button as String" + ) + XCTAssertEqual( // The bookmark icon is already in a filled state and it isn't necessary to click the add button + addressBarBookmarkButtonValue, + "Bookmarked", + "The accessibility value of the address bar bookmark button must be \"Bookmarked\", which indicates the icon in the filled state." + ) + XCTAssertTrue( // Check Add Bookmark dialog for existence but don't click on it + defaultBookmarkDialogButton.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The \"Add bookmark\" dialog option button didn't appear with the expected title in a reasonable timeframe." + ) + XCTAssertTrue( + bookmarksDialogAddToFavoritesCheckbox.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The add to favorites checkbox in the add bookmark dialog didn't appear with the expected title in a reasonable timeframe." + ) + + let bookmarksDialogAddToFavoritesCheckboxValue = try? XCTUnwrap( + bookmarksDialogAddToFavoritesCheckbox.value as? Bool, + "It wasn't possible to get the value of the bookmarks dialog's add to favorites checkbox as Bool" + ) + XCTAssertEqual( // The favorite checkbox in the dialog is already checked + bookmarksDialogAddToFavoritesCheckboxValue, + true, + "The the value of the bookmarks dialog's add to favorites checkbox must be checked, which indicates that the item has been favorited." + ) + } + + func test_favorites_canBeAddedTo_byClickingAddFavoriteInAddBookmarkPopover() { + openSiteToBookmark(bookmarkingViaDialog: false, escapingDialog: false) + // In order to directly click the bookmark button in the address bar, we need to hover over something in the bar area + optionsButton.hoverAfterExistenceTestSucceeds() + + addressBarBookmarkButton.clickAfterExistenceTestSucceeds() + XCTAssertTrue( // Check Add Bookmark dialog for existence before adding to favorites + defaultBookmarkDialogButton.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The \"Add bookmark\" dialog option button didn't appear with the expected title in a reasonable timeframe." + ) + + bookmarksDialogAddToFavoritesCheckbox.clickAfterExistenceTestSucceeds() + let bookmarksDialogAddToFavoritesCheckboxValue = try? XCTUnwrap( + bookmarksDialogAddToFavoritesCheckbox.value as? Bool, + "It wasn't possible to get the value of the bookmarks dialog's add to favorites checkbox as Bool" + ) + XCTAssertEqual( // The favorite checkbox in the dialog is already checked + bookmarksDialogAddToFavoritesCheckboxValue, + true, + "The the value of the bookmarks dialog's add to favorites checkbox must be checked, which indicates that the item has been favorited." + ) + } + + func test_favorites_canBeManuallyAddedTo_byClickingAddFavoriteInNewTabPage() throws { + toggleBookmarksBarShowFavoritesOn() + + favoriteGridAddFavoriteButton.clickAfterExistenceTestSucceeds() + let pageTitleForAddFavoriteDialog: String = try XCTUnwrap(pageTitle, "Couldn't unwrap page title") + let urlForAddFavoriteDialog = try XCTUnwrap(urlForBookmarksBar, "Couldn't unwrap page url") + app.typeText("\(pageTitleForAddFavoriteDialog)\t") + app.typeURL(urlForAddFavoriteDialog) + let newFavorite = app.otherElements.staticTexts[pageTitleForAddFavoriteDialog] + + XCTAssertTrue( + newFavorite.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The new favorite on the new tab page did not become available in a reasonable timeframe." + ) + } + + func test_favorites_canBeAddedToFromManageBookmarksView() { + openSiteToBookmark(bookmarkingViaDialog: true, escapingDialog: true) + bookmarksMenu.clickAfterExistenceTestSucceeds() + manageBookmarksMenuItem.clickAfterExistenceTestSucceeds() + bookmarkTableCellViewFavIconImageView.hoverAfterExistenceTestSucceeds() + bookmarkTableCellViewMenuButton.clickAfterExistenceTestSucceeds() + + contextualMenuAddBookmarkToFavoritesMenuItem.clickAfterExistenceTestSucceeds() + XCTAssertTrue( + bookmarksManagementAccessoryImageView.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "Bookmarks accessory view favorites indicator didn't load with the expected title in a reasonable timeframe." + ) + let bookmarksManagementAccessoryImageViewValue = try? XCTUnwrap( + bookmarksManagementAccessoryImageView.value as? String, + "It wasn't possible to get the value of the bookmarks management accessory image view as String" + ) + + XCTAssertEqual( + bookmarksManagementAccessoryImageViewValue, + "Favorited", + "The accessibility value of the favorite accessory view on the bookmark management view must be \"Favorited\"." + ) + } + + func test_bookmarks_canBeViewedInBookmarkMenuItem() { + openSiteToBookmark(bookmarkingViaDialog: true, escapingDialog: true) + addressBarBookmarkButton.clickAfterExistenceTestSucceeds() + + bookmarksMenu.clickAfterExistenceTestSucceeds() + let bookmarkedItemInMenu = app.menuItems[pageTitle] + + XCTAssertTrue( + bookmarkedItemInMenu.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "Bookmarked page couldn't be detected in the bookmarks menu in a reasonable timeframe." + ) + } + + func test_bookmarks_canBeViewedInAddressBarBookmarkDialog() { + openSiteToBookmark(bookmarkingViaDialog: true, escapingDialog: true) + XCTAssertTrue( + addressBarBookmarkButton.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "Address bar bookmark button didn't load with the expected title in a reasonable timeframe." + ) + let addressBarBookmarkButtonValue = try? XCTUnwrap( + addressBarBookmarkButton.value as? String, + "It wasn't possible to get the value of the bookmarks management accessory image view as String" + ) + XCTAssertEqual( + addressBarBookmarkButtonValue, + "Bookmarked", + "The accessibility value of the Address Bar Bookmark Button must be \"Bookmarked\"." + ) + + addressBarBookmarkButton.click() + let bookMarkDialogBookmarkTitle = app.textFields[pageTitle] + + XCTAssertTrue( + bookMarkDialogBookmarkTitle.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The bookmarked url title wasn't found in the bookmark dialog in a bookmarked state in a reasonable timeframe." + ) + } + + func test_bookmarksTab_canBeViewedViaMenuItemManageBookmarks() { + openSiteToBookmark(bookmarkingViaDialog: true, escapingDialog: true) + bookmarksMenu.clickAfterExistenceTestSucceeds() + + manageBookmarksMenuItem.clickAfterExistenceTestSucceeds() + + XCTAssertTrue( + bookmarksTabPopup.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "Bookmarks tab bookmarks popup didn't load with the expected title in a reasonable timeframe." + ) + } + + func test_favorites_appearWithTheCorrectIndicatorInBookmarksTab() { + openSiteToBookmark(bookmarkingViaDialog: true, escapingDialog: false) + XCTAssertTrue( + bookmarksDialogAddToFavoritesCheckbox.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The add to favorites checkbox in the add bookmark dialog didn't appear with the expected title in a reasonable timeframe." + ) + let bookmarksDialogAddToFavoritesCheckboxValue = try? XCTUnwrap( + bookmarksDialogAddToFavoritesCheckbox.value as? Bool, + "It wasn't possible to get the value of the bookmarks dialog's add to favorites checkbox as Bool" + ) + if bookmarksDialogAddToFavoritesCheckboxValue == false { + bookmarksDialogAddToFavoritesCheckbox.click() + } + app.typeKey(.escape, modifierFlags: []) // Exit dialog + + bookmarksMenu.clickAfterExistenceTestSucceeds() + manageBookmarksMenuItem.clickAfterExistenceTestSucceeds() + XCTAssertTrue( + bookmarksTabPopup.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "Bookmarks tab bookmarks popup didn't load with the expected title in a reasonable timeframe." + ) + XCTAssertTrue( + bookmarksManagementAccessoryImageView.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "Bookmarks accessory view favorites indicator didn't load with the expected title in a reasonable timeframe." + ) + let bookmarksManagementAccessoryImageViewValue = try? XCTUnwrap( + bookmarksManagementAccessoryImageView.value as? String, + "It wasn't possible to get the value of the bookmarks management accessory image view as String" + ) + + XCTAssertEqual( + bookmarksManagementAccessoryImageViewValue, + "Favorited", + "The accessibility value of the favorite accessory view on the bookmark management view must be \"Favorited\"." + ) + } + + func test_favorites_appearInNewTabFavoritesGrid() throws { + openSiteToBookmark(bookmarkingViaDialog: true, escapingDialog: false) + XCTAssertTrue( + bookmarksDialogAddToFavoritesCheckbox.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The add to favorites checkbox in the add bookmark dialog didn't appear with the expected title in a reasonable timeframe." + ) + let bookmarksDialogAddToFavoritesCheckboxValue = try? XCTUnwrap( + bookmarksDialogAddToFavoritesCheckbox.value as? Bool, + "It wasn't possible to get the value of the bookmarks dialog's add to favorites checkbox as Bool" + ) + if bookmarksDialogAddToFavoritesCheckboxValue == false { + bookmarksDialogAddToFavoritesCheckbox.click() + } + app.typeKey(.escape, modifierFlags: []) // Exit dialog + + toggleBookmarksBarShowFavoritesOn() + let unwrappedPageTitle = try XCTUnwrap(pageTitle, "It wasn't possible to unwrap pageTitle") + let firstFavoriteInGridMatchingTitle = app.staticTexts["HomePage.Models.FavoriteModel.\(unwrappedPageTitle)"].firstMatch + + XCTAssertTrue( + firstFavoriteInGridMatchingTitle.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The favorited item in the grid did not become available in a reasonable timeframe." + ) + } + + func test_favorites_canBeRemovedFromAddressBarBookmarkDialog() { + openSiteToBookmark(bookmarkingViaDialog: true, escapingDialog: false) + XCTAssertTrue( + bookmarksDialogAddToFavoritesCheckbox.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The add to favorites checkbox in the add bookmark dialog didn't appear with the expected title in a reasonable timeframe." + ) + let bookmarksDialogAddToFavoritesCheckboxValue = try? XCTUnwrap( + bookmarksDialogAddToFavoritesCheckbox.value as? Bool, + "It wasn't possible to get the value of the bookmarks dialog's add to favorites checkbox as Bool" + ) + if bookmarksDialogAddToFavoritesCheckboxValue == false { + bookmarksDialogAddToFavoritesCheckbox.click() // Favorite the bookmark + } + app.typeKey(.escape, modifierFlags: []) // Exit dialog + + XCTAssertTrue( + addressBarBookmarkButton.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "Address bar bookmark button didn't load with the expected title in a reasonable timeframe." + ) + let addressBarBookmarkButtonValue = try? XCTUnwrap( + addressBarBookmarkButton.value as? String, + "It wasn't possible to get the value of the bookmarks management accessory image view as String" + ) + XCTAssertEqual( + addressBarBookmarkButtonValue, + "Bookmarked", + "The accessibility value of the Address Bar Bookmark Button must be \"Bookmarked\"." + ) + addressBarBookmarkButton.click() + XCTAssertTrue( + bookmarksDialogAddToFavoritesCheckbox.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The add to favorites checkbox in the add bookmark dialog didn't appear with the expected title in a reasonable timeframe." + ) + let bookmarksDialogAddToFavoritesCheckboxNewValue = try? XCTUnwrap( + bookmarksDialogAddToFavoritesCheckbox.value as? Bool, + "It wasn't possible to get the value of the bookmarks dialog's add to favorites checkbox as Bool" + ) + if bookmarksDialogAddToFavoritesCheckboxNewValue == true { + bookmarksDialogAddToFavoritesCheckbox.click() // Unfavorite the bookmark + } + let bookmarksDialogAddToFavoritesCheckboxLastValue = try? XCTUnwrap( + bookmarksDialogAddToFavoritesCheckbox.value as? Bool, + "It wasn't possible to get the value of the bookmarks dialog's add to favorites checkbox as Bool" + ) + let addToFavoritesLabel = "Add to Favorites" + + XCTAssertEqual( + bookmarksDialogAddToFavoritesCheckboxLastValue, + false, + "The favorite checkbox in the add bookmark dialog must now be unchecked" + ) + XCTAssertEqual( + bookmarksDialogAddToFavoritesCheckbox.label, + addToFavoritesLabel, + "The label of the add to favorites checkbox must now be \"\(addToFavoritesLabel)\"" + ) + } + + func test_favorites_canBeRemovedFromManageBookmarks() { + openSiteToBookmark(bookmarkingViaDialog: true, escapingDialog: false) + XCTAssertTrue( + bookmarksDialogAddToFavoritesCheckbox.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The add to favorites checkbox in the add bookmark dialog didn't appear with the expected title in a reasonable timeframe." + ) + let bookmarksDialogAddToFavoritesCheckboxValue = try? XCTUnwrap( + bookmarksDialogAddToFavoritesCheckbox.value as? Bool, + "It wasn't possible to get the value of the bookmarks dialog's add to favorites checkbox as Bool" + ) + if bookmarksDialogAddToFavoritesCheckboxValue == false { + bookmarksDialogAddToFavoritesCheckbox.click() // Favorite the bookmark + } + app.typeKey(.escape, modifierFlags: []) // Exit dialog + + bookmarksMenu.clickAfterExistenceTestSucceeds() + manageBookmarksMenuItem.clickAfterExistenceTestSucceeds() + bookmarkTableCellViewFavIconImageView.hoverAfterExistenceTestSucceeds() + bookmarkTableCellViewMenuButton.clickAfterExistenceTestSucceeds() + contextualMenuRemoveBookmarkFromFavoritesMenuItem.clickAfterExistenceTestSucceeds() + + XCTAssertTrue( + bookmarksManagementAccessoryImageView.waitForNonExistence(timeout: UITests.Timeouts.elementExistence), + "Bookmarks accessory view favorites indicator didn't disappear from the view in a reasonable timeframe." + ) + } + + func test_favorites_canBeRemovedFromNewTabViaContextClick() throws { + toggleBookmarksBarShowFavoritesOn() + openSiteToBookmark(bookmarkingViaDialog: true, escapingDialog: false) + XCTAssertTrue( + bookmarksDialogAddToFavoritesCheckbox.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The add to favorites checkbox in the add bookmark dialog didn't appear with the expected title in a reasonable timeframe." + ) + let bookmarksDialogAddToFavoritesCheckboxValue = try? XCTUnwrap( + bookmarksDialogAddToFavoritesCheckbox.value as? Bool, + "It wasn't possible to get the value of the bookmarks dialog's add to favorites checkbox as Bool" + ) + if bookmarksDialogAddToFavoritesCheckboxValue == false { + bookmarksDialogAddToFavoritesCheckbox.click() // Favorite the bookmark + } + app.typeKey(.escape, modifierFlags: []) // Exit dialog + app.typeKey("w", modifierFlags: [.command, .option, .shift]) // Close all windows + app.typeKey("n", modifierFlags: .command) // New window + + let unwrappedPageTitle = try XCTUnwrap(pageTitle, "It wasn't possible to unwrap pageTitle") + let firstFavoriteInGridMatchingTitle = app.staticTexts["HomePage.Models.FavoriteModel.\(unwrappedPageTitle)"].firstMatch + XCTAssertTrue( + firstFavoriteInGridMatchingTitle.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The favorited item in the grid did not become available in a reasonable timeframe." + ) + firstFavoriteInGridMatchingTitle.rightClick() + removeFavoritesContextMenuItem.clickAfterExistenceTestSucceeds() + + XCTAssertTrue( + firstFavoriteInGridMatchingTitle.waitForNonExistence(timeout: UITests.Timeouts.elementExistence), + "The favorited item in the grid did not disappear in a reasonable timeframe." + ) + } + + func test_bookmark_canBeRemovedViaAddressBarIconClick() { + toggleShowBookmarksBarAlwaysOn() + app.typeKey("w", modifierFlags: [.command, .option, .shift]) + app.typeKey("n", modifierFlags: .command) + openSiteToBookmark(bookmarkingViaDialog: true, escapingDialog: true) + + addressBarBookmarkButton.clickAfterExistenceTestSucceeds() + defaultBookmarkOtherButton.clickAfterExistenceTestSucceeds() + app.typeKey(.escape, modifierFlags: []) // Exit dialog + app.typeKey("w", modifierFlags: [.command, .option, .shift]) + app.typeKey("n", modifierFlags: .command) + + XCTAssertTrue( + app.staticTexts[pageTitle].waitForNonExistence(timeout: UITests.Timeouts.elementExistence), + "Since there is no bookmark of the page, and we show bookmarks in the bookmark bar, the title of the page should not appear in a new browser window anywhere." + ) + } + + func test_bookmark_canBeRemovedFromBookmarksTabViaHoverAndContextMenu() { + app.typeKey("w", modifierFlags: [.command, .option, .shift]) + app.typeKey("n", modifierFlags: .command) + openSiteToBookmark(bookmarkingViaDialog: true, escapingDialog: true) + + bookmarksMenu.clickAfterExistenceTestSucceeds() + manageBookmarksMenuItem.clickAfterExistenceTestSucceeds() + bookmarkTableCellViewFavIconImageView.hoverAfterExistenceTestSucceeds() + bookmarkTableCellViewMenuButton.clickAfterExistenceTestSucceeds() + contextualMenuDeleteBookmarkMenuItem.clickAfterExistenceTestSucceeds() + app.typeKey("w", modifierFlags: [.command, .option, .shift]) + app.typeKey("n", modifierFlags: .command) + + XCTAssertTrue( + app.staticTexts[pageTitle].waitForNonExistence(timeout: UITests.Timeouts.elementExistence), + "Since there is no bookmark of the page, and we show bookmarks in the bookmark bar, the title of the page should not appear in a new browser window anywhere." + ) + } + + func test_bookmark_canBeRemovedFromBookmarksBarViaRightClick() { +// This test uses coordinates (instead of accessibility IDs) to address the elements of the right click. As the writer of this test, I see this +// as a fragile test hook. However, I think it is preferable to making changes to the UI element it tests for this test alone. The reason is +// that the bookmark item on the bookmark bar isn't yet an accessibility-enabled UI element and doesn't appear to have a natural anchor point +// from which we can set its accessibility values without redesigning it. However, redesigning a road-tested UI element for a single test isn't a +// good idea, since the road-testing is also (valuable) testing and we don't want a single test to be the driver of a possible behavioral +// change in existing interface. +// +// My advice is to keep this as-is for now, with an awareness that it can fail if the coordinates of the items in the right-click menu change, +// or if the system where the testing is done has accessibility settings which change scaling. When the time comes to update this element, into +// SwiftUI, or into a general accessibility revision (for end-user accessibility rather than UI test accessibility), that will be the natural +// time to correct this test and give it accessibility ID access. Until then, I have added some hinting in the failure reason to explain why +// this test can fail while the app is working correctly. -Halle Winkler + + toggleShowBookmarksBarAlwaysOn() + app.typeKey("w", modifierFlags: [.command, .option, .shift]) + app.typeKey("n", modifierFlags: .command) + openSiteToBookmark(bookmarkingViaDialog: true, escapingDialog: true) + app.typeKey("w", modifierFlags: [.command, .option, .shift]) + app.typeKey("n", modifierFlags: .command) + + XCTAssertTrue( + bookmarksBarCollectionView.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The bookmarks bar collection view failed to become available in a reasonable timeframe." + ) + let bookmarkBarBookmarkIcon = bookmarksBarCollectionView.images.firstMatch + XCTAssertTrue( + bookmarkBarBookmarkIcon.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The bookmarks bar bookmark icon failed to become available in a reasonable timeframe." + ) + let bookmarkBarBookmarkIconCoordinate = bookmarkBarBookmarkIcon.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.5)) + let deleteContextMenuItemCoordinate = bookmarkBarBookmarkIcon.coordinate(withNormalizedOffset: CGVector(dx: 0.9, dy: 9.0)) + bookmarkBarBookmarkIconCoordinate.rightClick() + deleteContextMenuItemCoordinate.click() + app.typeKey("w", modifierFlags: [.command, .option, .shift]) + app.typeKey("n", modifierFlags: .command) + + XCTAssertTrue( + app.staticTexts[pageTitle].waitForNonExistence(timeout: UITests.Timeouts.elementExistence), + "Since there is no bookmark of the page, and we show bookmarks in the bookmark bar, the title of the page should not appear in a new browser window anywhere. In this specific test, it is highly probable that the reason for a failure (when this area of the app appears to be working correctly) is the contextual menu being rearranged, since it has to address the menu elements by coordinate." + ) + } +} + +private extension BookmarksAndFavoritesTests { + /// Reset the bookmarks so we can rely on a single bookmark's existence + func resetBookmarks() { + app.typeKey("n", modifierFlags: [.command]) // Can't use debug menu without a window + XCTAssertTrue( + resetBookMarksMenuItem.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "Reset bookmarks menu item didn't become available in a reasonable timeframe." + ) + resetBookMarksMenuItem.click() + } + + /// Make sure that we can reply on the bookmarks bar always appearing + func toggleShowBookmarksBarAlwaysOn() { + app.typeKey(",", modifierFlags: [.command]) // Open settings + + XCTAssertTrue( + settingsAppearanceButton.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The user settings appearance section button didn't become available in a reasonable timeframe." + ) + settingsAppearanceButton.click(forDuration: 0.5, thenDragTo: settingsAppearanceButton) + XCTAssertTrue( + showBookmarksBarPreferenceToggle.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The toggle for showing the bookmarks bar didn't become available in a reasonable timeframe." + ) + + let showBookmarksBarIsChecked = try? XCTUnwrap( + showBookmarksBarPreferenceToggle.value as? Bool, + "It wasn't possible to get the \"Show bookmarks bar\" value as a Bool" + ) + if showBookmarksBarIsChecked == false { + showBookmarksBarPreferenceToggle.click() + } + XCTAssertTrue( + showBookmarksBarPopup.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The \"Show Bookmarks Bar\" popup button didn't become available in a reasonable timeframe." + ) + showBookmarksBarPopup.click() + XCTAssertTrue( + showBookmarksBarAlways.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The \"Show Bookmarks Bar Always\" button didn't become available in a reasonable timeframe." + ) + showBookmarksBarAlways.click() + } + + /// Make sure that appearance tab has been used to set "show favorites" to true + func toggleBookmarksBarShowFavoritesOn() { + app.typeKey(",", modifierFlags: [.command]) // Open settings + + XCTAssertTrue( + settingsAppearanceButton.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The user settings appearance section button didn't become available in a reasonable timeframe." + ) + settingsAppearanceButton.click(forDuration: 0.5, thenDragTo: settingsAppearanceButton) + + XCTAssertTrue( + showFavoritesPreferenceToggle.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The user settings appearance section show favorites toggle didn't become available in a reasonable timeframe." + ) + let showFavoritesPreferenceToggleIsChecked = showFavoritesPreferenceToggle.value as? Bool + if showFavoritesPreferenceToggleIsChecked == false { // If untoggled, + showFavoritesPreferenceToggle.click() // Toggle "show favorites" + } + app.typeKey("w", modifierFlags: [.command, .option, .shift]) // Close settings and everything else + app.typeKey("n", modifierFlags: .command) // New window + } + + /// Open the initial site to be bookmarked, bookmarking it and/or escaping out of the dialog only if needed + /// - Parameter bookmarkingViaDialog: open bookmark dialog, adding bookmark + /// - Parameter escapingDialog: `esc` key to leave dialog + func openSiteToBookmark(bookmarkingViaDialog: Bool, escapingDialog: Bool) { + XCTAssertTrue( + addressBarTextField.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The address bar text field didn't become available in a reasonable timeframe." + ) + addressBarTextField.typeURL(urlForBookmarksBar) + XCTAssertTrue( + app.windows.webViews[pageTitle].waitForExistence(timeout: UITests.Timeouts.elementExistence), + "Visited site didn't load with the expected title in a reasonable timeframe." + ) + if bookmarkingViaDialog { + app.typeKey("d", modifierFlags: [.command]) // Add bookmark + if escapingDialog { + app.typeKey(.escape, modifierFlags: []) // Exit dialog + } + } + } +} diff --git a/UITests/BookmarksBarTests.swift b/UITests/BookmarksBarTests.swift index 55eecbe52b..eacea0c9aa 100644 --- a/UITests/BookmarksBarTests.swift +++ b/UITests/BookmarksBarTests.swift @@ -55,7 +55,6 @@ class BookmarksBarTests: XCTestCase { resetBookmarksAndAddOneBookmark() app.typeKey("w", modifierFlags: [.command, .option, .shift]) // Close windows openSettingsAndSetShowBookmarksBarToUnchecked() - settingsWindow = app.windows.containing(.checkBox, identifier: "Preferences.AppearanceView.showBookmarksBarPreferenceToggle").firstMatch openSecondWindowAndVisitSite() siteWindow = app.windows.containing(.webView, identifier: pageTitle).firstMatch } diff --git a/UITests/Common/UITests.swift b/UITests/Common/UITests.swift index 40f28936f9..438cf5be4d 100644 --- a/UITests/Common/UITests.swift +++ b/UITests/Common/UITests.swift @@ -24,7 +24,7 @@ enum UITests { /// Timeout constants for different test requirements enum Timeouts { /// Mostly, we use timeouts to wait for element existence. This is about 3x longer than needed, for CI resilience - static let elementExistence: Double = 2.5 + static let elementExistence: Double = 5.0 /// The fire animation time has environmental dependencies, so we want to wait for completion so we don't try to type into it static let fireAnimation: Double = 30.0 } diff --git a/UITests/Common/XCUIElementExtension.swift b/UITests/Common/XCUIElementExtension.swift index 7e72c41a02..1e0d0ba991 100644 --- a/UITests/Common/XCUIElementExtension.swift +++ b/UITests/Common/XCUIElementExtension.swift @@ -62,4 +62,20 @@ extension XCUIElement { self.typeText("\r") } } + + func clickAfterExistenceTestSucceeds() { + XCTAssertTrue( + self.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "\(self.debugDescription) didn't load with the expected title in a reasonable timeframe." + ) + self.click() + } + + func hoverAfterExistenceTestSucceeds() { + XCTAssertTrue( + self.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "\(self.debugDescription) didn't load with the expected title in a reasonable timeframe." + ) + self.hover() + } } From 0d7c9de8780696937690d0841d0a7fd73a88322c Mon Sep 17 00:00:00 2001 From: Tom Strba <57389842+tomasstrba@users.noreply.github.com> Date: Tue, 9 Apr 2024 15:31:56 +0200 Subject: [PATCH 3/4] New daily pixel for history save failures (#2533) Task/Issue URL: https://app.asana.com/0/0/1206920829163329/f **Description**: Adding new daily pixel for history save failures to better understand how many devices are affected by a potential issue --- DuckDuckGo/History/Services/EncryptedHistoryStore.swift | 3 +++ DuckDuckGo/Statistics/PixelEvent.swift | 3 +++ DuckDuckGo/Statistics/PixelParameters.swift | 1 + 3 files changed, 7 insertions(+) diff --git a/DuckDuckGo/History/Services/EncryptedHistoryStore.swift b/DuckDuckGo/History/Services/EncryptedHistoryStore.swift index 361ab26a81..36838c0539 100644 --- a/DuckDuckGo/History/Services/EncryptedHistoryStore.swift +++ b/DuckDuckGo/History/Services/EncryptedHistoryStore.swift @@ -175,6 +175,7 @@ final class EncryptedHistoryStore: HistoryStoring { fetchedObjects = try self.context.fetch(fetchRequest) } catch { Pixel.fire(.debug(event: .historySaveFailed, error: error)) + Pixel.fire(.debug(event: .historySaveFailedDaily, error: error), limitTo: .dailyFirst) promise(.failure(error)) return } @@ -203,6 +204,7 @@ final class EncryptedHistoryStore: HistoryStoring { switch insertionResult { case .failure(let error): Pixel.fire(.debug(event: .historySaveFailed, error: error)) + Pixel.fire(.debug(event: .historySaveFailedDaily, error: error), limitTo: .dailyFirst) context.reset() promise(.failure(error)) case .success(let visitMOs): @@ -210,6 +212,7 @@ final class EncryptedHistoryStore: HistoryStoring { try self.context.save() } catch { Pixel.fire(.debug(event: .historySaveFailed, error: error)) + Pixel.fire(.debug(event: .historySaveFailedDaily, error: error), limitTo: .dailyFirst) context.reset() promise(.failure(HistoryStoreError.savingFailed)) return diff --git a/DuckDuckGo/Statistics/PixelEvent.swift b/DuckDuckGo/Statistics/PixelEvent.swift index 9f789182da..902e5b0b5a 100644 --- a/DuckDuckGo/Statistics/PixelEvent.swift +++ b/DuckDuckGo/Statistics/PixelEvent.swift @@ -326,6 +326,7 @@ extension Pixel { case historyCleanEntriesFailed case historyCleanVisitsFailed case historySaveFailed + case historySaveFailedDaily case historyInsertVisitFailed case historyRemoveVisitsFailed @@ -816,6 +817,8 @@ extension Pixel.Event.Debug { return "history_clean_visits_failed" case .historySaveFailed: return "history_save_failed" + case .historySaveFailedDaily: + return "history_save_failed_daily" case .historyInsertVisitFailed: return "history_insert_visit_failed" case .historyRemoveVisitsFailed: diff --git a/DuckDuckGo/Statistics/PixelParameters.swift b/DuckDuckGo/Statistics/PixelParameters.swift index 3b7d89b5c4..2fad773b42 100644 --- a/DuckDuckGo/Statistics/PixelParameters.swift +++ b/DuckDuckGo/Statistics/PixelParameters.swift @@ -256,6 +256,7 @@ extension Pixel.Event.Debug { .historyCleanEntriesFailed, .historyCleanVisitsFailed, .historySaveFailed, + .historySaveFailedDaily, .historyInsertVisitFailed, .historyRemoveVisitsFailed, .emailAutofillKeychainError, From ebb09c340f47ae13a3bd849caec8b769050d7e56 Mon Sep 17 00:00:00 2001 From: Anh Do <18567+quanganhdo@users.noreply.github.com> Date: Tue, 9 Apr 2024 09:40:07 -0400 Subject: [PATCH 4/4] Change DAU pixel for VPN to weekly (#2552) Task/Issue URL: https://app.asana.com/0/0/1206685546588311/f **Description**: Reported `cohort` now uses the week number. --- .../MacPacketTunnelProvider.swift | 2 +- .../PixelKit/PixelKit+Parameters.swift | 2 + .../PixelKit/Sources/PixelKit/PixelKit.swift | 32 ++++--- .../Tests/PixelKitTests/PixelKitTests.swift | 96 ++++++++++++++++--- 4 files changed, 104 insertions(+), 28 deletions(-) diff --git a/DuckDuckGo/NetworkProtection/NetworkExtensionTargets/NetworkExtensionTargets/MacPacketTunnelProvider.swift b/DuckDuckGo/NetworkProtection/NetworkExtensionTargets/NetworkExtensionTargets/MacPacketTunnelProvider.swift index c9a8819faa..0689316a76 100644 --- a/DuckDuckGo/NetworkProtection/NetworkExtensionTargets/NetworkExtensionTargets/MacPacketTunnelProvider.swift +++ b/DuckDuckGo/NetworkProtection/NetworkExtensionTargets/NetworkExtensionTargets/MacPacketTunnelProvider.swift @@ -151,7 +151,7 @@ final class MacPacketTunnelProvider: PacketTunnelProvider { PixelKit.fire( NetworkProtectionPixelEvent.networkProtectionActiveUser, frequency: .dailyOnly, - withAdditionalParameters: ["cohort": PixelKit.dateString(for: defaults.vpnFirstEnabled)], + withAdditionalParameters: [PixelKit.Parameters.vpnCohort: PixelKit.cohort(from: defaults.vpnFirstEnabled)], includeAppVersionParameter: true) case .reportConnectionAttempt(attempt: let attempt): switch attempt { diff --git a/LocalPackages/PixelKit/Sources/PixelKit/PixelKit+Parameters.swift b/LocalPackages/PixelKit/Sources/PixelKit/PixelKit+Parameters.swift index 777329156a..b692271709 100644 --- a/LocalPackages/PixelKit/Sources/PixelKit/PixelKit+Parameters.swift +++ b/LocalPackages/PixelKit/Sources/PixelKit/PixelKit+Parameters.swift @@ -66,6 +66,8 @@ public extension PixelKit { public static let vpnBreakageMetadata = "breakageMetadata" public static let reason = "reason" + + public static let vpnCohort = "cohort" } enum Values { diff --git a/LocalPackages/PixelKit/Sources/PixelKit/PixelKit.swift b/LocalPackages/PixelKit/Sources/PixelKit/PixelKit.swift index eec1e1332c..f55ec17ba4 100644 --- a/LocalPackages/PixelKit/Sources/PixelKit/PixelKit.swift +++ b/LocalPackages/PixelKit/Sources/PixelKit/PixelKit.swift @@ -72,13 +72,7 @@ public final class PixelKit { return calendar }() - private var dateFormatter: DateFormatter = { - let dateFormatter = DateFormatter() - dateFormatter.calendar = defaultDailyPixelCalendar - dateFormatter.timeZone = defaultDailyPixelCalendar.timeZone - dateFormatter.dateFormat = "yyyy-MM-dd" - return dateFormatter - }() + private static let weeksToCoalesceCohort = 6 private let dateGenerator: () -> Date @@ -99,6 +93,8 @@ public final class PixelKit { source: String? = nil, defaultHeaders: [String: String], log: OSLog, + dailyPixelCalendar: Calendar? = nil, + dateGenerator: @escaping () -> Date = Date.init, defaults: UserDefaults, fireRequest: @escaping FireRequest) { shared = PixelKit(dryRun: dryRun, @@ -106,6 +102,8 @@ public final class PixelKit { source: source, defaultHeaders: defaultHeaders, log: log, + dailyPixelCalendar: dailyPixelCalendar, + dateGenerator: dateGenerator, defaults: defaults, fireRequest: fireRequest) } @@ -318,13 +316,23 @@ public final class PixelKit { onComplete: onComplete) } - private func dateString(for date: Date?) -> String? { - guard let date else { return nil } - return dateFormatter.string(from: date) + private func cohort(from cohortLocalDate: Date?, dateGenerator: () -> Date = Date.init) -> String? { + guard let cohortLocalDate, + let baseDate = pixelCalendar.date(from: .init(year: 2023, month: 1, day: 1)), + let weeksSinceCohortAssigned = pixelCalendar.dateComponents([.weekOfYear], from: cohortLocalDate, to: dateGenerator()).weekOfYear, + let assignedCohort = pixelCalendar.dateComponents([.weekOfYear], from: baseDate, to: cohortLocalDate).weekOfYear else { + return nil + } + + if weeksSinceCohortAssigned > Self.weeksToCoalesceCohort { + return "" + } else { + return "week-" + String(assignedCohort + 1) + } } - public static func dateString(for date: Date?) -> String { - Self.shared?.dateString(for: date) ?? "" + public static func cohort(from cohortLocalDate: Date?, dateGenerator: () -> Date = Date.init) -> String { + Self.shared?.cohort(from: cohortLocalDate, dateGenerator: dateGenerator) ?? "" } public static func pixelLastFireDate(event: Event) -> Date? { diff --git a/LocalPackages/PixelKit/Tests/PixelKitTests/PixelKitTests.swift b/LocalPackages/PixelKit/Tests/PixelKitTests/PixelKitTests.swift index b9576d6a6a..3ce7447aae 100644 --- a/LocalPackages/PixelKit/Tests/PixelKitTests/PixelKitTests.swift +++ b/LocalPackages/PixelKit/Tests/PixelKitTests/PixelKitTests.swift @@ -258,17 +258,18 @@ final class PixelKitTests: XCTestCase { // Run test pixelKit.fire(event, frequency: .dailyOnly) // Fired - timeMachine.travel(by: 60 * 60 * 2) - pixelKit.fire(event, frequency: .dailyOnly) // Skipped (2 hours since last fire) + timeMachine.travel(by: .hour, value: 2) + pixelKit.fire(event, frequency: .dailyOnly) // Skipped - timeMachine.travel(by: 60 * 60 * 24 + 1000) - pixelKit.fire(event, frequency: .dailyOnly) // Fired (24 hours + 1000 seconds since last fire) + timeMachine.travel(by: .day, value: 1) + timeMachine.travel(by: .hour, value: 2) + pixelKit.fire(event, frequency: .dailyOnly) // Fired - timeMachine.travel(by: 60 * 60 * 10) - pixelKit.fire(event, frequency: .dailyOnly) // Skipped (10 hours since last fire) + timeMachine.travel(by: .hour, value: 10) + pixelKit.fire(event, frequency: .dailyOnly) // Skipped - timeMachine.travel(by: 60 * 60 * 14) - pixelKit.fire(event, frequency: .dailyOnly) // Fired (24 hours since last fire) + timeMachine.travel(by: .day, value: 1) + pixelKit.fire(event, frequency: .dailyOnly) // Fired // Wait for expectations to be fulfilled wait(for: [fireCallbackCalled], timeout: 0.5) @@ -303,28 +304,93 @@ final class PixelKitTests: XCTestCase { // Run test pixelKit.fire(event, frequency: .justOnce) // Fired - timeMachine.travel(by: 60 * 60 * 2) + timeMachine.travel(by: .hour, value: 2) pixelKit.fire(event, frequency: .justOnce) // Skipped (already fired) - timeMachine.travel(by: 60 * 60 * 24 + 1000) + timeMachine.travel(by: .day, value: 1) + timeMachine.travel(by: .hour, value: 2) pixelKit.fire(event, frequency: .justOnce) // Skipped (already fired) - timeMachine.travel(by: 60 * 60 * 10) + timeMachine.travel(by: .hour, value: 10) pixelKit.fire(event, frequency: .justOnce) // Skipped (already fired) - timeMachine.travel(by: 60 * 60 * 14) + timeMachine.travel(by: .day, value: 1) pixelKit.fire(event, frequency: .justOnce) // Skipped (already fired) // Wait for expectations to be fulfilled wait(for: [fireCallbackCalled], timeout: 0.5) } + + func testVPNCohort() { + XCTAssertEqual(PixelKit.cohort(from: nil), "") + assertCohortEqual(.init(year: 2023, month: 1, day: 1), reportAs: "week-1") + assertCohortEqual(.init(year: 2024, month: 2, day: 24), reportAs: "week-60") + } + + private func assertCohortEqual(_ cohort: DateComponents, reportAs reportedCohort: String) { + var calendar = Calendar.current + calendar.timeZone = TimeZone(secondsFromGMT: 0)! + calendar.locale = Locale(identifier: "en_US_POSIX") + + let cohort = calendar.date(from: cohort) + let timeMachine = TimeMachine(calendar: calendar, date: cohort) + + PixelKit.setUp(appVersion: "test", + defaultHeaders: [:], + log: .disabled, + dailyPixelCalendar: calendar, + dateGenerator: timeMachine.now, + defaults: userDefaults()) { _, _, _, _, _, _ in } + + // 1st week + XCTAssertEqual(PixelKit.cohort(from: cohort, dateGenerator: timeMachine.now), reportedCohort) + + // 2nd week + timeMachine.travel(by: .weekOfYear, value: 1) + XCTAssertEqual(PixelKit.cohort(from: cohort, dateGenerator: timeMachine.now), reportedCohort) + + // 3rd week + timeMachine.travel(by: .weekOfYear, value: 1) + XCTAssertEqual(PixelKit.cohort(from: cohort, dateGenerator: timeMachine.now), reportedCohort) + + // 4th week + timeMachine.travel(by: .weekOfYear, value: 1) + XCTAssertEqual(PixelKit.cohort(from: cohort, dateGenerator: timeMachine.now), reportedCohort) + + // 5th week + timeMachine.travel(by: .weekOfYear, value: 1) + XCTAssertEqual(PixelKit.cohort(from: cohort, dateGenerator: timeMachine.now), reportedCohort) + + // 6th week + timeMachine.travel(by: .weekOfYear, value: 1) + XCTAssertEqual(PixelKit.cohort(from: cohort, dateGenerator: timeMachine.now), reportedCohort) + + // 7th week + timeMachine.travel(by: .weekOfYear, value: 1) + XCTAssertEqual(PixelKit.cohort(from: cohort, dateGenerator: timeMachine.now), reportedCohort) + + // 8th week + timeMachine.travel(by: .weekOfYear, value: 1) + XCTAssertEqual(PixelKit.cohort(from: cohort, dateGenerator: timeMachine.now), "") + } } private class TimeMachine { - private var date = Date(timeIntervalSince1970: 0) + private var date: Date + private let calendar: Calendar + + init(calendar: Calendar? = nil, date: Date? = nil) { + self.calendar = calendar ?? { + var calendar = Calendar.current + calendar.timeZone = TimeZone(secondsFromGMT: 0)! + calendar.locale = Locale(identifier: "en_US_POSIX") + return calendar + }() + self.date = date ?? .init(timeIntervalSince1970: 0) + } - func travel(by timeInterval: TimeInterval) { - date = date.addingTimeInterval(timeInterval) + func travel(by component: Calendar.Component, value: Int) { + date = calendar.date(byAdding: component, value: value, to: now())! } func now() -> Date {