UI testing - accessibility IDs, seams and suite - #122
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a deterministic macOS UI test suite for BrewUI by adding a shared accessibility-ID module, formalizing UI-testing launch/fixture contracts, and adding production-inert seams for the two process boundaries (network + brew subprocess). It also wires UI tests into CI and adds local tooling/docs to run and debug the suite.
Changes:
- Add shared, dependency-free modules for UI-test identity (
BrewAccessibilityID/AXID) and UI-test launch contract + fixture payload (BrewUITestContract). - Add UI-testing seams in the app composition root to stub
URLSessiontraffic viaURLProtocoland to route allbrewinvocations through a single execution context (including UI-testing fake-brew wiring). - Add a 26-test Page Object Model UI test suite (
BrewUITests/), plusscripts/test-uiand a dedicated CI job.
Reviewed changes
Copilot reviewed 67 out of 68 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/BrewAccessibilityIDTests/AXIDTests.swift | Pins AXID raw identifier wire format with unit tests to catch drift early. |
| Sources/BrewUITestContract/BrewUITestingFixturePayload.swift | Defines UI-test launch environment keys and the encoded fixture payload contract. |
| Sources/BrewUIComponents/Views/View+AXID.swift | Adds a View modifier to attach AXID identifiers to SwiftUI views. |
| Sources/BrewUIComponents/Views/AsyncContentView.swift | Tags shared error chrome with AXID identifiers for UI testing. |
| Sources/BrewRepositories/BrewInstalledPackagesRepository.swift | Routes brew info through an injected execution context (supports UI testing). |
| Sources/BrewRepositories/BrewConfigRepository.swift | Routes brew config through an injected execution context (supports UI testing). |
| Sources/BrewNetworking/BrewAPIClient.swift | Adds a per-session URLProtocol seam for in-process network stubbing. |
| Sources/BrewFeatureInstalled/Views/UpgradesPackagesView.swift | Adds AXID identifiers for Upgrades screen, list, and rows. |
| Sources/BrewFeatureInstalled/Views/InstalledPackagesView.swift | Adds AXID identifiers for Installed screen, list, and rows. |
| Sources/BrewFeatureInstalled/Views/InstalledPackageDetailView.swift | Adds AXID identifiers for detail pane + uninstall/upgrade affordances. |
| Sources/BrewFeatureDoctor/Views/DoctorView.swift | Adds AXID identifier for Doctor screen root. |
| Sources/BrewFeatureDiscover/Views/DiscoverPackagesView.swift | Adds AXID identifiers for Discover screen + list (notes toolbar-search limitation). |
| Sources/BrewFeatureDiscover/Views/DiscoverPackageDetailView.swift | Adds AXID identifiers for detail pane + install button. |
| Sources/BrewFeatureConsole/Views/ConsoleToolbar.swift | Adds AXID identifier and explicit accessibility label for console toggle. |
| Sources/BrewFeatureConsole/Views/ConsoleStatusBar.swift | Combines status text for VO/UI testing and adds AXID identifiers/labels. |
| Sources/BrewFeatureConsole/Views/ConsolePanel.swift | Tags console container with AXID and makes it an accessibility container. |
| Sources/BrewFeatureConsole/Views/ConsoleBody.swift | Tags console output list with an AXID identifier. |
| Sources/BrewFeatureConfig/Views/ConfigView.swift | Adds AXID identifiers for Configuration screen + brew-not-found empty state. |
| Sources/BrewCLI/BrewCommandExecutionContext+UITesting.swift | Adds .uiTesting(brewURL:) execution context for fake-brew + safe “no brew” mode. |
| Sources/BrewAccessibilityID/AXID.swift | Introduces the shared AXID enum that defines all stable UI test identifiers. |
| scripts/test-ui | Adds a local script to run the UI test plan like CI. |
| Package.swift | Adds new SwiftPM products/targets and wires dependencies for BrewAccessibilityID. |
| Homebrew/Views/SidebarItem.swift | Adds explicit mapping from app sidebar destinations to test destinations (AXID). |
| Homebrew/Views/MainSidebarView.swift | Adds AXID identifiers for sidebar container and sidebar row items. |
| Homebrew/UITesting/BrewUITestingStubURLProtocol.swift | Implements a fixture-driven URLProtocol responder inside the app process. |
| Homebrew/UITesting/BrewUITestingLaunchConfiguration.swift | Parses UI-testing launch arguments/environment into a single config object. |
| Homebrew/UITesting/BrewUITestingFixtureInstaller.swift | Installs fixture payload into app-owned temp dir and publishes paths via env. |
| Homebrew/BrewApp.swift | Branches once for UI testing: installs fixtures, isolates caches, swaps seams. |
| Homebrew.xcodeproj/project.pbxproj | Links new SwiftPM products into app/UI test targets; adjusts build settings. |
| CONVENTIONS.md | Updates documented convention: use AXID + .axid(_:) and never raw ID strings. |
| BrewUITests/TROUBLESHOOTING.md | Documents platform/environment gotchas and debugging guidance for macOS UI tests. |
| BrewUITests/Tests/UninstallUITests.swift | UI tests for uninstall flow and inventory reconciliation. |
| BrewUITests/Tests/NavigationUITests.swift | UI tests ensuring each sidebar destination loads and navigation is stable. |
| BrewUITests/Tests/LaunchSmokeUITests.swift | Smoke tests that -uiTesting launch and empty scenario behave correctly. |
| BrewUITests/Tests/InstallUITests.swift | UI tests for Discover search + install + Installed reconciliation path. |
| BrewUITests/Tests/InstalledUITests.swift | UI tests for installed inventory rendering, search, upgrades slice, large output. |
| BrewUITests/Tests/ErrorStateUITests.swift | UI tests for error surfaces across HTTP failures, decode failures, missing brew. |
| BrewUITests/Tests/DoctorUITests.swift | UI tests for Doctor healthy + warnings (non-zero exit treated as data). |
| BrewUITests/Tests/ConsoleUITests.swift | UI tests for streamed console output and success/failure status reporting. |
| BrewUITests/Tests/ConfigUITests.swift | UI tests for parsed brew config entries. |
| BrewUITests/Screens/UpgradesScreen.swift | Page object for Upgrades screen. |
| BrewUITests/Screens/Sidebar.swift | Page object for sidebar navigation returning loaded screens. |
| BrewUITests/Screens/Screen.swift | Base screen protocol + shared chrome assertions/actions (sidebar/console/error). |
| BrewUITests/Screens/PackageDetailScreen.swift | Page object for detail pane actions (install/uninstall/upgrade). |
| BrewUITests/Screens/InstalledScreen.swift | Page object for Installed screen. |
| BrewUITests/Screens/DoctorScreen.swift | Page object for Doctor screen assertions. |
| BrewUITests/Screens/DiscoverScreen.swift | Page object for Discover screen + search behavior. |
| BrewUITests/Screens/ConsoleScreen.swift | Page object for console expand/collapse and output/status assertions. |
| BrewUITests/Screens/ConfigScreen.swift | Page object for Configuration screen assertions. |
| BrewUITests/Harness/FakeBrew.swift | Builds the fake brew dispatcher + fixture payload for scenarios. |
| BrewUITests/Harness/BrewUITestScenario.swift | Enumerates the suite’s deterministic scenarios. |
| BrewUITests/Harness/BrewUITestDiagnostics.swift | Improves failure logs with foreground/window/tree diagnostics. |
| BrewUITests/Harness/BrewUITestCase.swift | Base XCTest case with launch helpers and process cleanup. |
| BrewUITests/Harness/BrewApp.swift | Launches app with payload/env and applies macOS foreground/window workaround. |
| BrewUITests/Fixtures/ScenarioFixtures.swift | Provides per-scenario fixture sets for both HTTP and fake-brew boundaries. |
| BrewUITests/Fixtures/FixturePackage.swift | Single-source package metadata rendered into each wire format fixture. |
| BrewUITests/Elements/BrewUIStaticText.swift | Element wrapper for stable text assertions (label/value handling). |
| BrewUITests/Elements/BrewUISearchField.swift | Special-cased toolbar search field wrapper (no AXID possible currently). |
| BrewUITests/Elements/BrewUIList.swift | List wrapper providing row lookup + row count assertions by AXID prefix. |
| BrewUITests/Elements/BrewUIElement.swift | Core element wrapper enforcing self-waiting and better diagnostics. |
| BrewUITests/Elements/BrewUICell.swift | Row wrapper for clicking token-addressed list rows. |
| BrewUITests/Elements/BrewUIButton.swift | Button wrapper using macOS click() and hittability waits. |
| BrewUITests/BrewUITestsLaunchTests.swift | Removes template UI test class in favor of the new suite. |
| BrewUITests/BrewUITests.swift | Removes template UI test class in favor of the new suite. |
| Brew-UI.xctestplan | Configures UI test plan options (screenshots, random ordering, timeouts). |
| .gitignore | Ignores UI test result bundles and coverage .profraw artifacts. |
| .github/workflows/pr_build_test.yml | Adds a dedicated CI job to run UI tests via scripts/test-ui. |
| .ai/memory.md | Records durable decisions/patterns introduced by the UI testing plan. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
graeme
force-pushed
the
ui-tests-phase-2
branch
from
August 9, 2026 08:47
ee3ee13 to
b4b58ab
Compare
UI testing needs two things spelled exactly once, in a place both the app and the test bundle can see: the identifier of every testable element, and the launch contract between the two processes. Both live in dependency-free SwiftPM targets so linking them into the test bundle cannot drag app code along with it. BrewAccessibilityID holds AXID, whose rawValue is the only place an identifier string is written. Views attach it with .axid(_:) rather than accessibilityIdentifier with a literal, and AXIDTests pins the wire format so drift breaks a unit test instead of surfacing as a missing element in a UI test. BrewUITestContract holds the launch environment keys and the fixture payload the two processes exchange. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Attaches AXID across the app: the sidebar and its rows, the five primary screens, the package detail views, the console, and the shared failure chrome in AsyncContentView. Two choices worth knowing about. The error state is deliberately screen-agnostic, because every loadable surface renders the same view, so a test scopes the query to a screen root to say which surface failed. And Upgrades rows carry their own identity rather than reusing the Installed ones: the same package is legitimately in both lists at once, and sharing ids would let an assertion about one be satisfied by the other. Identity stays orthogonal to labels throughout. accessibilityLabel remains for VoiceOver; these identifiers exist only so tests never match on displayed text or index. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two process boundaries, both public, both inert in production. URLSessionBrewAPIClient.stubbed(protocolClasses:) sets protocol classes on one ephemeral session's configuration rather than calling URLProtocol.registerClass, which would also capture .shared and therefore every other client in the process. BrewCommandExecutionContext.uiTesting(brewURL:) uses the real BrewCommandService against a fake executable, so Process spawning, pipes, output streaming and exit handling all stay under test. Deliberately not LoginShellBrewCommandRunner: wrapping a fake brew in the developer's login shell would re-introduce the dotfile dependence that runner exists to provide in production. A nil URL resolves nothing, which is how a test drives the brew-not-found surfaces. BrewInstalledPackagesRepository and BrewConfigRepository now take an execution context instead of each constructing its own runner and locator, so one context covers every brew invocation in the process rather than just the command center's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BrewApp.init() makes one check, on the launch argument, and every seam is a single guard away from the untouched production wiring. Being in UI-test mode is never threaded further into the app. Under that flag the app writes the run's fixture tree into its own temporary directory from the payload in its launch environment. The alternative -- the test runner writes files and the app reads them -- needs one directory two processes are both allowed to use, and there is no reliable one: a path the runner can write is not necessarily one the app can read and execute from, and the failure is an opaque EPERM on whichever side loses. Letting one process create, execute and own the files removes the question. The installer setenvs the resulting paths because the stub URLProtocol reads them from a URLSession loading thread and the fake brew inherits them as a subprocess. BrewUITestingStubURLProtocol answers from those fixtures keyed by request path, so the real client still builds requests, negotiates ETag/304, decodes and caches; an error case is a fixture rather than a code path. The on-disk caches are redirected to a per-run container so one run cannot decide the next run's behaviour or overwrite a real install's cache. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Elements are the only layer that touches XCUIElement. Every operation self-waits, because a bare exists read races the app's next render and is the largest single source of XCUITest flake, and failures are anchored to the caller so a red test points at the assertion rather than into the wrapper. Screens own where things are; tests own what should be true, and actions return the next screen so an illegal sequence fails to compile. FakeBrew is a lookup table rather than a case over subcommands: stdout, stderr and exit-code files are all optional, so adding a command to a scenario means adding a file. A .next-info file is its only state, and it is what makes an uninstall actually remove a row -- the repository reconciles off the command centre's completion stream and has to see a changed world. The suite covers navigation, Installed, install, uninstall, config, doctor and console, plus the error cases this whole approach exists for: a 500 through the real API client, output the real decoder refuses, a non-zero exit from the real runner, and a locator that resolves nothing. Failures report whether the app is running, foregrounded and has a window, since "element not found" cannot otherwise distinguish a launch problem from a wrong identifier. Replaces the Xcode template, which launched the app with no -uiTesting argument and so ran against the real network and the machine's real brew. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 67 out of 68 changed files in this pull request and generated no new comments.
Suppressed comments (11)
.github/workflows/pr_build_test.yml:39
- The pull-request filter has the same gap: changes confined to
Homebrew/**orBrew-UI.xctestplanwill not trigger this workflow, so the new UI-test job may not run when app UI or test-plan behavior changes.
- "scripts/test-ui"
.github/workflows/pr_build_test.yml:19
- The new UI-test job can still be skipped for app-only or test-plan-only pushes: this filter watches
Brew/**, but the app sources are underHomebrew/**, andBrew-UI.xctestplanis not listed. Add both paths so changes to the code under test or its plan run the suite.
This issue also appears on line 39 of the same file.
- "scripts/test-ui"
Sources/BrewUIComponents/Views/AsyncContentView.swift:72
- This identifier is attached to a button that the parent later collapses with
.accessibilityElement(children: .combine). The Retry button therefore is not preserved as a separately queryable/clickableerror.retryelement, soScreen.retry()cannot address it and VoiceOver loses the distinct Retry control. Keep the button outside the combined element or use a containing structure that preserves interactive children.
BrewUITests/Elements/BrewUIList.swift:51 - This assertion reads the query count only once instead of self-waiting. While
Installedis loading,AsyncContentViewrenders four identified placeholder rows, so the empty-inventory smoke test can fail on a slower machine before the real zero-row result arrives. Poll the row query until its count matches or the timeout expires.
func assertCount(
_ expected: Int,
file: StaticString = #filePath,
line: UInt = #line,
) -> Self {
XCTAssertEqual(count, expected, "Unexpected row count in \(id.rawValue)", file: file, line: line)
BrewUITests/Tests/LaunchSmokeUITests.swift:27
- Neither assertion proves that the async inventory load reached
.loaded: the no-error check succeeds immediately during.loading, and a later.failedstate also has zero package rows. Depending on timing, this test can pass for the failure it explicitly intends to reject. Add an explicit loaded-success accessibility signal and wait for it before checking zero rows/no error.
BrewUITests/Tests/ErrorStateUITests.swift:75 - This negative assertion runs as soon as the screen root appears, which is before the asynchronous inventory request settles, so it can pass and end the test before a later error renders. Gate it on a known package first so the test proves the healthy load completed.
BrewUITests/Tests/ConsoleUITests.swift:27 - These final-state assertions do not prove streaming. The fake
brewuses onecatand exits immediately, andassertOutputContainsmay wait until after exit; an implementation that buffers everything until termination would still render both rows and pass. Add a delayed/gated fixture and assert the first line while the command is still running before allowing it to finish.
BrewUITests/Fixtures/ScenarioFixtures.swift:259 - These ETag files do not currently exercise the 304 branch. Each launch creates a new cache directory and clears the UI-test defaults, and after the initial 200 response the catalogue remains fresh, so no test sends a second request with
If-None-Match. Add a pre-seeded-cache scenario or force a refresh after the first fetch before claiming 304 coverage.
// A stable ETag on the catalogue so a second launch in the same run exercises the real
// client's If-None-Match / 304 path rather than always taking the 200 branch.
"api_formula.json.etag": text("\"formula-fixture\""),
"api_cask.json.etag": text("\"cask-fixture\""),
Homebrew/UITesting/BrewUITestingStubURLProtocol.swift:11
- This documents the discarded on-disk handoff design.
BrewUITestingFixtureInstallernow decodes the launch-environment payload and writes the fixture files from the app process during launch, not from the test process before launch.
BrewUITests/Harness/BrewUITestScenario.swift:12 - The test process no longer materializes fixtures on disk. It sends an encoded payload, and the app installs that tree into its own temporary directory during launch; update this contract description so future harness changes follow the implemented boundary.
/// A scenario names one fixture set, which the test process materialises to disk before launch and
/// which both process boundaries then read: the fake `brew` serves `<scenario>/brew`, and the app's
/// stubbed `URLSession` serves `<scenario>/http`. Because both seams are data, an error case is a
.ai/memory.md:535
- This durable memory entry records the superseded design and even names a nonexistent
FakeBrew.install(scenario:). The current implementation hasFakeBrew.payload(for:)build bytes thatBrewUITestingFixtureInstallerwrites from the app process; correct this entry because.ai/memory.mdis the repository's long-term source of truth.
- **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.
graeme
force-pushed
the
ui-tests-phase-2
branch
2 times, most recently
from
August 9, 2026 09:07
16b7a29 to
a76d9a0
Compare
MikeMcQuaid
approved these changes
Aug 9, 2026
scripts/test-ui mirrors what CI runs, so green locally means green on CI. It deliberately does not override -derivedDataPath: a project-relative DerivedData location reproducibly makes the test runner hang for around five minutes before failing to establish a connection, where the identical run against the default location passes in seconds. The ui-test CI job runs alongside the unit job with no needs: dependency, so UI coverage costs wall-clock time on a separate runner rather than sitting on the critical path. It replaces ui_smoke.yml, which triggered on the same pull_request event and ran the same Brew-UI scheme, so keeping both would run every test twice. Its inline signing block is gone with it: the default path was the ad-hoc signing scripts/test-ui already does, and keeping the logic in the script is what keeps a local run honest about CI. TROUBLESHOOTING.md covers the things that make a local run fail for reasons unrelated to the code, since the suite drives real windows and real input. Also ignores the artefacts a UI run leaves behind: coverage drops .profraw wherever an instrumented binary runs, and the script writes a result bundle at the repository root. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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>
actions/checkout leaves a token in .git/config by default, and this job uploads artifacts on failure, so the credential can ride along. Nothing here pushes, so dropping it costs nothing. Flagged by zizmor; the rest of the repository's workflows already set this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Lets the workflow be run against any branch from the Actions tab, and takes an optional -only-testing filter so a single UI test can be re-run without pushing a commit. The filter reaches the script through the environment rather than being interpolated into the run block, so the input cannot inject shell. The concurrency group gains a ref fallback: a dispatched run has no pull request number, so without it every manual run would share one group and cancel whichever was already going. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
graeme
force-pushed
the
ui-tests-phase-2
branch
from
August 9, 2026 12:18
a76d9a0 to
592ff07
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR: UI testing - identity, process-boundary seams, and a deterministic suite
Summary
Takes the app from no UI tests to a 26-test deterministic suite wired into CI. Only the two process
boundaries are mocked (network via a stubbed
URLProtocol, shell via a fakebrew), so all of ourown code runs under test, error paths included.
Changes
Identity
BrewAccessibilityID, a dependency-free target linked by both the app andBrewUITests, spellseach identifier once.
AXIDTestspins the wire format, so drift breaks a unit test.Seams, both inert in production
URLSessionBrewAPIClient.stubbed(protocolClasses:)sets protocol classes on one ephemeralsession, never globally, which would also capture
.shared.BrewCommandExecutionContext.uiTesting(brewURL:)runs the realBrewCommandServiceagainst afake executable, without the login shell wrapper, for every brew invocation in the process.
BrewApp.init()branches once, on-uiTesting, and redirects on-disk caches per run so runscannot affect each other or a real install.
Suite (
BrewUITests/)Elements/is the only layer touchingXCUIElement, and every operation self-waits.Screens/returns the next screen from each action, so illegal navigation fails to compile.
FakeBrewis a fixture-table bash dispatcher across 10 scenarios, delivered in the launchenvironment and written to disk by the app.
error cases this approach exists for: a 500 through the real API client, output the real decoder
refuses, a non-zero exit, and a locator that resolves nothing.
Tooling
scripts/test-ui, a parallelui-testCI job, andBrewUITests/TROUBLESHOOTING.md.Testing
scripts/test-ui- 26 UI tests passscripts/test- 636 + 19 unit tests pass--strict, BrewUILint cleanPR checklist
Claude Code wrote the module, seams and suite; verification was a full
scripts/test-uirun on areal machine, so every assertion is backed by a passing run, not inspection.
Follow-ups
Brew.rawand the live plan land next.