132 lines
10 KiB
Markdown
Executable File
132 lines
10 KiB
Markdown
Executable File
---
|
||
title: 'macOS tests: worktrees from the 2026-10-14 session'
|
||
date: '2026-10-14'
|
||
tags:
|
||
- ddg
|
||
- macos
|
||
- tests
|
||
- worktrees
|
||
status: in-progress
|
||
---
|
||
# macOS tests: worktrees from the 2026-10-14 session
|
||
|
||
All worktrees are in `/Users/admin/DuckDuckGo/apple-browsers.git/<name>`, on branch `alex/<name>`. Nothing is committed.
|
||
|
||
## Summary
|
||
|
||
| Worktree | Created from | Purpose | Status |
|
||
|---|---|---|---|
|
||
| `macos-unit-tests-no-app-delegate` | origin/main `ca834f869a` | Unit tests no longer use the app delegate, buildable folder, test plans | Done, not committed |
|
||
| `fix-address-bar-tests-flakiness` | origin/main `ddb1a82283` | Fix flaky `AddressBarTests`: mocks plus two leak root causes | Done, not committed |
|
||
| `fix-ask-ai-chat-button-animation-leak` | origin/main `e698211924` | Ask AI Chat button animation leak plus regression test | Done, not committed |
|
||
| ~~`animation-leak-audit`~~ | origin/main `e698211924` | Audit `NSAnimationContext` completion handlers for leaks | Done: no leaks found; worktree and branch removed |
|
||
| `fix-fire-window-video-leak` | origin/main `e698211924` | Video keeps playing after a Fire Window is closed: reproduce, fix, regression test | Not reproduced: 4 guard tests only, no fix (see update below) |
|
||
| `spm-browser-tests` (existing) | — | Source of the unit-test changes (SwiftPM variant) | Not changed |
|
||
|
||
## macos-unit-tests-no-app-delegate
|
||
|
||
Branch `alex/macos-unit-tests-no-app-delegate`. 207 files changed, +5553/−7870 lines of uncommitted changes. Most of the deletions are pbxproj entries.
|
||
|
||
- **Ported from `spm-browser-tests`.** Four commits (915209f987, d22aa6ca14, 966cad8578, 7f39ce3952) take the unit tests off the app delegate, move tests that build a main window to the integration tests, and skip disabled tests in code. The commit that turns the app into a Swift package was left out. Unit Tests stays an Xcode target.
|
||
- **`macOS/UnitTests` is a buildable folder** (pbxproj objectVersion 70), with exceptions for the 12 files main never compiled.
|
||
- `DataImportResources` is listed in `explicitFolders`.
|
||
- `privacy-reference-tests` stays a folder reference.
|
||
- **Test plans:** `UnitTests/Unit Tests.xctestplan`, `TestPlans/macOS Browser.xctestplan`, and `TestPlans/macOS Browser App Store.xctestplan`. They set language en/US, keep code coverage, and keep the App Store skip.
|
||
- **No app delegate in unit tests:** for `runType == .unitTests`, `Application.init` creates neither `AppDelegate` nor the main menu, so any access to the delegate crashes.
|
||
- Every place unit tests still reached the delegate was found with a trace and a default-argument scanner, then given a mock.
|
||
- App code takes those dependencies as parameters that default to the app's values: AIChat omnibar, PermissionAuthorizationView, BookmarkManagementDetailViewController, BookmarksEmptyStateView, SyncDeviceButtonModel, NewTabPageCoordinator/ActionsManager, NewTabPageOmnibarActionsHandler, and others.
|
||
- `open(_ bookmark:)` and `open(_ historyEntry:)` moved into the `WindowControllersManagerProtocol` extension.
|
||
- `BookmarksBarShownTriggerTests` moved to the integration tests.
|
||
- **No network in unit tests:** `allowsUpdates` is false for unit tests, so there is no Sparkle appcast check, and `SuggestionContainerTests` uses a URL session mock.
|
||
- **Results:**
|
||
- Unit: all pass except 10 snapshot tests, which require macOS 27 (this machine runs 26.7.1).
|
||
- Full plan: 9484 passed. Besides the snapshot tests, 6 `AddressBarTests` fail; they also fail on origin/main and are fixed in the next worktree.
|
||
- App Store scheme builds.
|
||
- swiftlint --strict is clean on 315 files.
|
||
- **Note:** the privacy-reference-tests submodule had to be initialized in this worktree.
|
||
|
||
## fix-address-bar-tests-flakiness
|
||
|
||
Branch `alex/fix-address-bar-tests-flakiness`. 3 files, +100/−6.
|
||
|
||
- **Root cause:** the tests didn't actually hang. In DEBUG, `ensureObjectDeallocated(... .interrupt)` sends SIGINT when an object is still alive. Two causes:
|
||
1. `TestRunHelper` read `NSApp.windows`, which autoreleased the address bar's suggestions window. Fixed with `autoreleasepool`.
|
||
2. The `animateAskAIChatButtonContraction()` completion handler captured `self` strongly. Fixed with `[weak self]`. The audit refined this finding: it isn't a permanent leak. While the display is asleep, the completion is held until it wakes.
|
||
- **Mocks and settings in `AddressBarTests`:**
|
||
- OHHTTPStubs answers `duckduckgo.com/ac/` and blocks all other external network.
|
||
- AI Chat settings are pinned in `NSArgumentDomain`.
|
||
- The user's settings are saved and restored.
|
||
- **Results:**
|
||
- Isolated: 7 out of 7 runs with 0 failures and 0 restarts. Before the fix, origin/main had 1–6 failures per run.
|
||
- Full Integration Tests ×2: 0 AddressBarTests failures.
|
||
- The 3 remaining failures also fail on main because of this machine's settings: FireCoordinator (locale) and NTP search (`aichat.enabled=0`).
|
||
- The comment in `AddressBarButtonsViewController` was reworded on 2026-10-15 to say the completion is held until the animation ends. The handler change is now identical to the one in `fix-ask-ai-chat-button-animation-leak`.
|
||
|
||
## fix-ask-ai-chat-button-animation-leak
|
||
|
||
Branch `alex/fix-ask-ai-chat-button-animation-leak`, based on origin/main `e698211924`. **Done, not committed.** 2 files, +63/−3.
|
||
|
||
- **Fix** in `AddressBarButtonsViewController.animateAskAIChatButtonContraction()`: `[weak self]` plus `guard let self`.
|
||
- The comment explains why: the completion is deferred until the animation ends, indefinitely while the display link is paused.
|
||
- `isAskAIChatButtonExpanded` changed from `private` to `private(set)` so the test can read it.
|
||
- The rest of the file was checked: the expansion animation has no completion handler, and the `asyncAfter` and `sink` closures already use `[weak self]`.
|
||
- **Regression test:** `AddressBarTests.testWhenWindowIsClosedWhileAskAIChatButtonContracts_windowObjectsAreDeallocated`.
|
||
- It sets text in the address bar so the button expands, then calls `makeFirstResponder(nil)` to start the contraction.
|
||
- In the same autorelease pool, it closes and releases the window.
|
||
- It immediately asserts that `AddressBarButtonsViewController` and `TabCollectionViewModel` are released. There are no sleeps or waits, so the result doesn't depend on display state.
|
||
- **Results:**
|
||
- main: 10/10 FAIL (5 with the display awake, 5 asleep).
|
||
- fix: 15/15 pass (asleep, awake, and `-test-iterations 5`).
|
||
- Full `AddressBarTests`: 0 failures. `test_WhenControlTextDidChange…` sometimes crashes with SIGINT; that is the `TestRunHelper` autorelease problem fixed in `fix-address-bar-tests-flakiness`.
|
||
- Lint: 0.
|
||
- **Overlap:** `fix-address-bar-tests-flakiness` contains the same handler fix (the comment was aligned on 2026-10-15; the change is identical). Whichever lands first, the other must drop or rebase this hunk.
|
||
|
||
## animation-leak-audit (removed)
|
||
|
||
Checked with standalone AppKit harnesses (in the session scratchpad: `scratchpad/anim/exp*.swift`). No permanent leaks were found, and the code was not changed. The worktree and branch were removed.
|
||
|
||
- **Key finding:** AppKit does call `runAnimationGroup` completion handlers after the window is closed.
|
||
- Animations of a view's `alphaValue`/`frame`, `performBatchUpdates`, and `scroll` always complete.
|
||
- Display-link-driven animations (`NSLayoutConstraint.animator().constant`, `NSWindow.animator().alphaValue`) stall while the display is asleep. Their completion, along with any strong captures, is held until the display wakes.
|
||
- This Mac has `displaysleep 10`, so its display is often asleep. That explains the flaky dealloc checks in tests.
|
||
- **Checked, no leaks found:**
|
||
- `NavigationBarBadgeAnimator` (alpha only, no view controller or window captured).
|
||
- `BrowserTabViewController.dismissCookiePopupProtectionOptInDialog` (alpha only, `self` not captured).
|
||
- `AIChatCoordinator.transitionSidebar` and `TabBarViewController` (weak captures, or completions that always run).
|
||
- `PromptBarPresenter.dismiss`, `CircularProgressView`, `SpinnerView`, `JSAlertController`, and others.
|
||
|
||
## fix-fire-window-video-leak
|
||
|
||
Branch `alex/fix-fire-window-video-leak`. Not reproduced; see the update below.
|
||
|
||
## Temporary and source worktrees
|
||
|
||
- `spm-browser-tests` (`alex/spm-browser-tests`) is the existing source of the unit-test changes. Not changed.
|
||
- A detached origin/main baseline in the session scratchpad was used to compare integration tests and has been removed.
|
||
|
||
|
||
## Update 2026-10-15: fix-fire-window-video-leak
|
||
|
||
Status: **not reproduced, no fix.**
|
||
|
||
- **Worktree:** `fix-fire-window-video-leak`, branch `alex/fix-fire-window-video-leak`, based on origin/main `e698211924`.
|
||
- **Changes:** only `macOS/IntegrationTests/Fire/FireWindowClosingTests.swift` (+153), with four synchronous guard tests:
|
||
- closing a Fire Window with media playing;
|
||
- closing it via the Fire popover;
|
||
- closing the last tab in a Fire Window;
|
||
- a regular window, for comparison.
|
||
|
||
The media is a generated 440 Hz WAV served from `https://media.test` through `TestSchemeHandler`, so no network is involved. All the tests **pass on main** (35/35 with the display asleep, 7/7 with it awake), so they are guard tests, not regression tests.
|
||
- **What was tried:**
|
||
- Content: `data:` audio, `https` video, and a real H.264/AAC mp4.
|
||
- Close paths: `performClose` / Cmd+Shift+W, the Fire popover close action (directly and through the real `FireCoordinator`), Cmd+W on the last tab, and removing the tab.
|
||
- Conditions: a background tab, the Lottie fire animation, the display asleep and awake.
|
||
|
||
In every case the Tab, WebView and window were released within 1–3 s, and media stopped.
|
||
- **How the check was validated:** with a deliberate Tab leak added in `animateBurningIfNeededAndClose`, the test failed.
|
||
- **Findings:**
|
||
1. `async` tests mask releases: objects live until the test ends. The guard tests are synchronous for this reason.
|
||
2. `DeallocationTests.testWhenLastTabClosed_windowIsDeallocated` crashes the test host when the display is asleep (probably `TabPreviewWindowController`) and passes when it is awake. This is a separate display-sleep flake.
|
||
- **Manual check in the app:** not possible, because `osascript` has no Accessibility access.
|
||
- **Next steps:** get the exact user steps (which site, PiP or fullscreen, a popup from the Fire Window, how the window was closed), or reproduce it by hand in a Debug build. The DEBUG deallocation checks will trigger on a leak.
|