Skip to content

Commit b4b58ab

Browse files
graemeclaude
andcommitted
Record the UI-testing decisions
Captures what the diff cannot show: why the identifier module is dependency-free and linked by both targets, why each seam is shaped the way it is, why the fixture tree travels in the launch environment rather than on disk, and why activating the app after launch is load-bearing rather than cosmetic. CONVENTIONS.md pointed at Utilities/AccessibilityIdentifiers.swift, which was never created; BrewAccessibilityID is what actually fills that role. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent f1986ee commit b4b58ab

2 files changed

Lines changed: 31 additions & 1 deletion

File tree

.ai/memory.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,3 +520,33 @@
520520
- **Why `-l` AND `-i`.** `-l` sources `.zprofile` / `.bash_profile` (Homebrew's documented install writes `brew shellenv` to `.zprofile`). `-i` additionally sources interactive rc files (`.zshrc` / `.bashrc`) so users who put their brew setup in those files also get parity. The tradeoff is real and accepted: interactive rc files may print banners or expect a TTY (we run with `/dev/null` stdin, so any TTY-dependent code in rc files will surface as warnings on stderr). If we ever see that as a meaningful noise source for users, the dial is `-l` only.
521521
- **Single spawning route.** Quoting is handled by `LoginShellBrewCommandRunner.singleQuoted` (POSIX `'…'` with `'\''` for embedded apostrophes), so package names and flags survive the shell parse intact. The three production `live()` factories (`BrewCommandExecutionContext.live()`, `BrewConfigRepository.live()`, `BrewInstalledPackagesRepository.live()`) all now construct `LoginShellBrewCommandRunner()`. Tests still use `BrewCommandService()` directly for raw-process plumbing tests — that's correct; the shell wrap is a live-wiring concern.
522522
- **Out of scope (tracked separately).** `brew.env` file parsing and `BrewEnvFileLocator` assume brew sees the same world the user does, which they now do — but parser/locator changes are their own work.
523+
524+
## 2026-08-05 — UI-test identity module + composition-root seams (PR 1 of the UI testing plan)
525+
526+
- **`BrewAccessibilityID` is the single source of truth for testable element identity**, superseding the never-created `Utilities/AccessibilityIdentifiers.swift` noted in the 2026-03-01 entry. It is a dependency-free SwiftPM target linked by **both** the app and the `BrewUITests` target (`Homebrew.xcodeproj``packageProductDependencies` on each), so an identifier string is spelled exactly once, in `AXID.rawValue`. Views attach identity with `.axid(_:)` (`Sources/BrewUIComponents/Views/View+AXID.swift`), never `accessibilityIdentifier` with a literal. `Tests/BrewAccessibilityIDTests` pins the wire format so drift breaks a unit test rather than a UI test.
527+
- **Parameterised cases carry the package token** (`installedRow(token:)`, `discoverRow(token:)`) — the token is the Homebrew name/token that `HomebrewPackageID.name` yields, so rows are addressable without label or index matching.
528+
- **`.searchable` fields cannot carry a custom accessibility identifier.** SwiftUI injects the field into the window toolbar; an `.axid` at the call site lands on the modified content and overwrites the screen's own identifier. `AXID.installedSearchField` / `.discoverSearchField` therefore exist but are unattached — query `app.searchFields` until the field is a custom view.
529+
- **Two process-boundary seams, both public and both inert in production:** `URLSessionBrewAPIClient.stubbed(protocolClasses:baseURL:)` sets protocol classes on one ephemeral session's configuration (never `URLProtocol.registerClass`, which would also capture `.shared`), and `BrewCommandExecutionContext.uiTesting(brewURL:)` uses the **real** `BrewCommandService` with `BrewExecutableLocator(overrideURL:)` and deliberately **no** `LoginShellBrewCommandRunner` — wrapping a fake brew in the user's login shell would re-introduce dotfile dependence (see 2026-06-23).
530+
- **`BrewApp.init()` branches once, on `BrewUITestingLaunchConfiguration.current()`**, which returns `nil` unless the `-uiTesting` launch argument is present. Both seams are a single `guard` away from the untouched `.live()` wiring; being in UI-test mode is never threaded further into the app.
531+
532+
## 2026-08-08 — Page Object Model + deterministic UI suite (PR 2 of the UI testing plan)
533+
534+
- **The suite mocks two process boundaries and nothing else.** Every layer of our own code runs for real: the real `BrewCommandService` spawns a real subprocess and drains real pipes; the real `URLSessionBrewAPIClient` builds requests, negotiates ETag/304, decodes and caches. That is what makes error cases worth writing — a 500 becomes `BrewAPIClientError.httpStatus` through the actual client, not a stubbed error value.
535+
- **One fixture tree feeds both seams.** `FakeBrew.install(scenario:)` writes a per-run temp directory containing `<scenario>/brew` (fake-brew fixtures) and `<scenario>/http` (response bodies), and hands the app the paths via launch environment. The HTTP responder lives **in the app** (`BrewUITestingStubURLProtocol`), not in `BrewUITests`: a `URLProtocol` registered in the test target runs in the *test* process and would never see the app's traffic. PR 2's plan offered both wirings; this is the one chosen.
536+
- **HTTP fixtures are addressed by request path**, `/``_` (`/api/formula.json``api_formula.json`), with optional sibling `.status` and `.etag` files. A matching `If-None-Match` gets a real 304, so the client's conditional-request branch is exercised rather than stubbed away.
537+
- **fake-brew is a lookup table, not a `case` over subcommands.** Files are `<argv joined by _>.stdout` / `.stderr` / `.exitcode` / `.next-info`. Adding a command to a scenario is adding a file. `.next-info` is the one piece of state: on a successful mutating run it becomes the answer to every later `brew info`, which is what lets an uninstall actually remove the row (the repository force-refreshes off the command center's running→idle transition and must see a changed world).
538+
- **PR 1 left two repositories outside the shell seam.** `BrewInstalledPackagesRepository.live()` and `BrewConfigRepository.live()` each constructed their own `LoginShellBrewCommandRunner` + `BrewExecutableLocator`, so the Installed list and Configuration tab ran **real brew** under `-uiTesting`. Both now take a `BrewCommandExecutionContext`, and `BrewApp` builds exactly one context for the whole process. `live()` still means the same wiring it always did.
539+
- **`BrewCommandExecutionContext.uiTesting(brewURL:)` now takes an optional.** `nil` installs a locator that always throws `BrewLookupError.executableNotFound` — that is how the `brewNotFound` scenario is expressed, and it also closes a hole: a `-uiTesting` launch that named no fake used to fall through to `.live()` and could have driven the developer's real Homebrew.
540+
- **UI-test runs are state-isolated even though PR 2's plan defers "state isolation" to PR 3.** `CatalogueCache` / `DiscoverAnalyticsCache` are given the run's scratch container and a `UITesting.`-prefixed `UserDefaults` namespace (cleared at launch). Without this, run *N*'s fixture catalogue decides run *N+1*'s behaviour and the fixtures overwrite the real app's Application Support cache on the same machine. PR 3's isolation is about real-brew/real-network runs; this is the minimum that makes "deterministic across ≥20 runs" mean anything.
541+
- **The console's expand/collapse state is read from the toggle button's accessibility label** ("Show console" when collapsed, "Hide console" when expanded — exactly one is mounted). The app auto-expands the panel by itself when a command starts, so a read-then-decide-then-click sequence races it; `ConsoleScreen.setExpanded(_:)` waits for the state it wants *before* concluding it needs to click.
542+
- **`AXID` gained the shared failure chrome** (`errorState` / `errorRetryButton` on `AsyncContentView`'s error branch, `brewNotFoundState` on Configuration). It is screen-agnostic by design — every loadable surface renders the same view — so screens scope the query to their own root to say which surface failed.
543+
- **Upgrades rows got their own identity** (`upgradesList` / `upgradesRow(token:)`) rather than reusing the Installed row ids. The same package is legitimately in both lists at once, and sharing ids would let an Upgrades assertion be satisfied by an Installed row.
544+
545+
## 2026-08-08 — UI-test fixtures travel in the launch environment, not on disk
546+
547+
- **The failure.** The obvious design — the test runner writes a fixture tree, the app reads it — needs one directory two different processes are both allowed to use, and on this machine there isn't one. `FileManager.temporaryDirectory` resolves per process, so the runner's temp dir gave the app `EPERM` when it tried to spawn the fake `brew` from it; `/private/tmp` then gave the *runner* `EPERM` creating directories. Neither cause was ever established (the runner has `ENABLE_APP_SANDBOX = NO` and Xcode's `XCTRunner.app` template carries no entitlements), and two rounds of guessing at the OS policy cost more than the redesign.
548+
- **The fix is to delete the question.** `BrewUITestContract` (a dependency-free SwiftPM target linked by both the app and `BrewUITests`, exactly like `BrewAccessibilityID`) carries `BrewUITestingFixturePayload` — a bag of relative paths to bytes plus which one is the executable — JSON, raw-DEFLATE, base64, passed in one launch-environment variable. `BrewUITestingFixtureInstaller` writes it into the **app's own** temp directory at launch, chmods the fake, and `setenv`s the resulting paths so the stub `URLProtocol` (which runs on `URLSession` threads) and the fake `brew` subprocess (which inherits its environment) can both find them. One process creates the files, runs them, and owns them.
549+
- **The payload is budgeted, not unbounded.** `BrewApp.launch` rejects anything over 512 KB; the tree compresses ~20× because fixture JSON is highly repetitive, so the large-inventory scenario lands well under it. If a scenario ever breaches the budget, shrink the scenario — the environment is shared with everything else the launch needs.
550+
- **`XCUIApplication.launch()` does not reliably foreground the app on macOS.** The runner keeps focus, and a backgrounded app's window has an **empty accessibility tree** — so every element query fails with "does not exist" and the only cure is clicking the Dock icon. `BrewApp.launch` calls `activate()` and waits for `.runningForeground`. This is load-bearing, not cosmetic.
551+
- **Element-not-found failures carry a diagnosis.** `BrewUITestDiagnostics` reports whether the app is running, foregrounded, has a window, and which identifiers are actually in the tree. "Expected installed.screen to exist within 60s" is true and explains nothing; distinguishing "never opened a window" from "wrong identifier" is the difference between a five-minute fix and a day.
552+
- **Assertions gated on a subprocess use the command timeout, not the render timeout.** `DoctorReport.placeholder.isHealthy` is `false`, so while `brew doctor` is in flight the Doctor screen shows the redacted *issues* skeleton and the healthy text does not exist yet. Same for Configuration, whose cards only exist once `brew config` has been parsed.

CONVENTIONS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ UI in `Brew/` uses **semantic tokens** under [`Brew/Theme/`](Brew/Theme/) (`Brew
7373

7474
**Documentation:** Use [DocC](https://www.swift.org/documentation/docc/) / Xcode doc comments for non-obvious `public` / `internal` API. Inline `//` explains **why**, not **what**.
7575

76-
**Accessibility:** Meaningful labels (and hints where needed) on interactive controls; keyboard shortcuts where it matters. **UI test IDs:** shared constants in `Utilities/AccessibilityIdentifiers.swift` — see [`ARCHITECTURE.md`](ARCHITECTURE.md)**File organisation**; do not duplicate strings in the test target.
76+
**Accessibility:** Meaningful labels (and hints where needed) on interactive controls; keyboard shortcuts where it matters. **UI test IDs:** the `AXID` enum in [`Sources/BrewAccessibilityID/`](Sources/BrewAccessibilityID/), linked by both the app and `BrewUITests`. Attach it with `.axid(_:)`; never write a raw identifier string in a view or a test. Identity is orthogonal to labels — keep `accessibilityLabel` for VoiceOver.
7777

7878
**Testing:** Prefer [Swift Testing](https://developer.apple.com/documentation/testing/); XCTest is fine. **Never** invoke real `brew` in tests — mock/stub only **boundaries**: `BrewCommandRunning` (subprocess) and, when needed, `BrewExecutableLocating` (e.g. `MissingBrewExecutableLocator` for “brew not found”). Prefer **slice tests** that use the real `BrewInstalledPackagesRepository` (and thus real parsing) with those fakes; shared helpers live under [`BrewTests/TestSupport/`](BrewTests/TestSupport/). Pure presentation tests may use `InstalledViewModel`’s `init(testing…)` without a repository. Cover errors and async paths, not only happy paths. See [`ARCHITECTURE.md`](ARCHITECTURE.md) for layer flow.
7979

0 commit comments

Comments
 (0)