From c7ecc49f88c9313ea5af6dda7f21812e3cca1b45 Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Wed, 19 Aug 2026 08:49:52 +0200 Subject: [PATCH 1/2] feat(macos): Add the UI test suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The app's unit tests run inside the app process, which puts three things out of reach: whether a menu item is enabled, what landed on the pasteboard, and what survives a terminate and relaunch. Those are exactly the behaviours that break without anyone noticing, and `QA.md` carried them as a manual checklist. This is the half of that checklist a machine can run. A UI test drives the app through its accessibility tree from a separate process, so `@testable import JP` is unavailable and must not be reached for. Anything checkable in-process belongs in `JPTests`, where it costs milliseconds instead of an app launch. Three things make the suite fast enough to be worth having. A suite shares one launched app, because launching costs about four seconds and the work under test costs milliseconds — so a test leaves the app as it found it, or the suite is ordered so that what one test leaves is what the next expects. Waits are on conditions rather than the clock, and not through `waitForExistence`, which reports an element about a second after it appears whatever the element; asking again finds it in under 100ms, and the timeout becomes the price of a failure rather than of a pass. And every animation goes through one lever the tests turn off, because XCUITest waits for the app to stop moving before each action it synthesizes. Anything the tests need the app to do differently goes through `DebugState`, gated on `#if DEBUG` and off unless an environment variable says otherwise, so a release build has no way to reach it. The pasteboard works this way and is enforced: no test may touch the system pasteboard, a debug build copies wherever `JP_DEBUG_PASTEBOARD` names, and `ClipboardPolicyTests` fails on any spelling of the general one. `just test-app-ui` is the CI job and runs everything even after a failure. While writing a test, run it by name instead. `fmt-app` and `lint-app` arrive here rather than with the app because they name every Swift directory in the repository, and this is the last one to exist. Signed-off-by: Jean Mertz --- apps/macos/UITests/AppUnderTest.swift | 566 ++++++++++++++++++ apps/macos/UITests/ConversationFixtures.swift | 117 ++++ .../macos/UITests/ConversationListTests.swift | 242 ++++++++ apps/macos/UITests/Diagnostics.swift | 69 +++ .../UITests/PinnedConversationTests.swift | 42 ++ apps/macos/UITests/PointerCursorTests.swift | 84 +++ apps/macos/UITests/Quiescence.swift | 96 +++ apps/macos/UITests/SharedApp.swift | 107 ++++ .../macos/UITests/TranscriptReflowTests.swift | 126 ++++ apps/macos/UITests/Transcripts.swift | 36 ++ apps/macos/UITests/UISuite.swift | 13 + apps/macos/UITests/WorkspaceFixture.swift | 273 +++++++++ apps/macos/project.yml | 23 + justfile | 30 + 14 files changed, 1824 insertions(+) create mode 100644 apps/macos/UITests/AppUnderTest.swift create mode 100644 apps/macos/UITests/ConversationFixtures.swift create mode 100644 apps/macos/UITests/ConversationListTests.swift create mode 100644 apps/macos/UITests/Diagnostics.swift create mode 100644 apps/macos/UITests/PinnedConversationTests.swift create mode 100644 apps/macos/UITests/PointerCursorTests.swift create mode 100644 apps/macos/UITests/Quiescence.swift create mode 100644 apps/macos/UITests/SharedApp.swift create mode 100644 apps/macos/UITests/TranscriptReflowTests.swift create mode 100644 apps/macos/UITests/Transcripts.swift create mode 100644 apps/macos/UITests/UISuite.swift create mode 100644 apps/macos/UITests/WorkspaceFixture.swift diff --git a/apps/macos/UITests/AppUnderTest.swift b/apps/macos/UITests/AppUnderTest.swift new file mode 100644 index 000000000..c884fcd80 --- /dev/null +++ b/apps/macos/UITests/AppUnderTest.swift @@ -0,0 +1,566 @@ +import Foundation +import Testing +import XCTest + +/// Stable names for the elements this suite reaches for. +/// +/// Deliberately spelled out rather than shared with the app's +/// `AccessibilityID`: these identifiers are the contract an external driver +/// holds the app to, documented in `AFFORDANCES.md`. A suite that imported the +/// constants would follow a rename instead of catching one, and a UI test runs +/// in another process anyway. +/// +/// `AccessibilityIDTests` pins the same strings from inside the app. +/// +/// Only the names this suite uses are here. The rest of the table stays out +/// until a test drives the state that shows it, so every name in this file is +/// one something depends on. +enum ID { + static let sidebarList = "sidebar.list" + static let sidebarFilter = "sidebar.filter" + static let sidebarFilterClear = "sidebar.filter.clear" + static let transcriptScroll = "transcript.scroll" + static let transcriptText = "transcript.text" + static let windowDivider = "window.divider" + + static func sidebarRow(_ conversationID: String) -> String { + "sidebar.row.\(conversationID)" + } + +} + +/// The app, launched against a fixture and driven from outside its process. +/// +/// Isolation is by environment, with one exception the environment cannot +/// reach: window state saved by `@SceneStorage` is keyed by bundle identifier, +/// and a UI test drives the developer's own build under the developer's own +/// identifier. `-ApplePersistenceIgnoreState` is the lever that leaves it +/// alone — the app neither restores what was saved nor saves what it had. +/// +/// A test of state restoration is the one case that needs the opposite, and +/// passes `keepingWindowState: true` knowingly. +/// +/// ## What a test costs +/// +/// A synthesized pointer event — a click, a double-click, a right-click, or +/// opening a menu-bar menu — costs 400-500ms. A key event costs ~50ms and +/// resolving an element ~40ms, so the pointer path is an order of magnitude +/// dearer than anything else a test does, and it dominates the run. +/// +/// None of it is the app. Timestamps on both sides put the app's own work 71ms +/// *after* `click()` has already returned, and the work itself at 2-4ms: XCTest +/// spends the 400ms before the event is delivered, so nothing the app does or +/// stops doing changes it. Turning off the post-event idle wait (see +/// ``Quiescence``) buys about 30ms of it and there is no second knob. +/// +/// What does move is the size of the accessibility tree, at roughly 0.3ms per +/// element per event. This fixture publishes ~230 elements, which is ~70ms of +/// each click; a fixture of 300 conversations publishes ~1130 and makes every +/// pointer event half again as expensive. Size a fixture for what the test +/// needs to say, not for realism. +/// +/// So: prefer a key event to a pointer event wherever the affordance allows, +/// and reach for a pointer event only where the pointer *is* what is under +/// test. +@MainActor +struct AppUnderTest { + let app: XCUIApplication + + /// How long to wait for the app to read its workspace and draw a list. + /// + /// Wider than ``timeout`` because it covers process start, not just work + /// the running app does. + static let launchTimeout: TimeInterval = 10 + + /// How long to wait for anything the app does once it is up. + /// + /// Deliberately short. Every wait here is on a condition rather than on the + /// clock, so a passing test returns the moment the element appears and this + /// number costs it nothing — it is the price of a *failure*, paid once per + /// broken assertion, and ten seconds of that is ten seconds of a red loop + /// spent watching a spinner. + /// + /// One second is far longer than anything the app does in reply to a click: + /// the workspace is already open by then, and reading a conversation of + /// four events is a file read. Raise it for a specific wait that genuinely + /// covers slower work rather than raising it here. + static let timeout: TimeInterval = 1 + + /// Launch against `fixture` and wait until the conversation list is on + /// screen. + /// + /// Waiting here rather than in each test is what keeps a test from acting on + /// a window that has not finished reading, which reads as an intermittent + /// failure rather than as the race it is. + static func launch( + against fixture: WorkspaceFixture, + keepingWindowState: Bool = false, + sourceLocation: SourceLocation = #_sourceLocation + ) -> AppUnderTest { + // Before the first event is synthesized, and reported rather than + // shrugged off: a suite quietly back to waiting after every event is a + // suite nobody notices has slowed down. + if let failure = Quiescence.installation { + let message = "the quiescence waits could not be turned off: \(failure)" + Diagnostics.append("\(sourceLocation.fileName):\(sourceLocation.line): \(message)") + Issue.record("\(message)", sourceLocation: sourceLocation) + } + + let app = XCUIApplication() + app.launchEnvironment = fixture.environment + if !keepingWindowState { + app.launchArguments = ["-ApplePersistenceIgnoreState", "YES"] + } + app.launch() + + let driven = AppUnderTest(app: app) + _ = driven.wait(for: driven.sidebar, timeout: launchTimeout) + + // Recorded once the app is up, so a run stopped part-way can still + // close it. Nothing else can: the app outlives the process that stops + // the run. See ``Diagnostics/processes``. + if let pid = fixture.appProcessID { + Diagnostics.recordAppProcess(pid) + } + + return driven + } + + /// Wait for `element` to exist, asking as often as asking costs. + /// + /// `XCUIElement.waitForExistence` reports an element about a second after + /// it appears, whatever it is: the two transcript waits in this suite + /// measured 1131ms and 1117ms against an app that draws them in tens of + /// milliseconds, and the number barely moves with the work involved. + /// + /// Resolving an element costs about 30ms, so a loop that simply asks again + /// polls at roughly 30Hz and finds it an order of magnitude sooner. No + /// sleep, and none needed: the query is what paces the loop. + func wait(for element: XCUIElement, timeout: TimeInterval = AppUnderTest.timeout) -> Bool { + let deadline = Date().addingTimeInterval(timeout) + + repeat { + if element.exists { + return true + } + } while Date() < deadline + + return false + } + + func terminate() { + app.terminate() + } + + /// The window showing the workspace, addressed by the title it carries. + /// + /// By title rather than `windows.firstMatch`, because a test that opened a + /// conversation window leaves it behind for the next one and first is not + /// the same as the workspace's. + func workspaceWindow(_ fixture: WorkspaceFixture) -> XCUIElement { + app.windows.element(matching: NSPredicate(format: "title BEGINSWITH %@", fixture.name)) + } + + /// Close the window titled `title`, if it is open. + /// + /// Tests that open a window close it again, so the next one starts from the + /// same arrangement it would have found on its own. + /// + /// Command-W rather than the close button, because a synthesized pointer + /// event costs around 400ms and a key event around 50. It acts on whichever + /// window is in front, which is why the title is checked afterwards instead + /// of aimed at: a Command-W arriving while the workspace window was in front + /// would close *that*, and every test after it would fail somewhere far away + /// from the cause. + func closeWindow( + titled title: String, + sourceLocation: SourceLocation = #_sourceLocation + ) { + let window = app.windows[title] + guard window.exists else { return } + + app.typeKey("w", modifierFlags: .command) + + guard waitForDisappearance(of: window) else { + record( + """ + the window titled "\(title)" was still open after Command-W, so \ + the key window was something else and that is what closed. \ + On screen: \(capture("stuck window \(title)")) + """, + sourceLocation: sourceLocation + ) + return + } + } + + /// Wait for `element` to stop existing, asking as often as asking costs. + /// + /// The counterpart to ``wait(for:timeout:)``, and paced the same way. + func waitForDisappearance( + of element: XCUIElement, + timeout: TimeInterval = AppUnderTest.timeout + ) -> Bool { + let deadline = Date().addingTimeInterval(timeout) + + repeat { + if !element.exists { + return true + } + } while Date() < deadline + + return false + } + + // Every accessor below names an element type and starts from the narrowest + // root it can. An untyped `descendants(matching: .any)` reads as convenient + // and costs a full snapshot of the app's accessibility tree on each + // evaluation, which is most of what a test spends its time on. The types + // are what the app actually publishes, read off a running instance with + // `debug_app_snapshot`. + + /// The conversation list, which exists only once the workspace is read. + /// + /// A SwiftUI `List` in a sidebar is an `NSOutlineView`. + var sidebar: XCUIElement { + app.outlines[ID.sidebarList] + } + + /// The box that narrows the conversation list. + var filter: XCUIElement { + app.textFields[ID.sidebarFilter] + } + + /// The button that empties the filter box, which exists only while the box + /// holds something. + var filterClear: XCUIElement { + app.buttons[ID.sidebarFilterClear] + } + + /// The scrolling transcript, which exists only once a conversation is read. + var transcript: XCUIElement { + app.scrollViews[ID.transcriptScroll] + } + + /// The row showing `conversation`. + /// + /// A row's identifier sits on the leaf inside its cell rather than on the + /// row, because the view carrying it collapses to one element. That leaf + /// reports no role of its own, which is why this asks for `.other` rather + /// than for a cell or a row. + func row(_ conversation: FixtureConversation) -> XCUIElement { + sidebar.descendants(matching: .other)[ID.sidebarRow(conversation.id)] + } + + /// The strip between the panes that resizes the sidebar. + /// + /// `.any` rather than a role, because the view reports none of its own: it is + /// a shape made into an accessibility element, and arrives as `AXUnknown`. + var divider: XCUIElement { + app.descendants(matching: .any)[ID.windowDivider] + } + + /// Wait until the system is displaying `cursor`. + /// + /// `NSCursor.currentSystem` reads what the window server is showing rather + /// than what this process asked for, so a test in another process can see the + /// cursor the app under test caused. Compared by image bytes: the accessor + /// hands back a fresh instance each time, so identity says nothing, and two + /// standard cursors differ in their pixels. + /// + /// Polled rather than read once, because the window server sets the cursor a + /// moment after the pointer arrives. + func waitForCursor( + _ cursor: NSCursor, + timeout: TimeInterval = AppUnderTest.timeout + ) -> Bool { + let deadline = Date().addingTimeInterval(timeout) + + repeat { + if cursorIs(cursor) { + return true + } + } while Date() < deadline + + return false + } + + /// Whether the system is showing `cursor` right now. + /// + /// Compared by image bytes, and only ever used to ask about a cursor the test + /// is looking *for*. Not every standard cursor can be recognised this way — + /// `NSCursor.arrow.image` does not match the bytes the system reports while + /// showing the arrow — so a test that needs a baseline asks whether the + /// cursor is *not* the one it expects next, rather than trying to name what it + /// currently is. + func cursorIs(_ cursor: NSCursor) -> Bool { + guard let current = NSCursor.currentSystem?.image.tiffRepresentation else { + return false + } + + return current == cursor.image.tiffRepresentation + } + + /// The cursor the system is showing, named against the standard ones. + /// + /// For a failure message: an `NSCursor`'s own description is a pointer + /// address, which says only that it was not the expected one. + func describeCursor() -> String { + guard let current = NSCursor.currentSystem?.image.tiffRepresentation else { + return "a cursor the system would not report" + } + + let known: [(String, NSCursor)] = [ + ("the arrow", .arrow), + ("the I-beam", .iBeam), + ("the pointing hand", .pointingHand), + ("the open hand", .openHand), + ("the column-resize cursor", .columnResize), + ("the row-resize cursor", .rowResize), + ("the left-right resize cursor", .resizeLeftRight), + ] + + let match = known.first { $0.1.image.tiffRepresentation == current } + return match?.0 ?? "a cursor matching none of the standard ones" + } + + /// The text the transcript is drawn as. + /// + /// The whole conversation is one text view, so there is no element per + /// message. Its value is every message it is showing, which is how a test + /// asserts on what is on screen. + var transcriptText: XCUIElement { + app.textViews[ID.transcriptText] + } + + /// Wait until a transcript shows exactly `text`. + /// + /// Exactly, and against the whole document rather than a phrase inside it: the + /// value of the text view is every message it is showing, so a substring match + /// would survive the speaker labels going missing, the messages arriving in the + /// wrong order, or a second copy of the conversation being appended. + /// + /// `within` scopes the search to one window, which is how a conversation pulled + /// into its own window is told apart from the workspace window behind it. The + /// identifier is the same in both. + /// + /// Polled rather than read once, because a transcript arrives a moment after + /// the row is clicked. + @discardableResult + func expectTranscript( + _ text: String, + _ description: String, + within scope: XCUIElement? = nil, + timeout: TimeInterval = AppUnderTest.timeout, + sourceLocation: SourceLocation = #_sourceLocation + ) -> Bool { + let element = (scope ?? app).textViews[ID.transcriptText] + let deadline = Date().addingTimeInterval(timeout) + var last: String? + + repeat { + last = element.exists ? element.value as? String : nil + if last == text { + return true + } + } while Date() < deadline + + let shot = capture(description) + record( + """ + \(description) never showed the expected transcript within \(timeout)s. \ + Showing instead: \(last.map { "\($0.debugDescription)" } ?? "no transcript at all"). \ + On screen: tmp/uitests/\(shot) + """, + sourceLocation: sourceLocation + ) + return false + } + + /// Open the menu-bar menu titled `title` and return the menu it drops down, + /// so its items can be read. + /// + /// The dropped-down menu rather than the bar item, because it is the root + /// every item below is addressed from. A title is not unique across the + /// app: Copy Link is both an Edit-menu item and a context-menu item, and + /// AppKit publishes both to the accessibility tree whether or not either + /// menu is open, so a query starting at the app can return the wrong one. + /// Starting at the menu cannot. + /// + /// macOS populates a menu when it is opened, so an item's presence and + /// enablement still cannot be read from a closed one. + @discardableResult + func openMenu(_ title: String) -> XCUIElement { + let bar = app.menuBars.menuBarItems[title] + _ = wait(for: bar) + bar.click() + return bar.menus.firstMatch + } + + /// Close whatever menu is open, by pressing Escape. + func closeMenu() { + app.typeKey(.escape, modifierFlags: []) + } + + /// Open the menu-bar menu `menu` and click `item` in it. + func chooseMenuItem( + _ item: String, + in menu: String, + sourceLocation: SourceLocation = #_sourceLocation + ) { + let entry = openMenu(menu).menuItems[item] + guard + expectAppears(entry, "\(item) in the \(menu) menu", sourceLocation: sourceLocation) + else { return } + + entry.click() + } + + /// Click `item` in the context menu that is open. + /// + /// A context menu has no handle to start from the way a menu-bar menu does, + /// so this picks between same-titled items by hittability: only the items + /// of an open menu are hittable, and the context menu is the one that is + /// open. Getting it wrong is worth avoiding rather than merely detecting — + /// the menu-bar twin of a context item is usually disabled, so the click + /// lands and silently does nothing, which looks exactly like the app + /// ignoring the menu. + func chooseContextMenuItem( + _ item: String, + sourceLocation: SourceLocation = #_sourceLocation + ) { + let matches = app.menuItems.matching(identifier: item) + _ = wait(for: matches.firstMatch) + + guard let entry = matches.allElementsBoundByIndex.first(where: \.isHittable) else { + record( + """ + no open menu holds an item titled "\(item)": \ + \(matches.count) match it, none of them on screen. \ + On screen instead: \(capture(item)) + """, + sourceLocation: sourceLocation + ) + return + } + + entry.click() + } + + /// Whether `menu` holds an item titled `item`. + /// + /// Presence, not enablement: an item AppKit injects can be there and greyed + /// out — Merge All Windows is, until there is a second window to merge — + /// and it is the presence that says the menu was not rebuilt out from under + /// it. + func menuItemExists(_ item: String, in menu: XCUIElement) -> Bool { + menu.menuItems[item].exists + } + + /// Whether `menu`'s `item` can be chosen. + func menuItemIsEnabled(_ item: String, in menu: XCUIElement) -> Bool { + let entry = menu.menuItems[item] + return entry.exists && entry.isEnabled + } + + /// Wait for `element`, recording what was on screen instead when it never + /// arrives. + /// + /// A bare `#expect(element.exists)` reports only that something was + /// missing, which is the least useful half of the story: the app was + /// showing *something*, and what it was showing is usually the whole + /// answer. This writes that screen to a PNG and names the file in the + /// failure. + /// + /// The path rather than the image, because a tool result is text all the + /// way to the assistant reading it. Attach the file to say what it shows. + @discardableResult + func expectAppears( + _ element: XCUIElement, + _ description: String, + timeout: TimeInterval = AppUnderTest.timeout, + sourceLocation: SourceLocation = #_sourceLocation + ) -> Bool { + if wait(for: element, timeout: timeout) { + return true + } + + let shot = capture(description) + record( + "\(description) never appeared within \(timeout)s. On screen instead: tmp/uitests/\(shot)", + sourceLocation: sourceLocation + ) + return false + } + + /// Wait for `element` to go away, recording what is still on screen when it + /// does not. + /// + /// The counterpart to ``expectAppears(_:_:timeout:sourceLocation:)``, for + /// the assertions that say something was torn down rather than built. + @discardableResult + func expectDisappears( + _ element: XCUIElement, + _ description: String, + timeout: TimeInterval = AppUnderTest.timeout, + sourceLocation: SourceLocation = #_sourceLocation + ) -> Bool { + if waitForDisappearance(of: element, timeout: timeout) { + return true + } + + let shot = capture(description) + record( + "\(description) never went away within \(timeout)s. On screen: tmp/uitests/\(shot)", + sourceLocation: sourceLocation + ) + return false + } + + /// Record a failure, in both places a reader might look. + /// + /// `Issue.record` alone is not enough under `xcodebuild`, which prints the + /// header naming the *kind* of issue and drops the message explaining it: + /// a run of ten failures arrives as ten identical `Issue recorded` lines. + /// So the message also goes to a file `swift_test_ui` collects. That is + /// also what lets the tool stop the run: it watches for the failure, and + /// the message it reports afterwards comes from here rather than from + /// output that was cut off mid-write. + func record(_ message: String, sourceLocation: SourceLocation) { + Diagnostics.append("\(sourceLocation.fileName):\(sourceLocation.line): \(message)") + Issue.record("\(message)", sourceLocation: sourceLocation) + } + + /// Write what the app is showing to a PNG, and return its file name. + /// + /// The name rather than the path: the file is written into the runner's + /// container, and `swift_test_ui` copies it into `tmp/uitests/` under the + /// same name. Naming the container path here would give a reader a path + /// that is longer and gone by the next run. + /// + /// Returns why it could not be written rather than throwing, because this + /// runs while a test is already failing and a second failure would bury the + /// first. + func capture(_ description: String) -> String { + let name = + description + .replacingOccurrences(of: "/", with: "-") + .replacingOccurrences(of: " ", with: "-") + .prefix(80) + let file = Diagnostics.directory + .appendingPathComponent("\(name)-\(UUID().uuidString.prefix(8)).png") + + do { + try FileManager.default.createDirectory( + at: Diagnostics.directory, + withIntermediateDirectories: true + ) + try app.screenshot().pngRepresentation.write(to: file) + } catch { + return "(no screenshot: \(error))" + } + + return file.lastPathComponent + } + +} diff --git a/apps/macos/UITests/ConversationFixtures.swift b/apps/macos/UITests/ConversationFixtures.swift new file mode 100644 index 000000000..3dafd9806 --- /dev/null +++ b/apps/macos/UITests/ConversationFixtures.swift @@ -0,0 +1,117 @@ +/// The workspace the conversation-list tests run against. +/// +/// A type of its own rather than statics on the suite, because a suite's own +/// `.sharedApp(...)` attribute cannot name the suite it is attached to: the +/// macro would have to resolve the type it is in the middle of expanding. +enum ConversationFixtures { + /// Oldest activity, so it sorts last. + static let readingList = FixtureConversation( + id: "17251488000", + title: "Reading list", + lastActivatedAt: "2024-09-01 09:00:00.0", + events: [ + FixtureConversation.userMessage( + at: "2024-09-01 09:00:00.0", from: "Jean", "What is on the reading list?"), + FixtureConversation.assistantMessage( + at: "2024-09-01 09:00:01.0", "Three books and a paper."), + ] + ) + + static let configPipeline = FixtureConversation( + id: "17251488010", + title: "Config pipeline", + lastActivatedAt: "2024-09-02 09:00:00.0", + events: [ + FixtureConversation.userMessage( + at: "2024-09-02 09:00:00.0", from: "Jean", "How does the config pipeline layer?" + ), + FixtureConversation.assistantMessage( + at: "2024-09-02 09:00:01.0", "Later layers win, field by field."), + ] + ) + + /// Newest activity, so it sorts first. + static let releaseNotes = FixtureConversation( + id: "17251488020", + title: "Release notes", + lastActivatedAt: "2024-09-03 09:00:00.0", + events: [ + FixtureConversation.userMessage( + at: "2024-09-03 09:00:00.0", from: "Jean", "Draft the release notes."), + FixtureConversation.assistantMessage( + at: "2024-09-03 09:00:01.0", "Drafted, with one open question."), + FixtureConversation.userMessage( + at: "2024-09-03 09:00:02.0", from: "Jean", "Answer it yourself."), + FixtureConversation.assistantMessage( + at: "2024-09-03 09:00:03.0", "Answered."), + ] + ) + + /// ``readingList``, pinned. + /// + /// The oldest of the three, so a list showing it first can only be showing it + /// there because it is pinned. + static let pinnedReadingList = FixtureConversation( + id: readingList.id, + title: readingList.title, + lastActivatedAt: readingList.lastActivatedAt, + pinnedAt: "2024-09-04 09:00:00.0", + events: readingList.events + ) + + /// One conversation tall enough to scroll, for the tests about re-wrapping. + /// + /// Prose rather than a repeated line, because the thing under test is text + /// finding new line breaks at a new width: a paragraph of one word repeated + /// wraps at the same places whatever the width, and would reflow invisibly. + static let longRead = FixtureConversation( + id: "17251488030", + title: "Long read", + lastActivatedAt: "2024-09-05 09:00:00.0", + events: [ + FixtureConversation.userMessage( + at: "2024-09-05 09:00:00.0", from: "Jean", "Explain the layout pipeline."), + FixtureConversation.assistantMessage( + at: "2024-09-05 09:00:01.0", paragraphs(40)), + ] + ) + + /// `count` paragraphs of varied prose, as one markdown message. + /// + /// Numbered so a reader of a failure can tell where in the document they are, + /// and of uneven length so the line breaks are not all in the same column. + private static func paragraphs(_ count: Int) -> String { + (1...count) + .map { index in + """ + ## Section \(index) + + The layout pipeline measures what it is given and wraps it to the \ + width it is offered, which is why a window resize is a text \ + problem rather than a drawing one. Paragraph \(index) exists to \ + take up enough room that the document is taller than any window \ + showing it. + """ + } + .joined(separator: "\n\n") + } + + /// A workspace holding all three. + static func make() throws -> WorkspaceFixture { + try WorkspaceFixture.make(conversations: [readingList, configPipeline, releaseNotes]) + } + + /// A workspace holding only ``longRead``. + /// + /// One conversation, so the sidebar publishes almost nothing and every + /// synthesized event in the test is cheap. + static func makeLongRead() throws -> WorkspaceFixture { + try WorkspaceFixture.make(conversations: [longRead]) + } + + /// The same three, with the oldest one pinned. + static func makeWithPinnedOldest() throws -> WorkspaceFixture { + try WorkspaceFixture.make( + conversations: [pinnedReadingList, configPipeline, releaseNotes]) + } +} diff --git a/apps/macos/UITests/ConversationListTests.swift b/apps/macos/UITests/ConversationListTests.swift new file mode 100644 index 000000000..8bc1908e5 --- /dev/null +++ b/apps/macos/UITests/ConversationListTests.swift @@ -0,0 +1,242 @@ +import Testing +import XCTest + +/// The Conversation list section of `QA.md`, run rather than read. +/// +/// The workspace is three conversations with fixed IDs, titles and activity +/// times, so a test can name the row it wants and say where it should sit. +/// +/// One app for the whole suite. These tests read the list and move the +/// selection around, which is state the next test can set for itself, so paying +/// a launch and a terminate each to start from a fresh process buys nothing. A +/// test that needs an app nobody has touched says so and launches its own with +/// ``AppUnderTest/launch(against:keepingWindowState:sourceLocation:)``. +extension UISuite { + @Suite( + "ConversationList", + .sharedApp { try ConversationFixtures.make() } + ) + @MainActor + struct ConversationListTests { + /// The suite's app, and the workspace it was launched against. + var driven: AppUnderTest { SharedAppBox.shared.app } + var fixture: WorkspaceFixture { SharedAppBox.shared.workspace } + + /// The workspace's directory name and nothing else. The window carries a + /// title because the Window menu lists it and a driver addresses it by + /// it, but it says only which workspace the window is on: a subtitle + /// counting conversations put a strip of chrome above the transcript that + /// the design does not have. + @Test("titles the window with the workspace name alone") + func namesTheWorkspace() { + #expect(driven.workspaceWindow(fixture).title == fixture.name) + } + + @Test("orders conversations most recently active first") + func ordersByActivity() { + let newest = driven.row(ConversationFixtures.releaseNotes) + let middle = driven.row(ConversationFixtures.configPipeline) + let oldest = driven.row(ConversationFixtures.readingList) + + guard + driven.expectAppears(newest, "the Release notes row"), + driven.expectAppears(middle, "the Config pipeline row"), + driven.expectAppears(oldest, "the Reading list row") + else { return } + + #expect(newest.frame.minY < middle.frame.minY) + #expect(middle.frame.minY < oldest.frame.minY) + } + + /// The date the row also shows is deliberately absent from its label: it + /// is relative for anything active today, so an assertion on it would + /// pass or fail depending on the minute the suite ran. + @Test("shows a row's title and event count together") + func labelsRows() { + #expect( + driven.row(ConversationFixtures.releaseNotes).label + == "Release notes, \(ConversationFixtures.releaseNotes.eventCountLabel)" + ) + } + + /// The row going away and coming back is what says the binding behind the + /// field is live, rather than the field merely showing the letters typed + /// into it: an accessibility value can be set on a text field without ever + /// reaching the state the list is drawn from. + /// + /// Leaves the box empty again, because the suite shares one app and every + /// test after this one expects the whole list. + @Test("narrows the list while filtering, and restores it when cleared") + func filtersAndClears() { + let hidden = driven.row(ConversationFixtures.readingList) + guard + driven.expectAppears(hidden, "the Reading list row"), + // Present whether or not there is anything to clear, so it is + // there to be found before a word has been typed. + driven.expectAppears(driven.filterClear, "the clear button") + else { return } + + driven.filter.click() + driven.filter.typeText("Release") + + guard driven.expectDisappears(hidden, "the Reading list row, once filtered") + else { return } + + driven.filterClear.click() + + driven.expectAppears(hidden, "the Reading list row, once cleared") + } + + /// The whole transcript, exactly: two messages, each under the name of + /// whoever said it. + @Test("selects a row on click, and the transcript follows") + func clickSelects() { + driven.row(ConversationFixtures.configPipeline).click() + + driven.expectTranscript( + Transcripts.configPipeline, "the Config pipeline transcript") + } + + /// The whole row is the click target, not just the text in it. A row + /// built as a label with padding around it leaves the padding dead, and + /// clicking beside a title is what a person does. + @Test("selects a row clicked in the empty space beside its title") + func clickBesideTitleSelects() { + driven.row(ConversationFixtures.readingList) + .coordinate(withNormalizedOffset: CGVector(dx: 0.75, dy: 0.85)) + .click() + + driven.expectTranscript(Transcripts.readingList, "the Reading list transcript") + } + + @Test("moves the selection with the arrow keys") + func arrowKeysMoveSelection() { + // Start at the top row, so one press down lands on a known one. + driven.row(ConversationFixtures.releaseNotes).click() + guard + driven.expectTranscript( + Transcripts.releaseNotes, "the Release notes transcript") + else { return } + + driven.app.typeKey(.downArrow, modifierFlags: []) + + driven.expectTranscript( + Transcripts.configPipeline, + "the Config pipeline transcript, after pressing down" + ) + } + + @Test("opens a conversation in its own window on double-click") + func doubleClickOpensAWindow() { + driven.row(ConversationFixtures.readingList).doubleClick() + defer { driven.closeWindow(titled: "Reading list") } + + let opened = driven.app.windows["Reading list"] + guard driven.expectAppears(opened, "a window titled Reading list") else { return } + + // Showing the conversation, not an empty pane. + driven.expectTranscript( + Transcripts.readingList, + "the conversation inside its own window", + within: opened + ) + } + + /// A different conversation from the double-click test, so a window that + /// test failed to close could not make this one pass. + @Test("opens a conversation in its own window from the context menu") + func contextMenuOpensAWindow() { + driven.row(ConversationFixtures.releaseNotes).rightClick() + driven.chooseContextMenuItem("Open in New Window") + defer { driven.closeWindow(titled: "Release notes") } + + let opened = driven.app.windows["Release notes"] + guard driven.expectAppears(opened, "a window titled Release notes") else { return } + + driven.expectTranscript( + Transcripts.releaseNotes, + "the conversation inside its own window", + within: opened + ) + } + + /// The URI lands on a pasteboard of the fixture's own, never the system + /// one — the app under test is told which to use, and + /// `ClipboardPolicyTests` holds the suite to it. + @Test("copies a conversation's URI from the context menu") + func contextMenuCopiesTheURI() { + driven.row(ConversationFixtures.readingList).rightClick() + driven.chooseContextMenuItem("Copy Link") + + #expect(fixture.copiedText() == ConversationFixtures.readingList.uri) + } + + /// Edit ▸ Copy Link acts on the sidebar selection, so it is greyed out + /// until there is one — and Escape is how a window gets back to having + /// none. + /// + /// The empty pane is waited for rather than assumed. Without it the + /// disabled half of this test would also pass against an Escape that did + /// nothing, in a suite where every earlier test leaves a selection + /// behind. + @Test("enables Edit ▸ Copy Link only once a conversation is selected") + func editCopyLinkFollowsTheSelection() { + driven.row(ConversationFixtures.configPipeline).click() + guard + driven.expectTranscript( + Transcripts.configPipeline, "the Config pipeline transcript") + else { return } + + driven.app.typeKey(.escape, modifierFlags: []) + guard + driven.expectDisappears(driven.transcript, "the transcript, after Escape") + else { return } + + let edit = driven.openMenu("Edit") + #expect(driven.menuItemIsEnabled("Copy Link", in: edit) == false) + driven.closeMenu() + + driven.row(ConversationFixtures.configPipeline).click() + driven.chooseMenuItem("Copy Link", in: "Edit") + + #expect(fixture.copiedText() == ConversationFixtures.configPipeline.uri) + } + + /// The window holds its two panes itself rather than in a + /// `NavigationSplitView`, so this item is the app's own and not AppKit's. + /// Its title flips with what it will do, and it is the only way back to a + /// hidden sidebar — there is no button for it. + /// + /// Leaves the sidebar showing, because the suite shares one app and every + /// other test addresses a row. + @Test("hides and shows the sidebar from the View menu") + func viewMenuTogglesTheSidebar() { + guard driven.expectAppears(driven.sidebar, "the conversation list") else { return } + + driven.chooseMenuItem("Hide Sidebar", in: "View") + guard + driven.expectDisappears(driven.sidebar, "the conversation list, once hidden") + else { return } + + driven.chooseMenuItem("Show Sidebar", in: "View") + driven.expectAppears(driven.sidebar, "the conversation list, brought back") + } + + /// Enter Full Screen and Merge All Windows are items AppKit injects into + /// menus SwiftUI builds from its own commands. A menu bar rebuilt at the + /// wrong moment — which a focused value that never compares equal to + /// itself causes, on every render — drops them. + @Test("keeps the AppKit-injected View and Window items after selecting") + func selectingKeepsTheInjectedMenuItems() { + driven.row(ConversationFixtures.releaseNotes).click() + + let view = driven.openMenu("View") + #expect(driven.menuItemExists("Enter Full Screen", in: view)) + driven.closeMenu() + + let window = driven.openMenu("Window") + #expect(driven.menuItemExists("Merge All Windows", in: window)) + driven.closeMenu() + } + } +} diff --git a/apps/macos/UITests/Diagnostics.swift b/apps/macos/UITests/Diagnostics.swift new file mode 100644 index 000000000..cc5fd8729 --- /dev/null +++ b/apps/macos/UITests/Diagnostics.swift @@ -0,0 +1,69 @@ +import Foundation + +/// Where a UI test writes what a reader needs and `xcodebuild` will not carry. +/// +/// Two things end up here: screenshots of what was on screen when an assertion +/// failed, and the failure messages themselves. The messages need a home +/// because swift-testing prints an issue's text on a line of its own, under a +/// header naming only the kind of issue, and `xcodebuild` keeps the header and +/// drops the line — so ten failures arrive as ten identical `Issue recorded` +/// entries, which says how many things broke and nothing about what. +/// +/// The directory is the runner's container, not the checkout. Xcode wraps a UI +/// test bundle in a generated, sandboxed runner app, so a write anywhere in the +/// project fails with `Operation not permitted` however the path is spelled. +/// `swift_test_ui` copies out of here and into `tmp/uitests/`. +enum Diagnostics { + /// The directory both screenshots and messages are written to. + static let directory = FileManager.default.temporaryDirectory + .appendingPathComponent("jp-uitests") + + /// Where the messages are written. + static let file = directory.appendingPathComponent("failures.txt") + + /// Where the process ids of the apps this run launched are written. + /// + /// A run stopped part-way is stopped from outside, by killing + /// `xcodebuild`. That does not reach the app: it is `testmanagerd` that + /// launched it, so it survives and stays on screen. These are how the tool + /// that stopped the run finds it, exactly, without matching on a name the + /// developer's own copy of JP also has. + static let processes = directory.appendingPathComponent("app.pids") + + /// Note that an app was launched, so a stopped run can still close it. + static func recordAppProcess(_ pid: String) { + append(pid, to: processes) + } + + /// Append one line, creating the file if this is the first. + /// + /// Silent on failure. This runs while a test is already failing, and a + /// second failure would bury the first. + static func append(_ line: String) { + append(line, to: file) + } + + private static func append(_ line: String, to file: URL) { + guard let data = (line + "\n").data(using: .utf8) else { return } + + try? FileManager.default.createDirectory( + at: directory, + withIntermediateDirectories: true + ) + + guard let handle = try? FileHandle(forWritingTo: file) else { + try? data.write(to: file) + return + } + + defer { try? handle.close() } + + do { + try handle.seekToEnd() + try handle.write(contentsOf: data) + } catch { + // Nothing useful left to do: the test is already failing, and the + // message is on its way to `Issue.record` regardless. + } + } +} diff --git a/apps/macos/UITests/PinnedConversationTests.swift b/apps/macos/UITests/PinnedConversationTests.swift new file mode 100644 index 000000000..e52aed472 --- /dev/null +++ b/apps/macos/UITests/PinnedConversationTests.swift @@ -0,0 +1,42 @@ +import Testing +import XCTest + +/// Pinning, end to end: a `pinned_at` timestamp on disk, through the library and +/// its C ABI, to a row that sits at the top of the list and says so. +/// +/// Its own app and its own workspace, unlike the rest of the list tests. The +/// shared fixture has no pins, and pinning one of its three conversations would +/// move the row that every ordering assertion in `ConversationListTests` names. +extension UISuite { + @Suite("PinnedConversations") + @MainActor + struct PinnedConversationTests { + @Test("lifts a pinned conversation above a more recently active one") + func pinnedSortsFirst() throws { + let fixture = try ConversationFixtures.makeWithPinnedOldest() + defer { fixture.remove() } + + let driven = AppUnderTest.launch(against: fixture) + defer { driven.terminate() } + + let pinned = driven.row(ConversationFixtures.pinnedReadingList) + let newest = driven.row(ConversationFixtures.releaseNotes) + + guard + driven.expectAppears(pinned, "the pinned Reading list row"), + driven.expectAppears(newest, "the Release notes row") + else { return } + + // Reading list is the oldest of the three, so without the pin it sits + // below both others; this is the pin moving it and nothing else. + #expect(pinned.frame.minY < newest.frame.minY) + + // And the row says it is pinned, which is the only way anything + // outside the app can tell the pin glyph is drawn. + #expect( + pinned.label + == "Reading list, \(ConversationFixtures.pinnedReadingList.eventCountLabel), pinned" + ) + } + } +} diff --git a/apps/macos/UITests/PointerCursorTests.swift b/apps/macos/UITests/PointerCursorTests.swift new file mode 100644 index 000000000..498e12c1c --- /dev/null +++ b/apps/macos/UITests/PointerCursorTests.swift @@ -0,0 +1,84 @@ +import AppKit +import Foundation +import Testing +import XCTest + +/// What the pointer becomes over the things that respond to it. +/// +/// The only test in this project that can see a cursor. A cursor is not in the +/// accessibility tree and is not composited into a screenshot, so nothing inside +/// the app can prove one was delivered — `ResizeCursorAreaTests` asserts the view +/// *asks* for a cursor and stayed green through two states where the pointer +/// never changed, which is exactly the gap this closes. +/// +/// `NSCursor.currentSystem` reads what the window server is displaying rather +/// than what the calling process requested, so this test process can read the +/// cursor the app under test caused. +/// +/// Its own app: it moves the pointer around and leaves it wherever the last hover +/// put it, which is not a state to hand the next suite. +extension UISuite { + @Suite("PointerCursor") + @MainActor + struct PointerCursorTests { + /// The pointer becomes the horizontal-resize cursor over the strip that + /// resizes the sidebar. + /// + /// Dragging that strip works, and the view asks for the right cursor over + /// the right area, and the pointer still does not change — the request is + /// made and not delivered. Until this passes, that is unfixed. + @Test("shows the horizontal-resize cursor over the pane divider") + func showsResizeCursorOverTheDivider() { + let fixture = try? ConversationFixtures.make() + guard let fixture else { + Issue.record("could not build the fixture workspace") + return + } + defer { fixture.remove() } + + let driven = AppUnderTest.launch(against: fixture) + defer { driven.terminate() } + + guard driven.expectAppears(driven.divider, "the pane divider") else { return } + + // The baseline, and it is not optional. `NSCursor.currentSystem` reads + // the cursor for the whole machine, so a column-resize cursor left + // showing by anything at all would pass the assertion below without + // this app having done a thing. Establishing the arrow first turns "it + // is the right cursor" into "it changed to the right cursor". + // + // The conversation list rather than the transcript: the transcript does + // not exist until something is selected, and hovering a missing element + // fails without stopping the test — which is how an earlier version of + // this passed with no baseline at all. + driven.sidebar.hover() + let arrowFirst = driven.waitForCursor(.arrow) + + #expect( + arrowFirst, + """ + over the conversation list the pointer was \(driven.describeCursor()) \ + rather than the arrow, so this run cannot say whether the divider \ + changed anything. + """ + ) + guard arrowFirst else { return } + + driven.divider.hover() + + // Read into a `Bool` first: swift-testing reports the expression it + // evaluated, and `driven` holds an `XCUIApplication` whose description + // is the entire element tree. + let changed = driven.waitForCursor(.columnResize) + + #expect( + changed, + """ + the pointer over the pane divider did not become the column-resize \ + cursor. It stayed \(driven.describeCursor()). The strip drags \ + correctly, so the gesture reaches it and the pointer does not. + """ + ) + } + } +} diff --git a/apps/macos/UITests/Quiescence.swift b/apps/macos/UITests/Quiescence.swift new file mode 100644 index 000000000..d864cefc3 --- /dev/null +++ b/apps/macos/UITests/Quiescence.swift @@ -0,0 +1,96 @@ +import Foundation +import ObjectiveC + +/// Stops XCUITest waiting for the app under test to go quiet after every event +/// it synthesizes. +/// +/// Worth about a second of a sixteen-second run, which is less than it sounds +/// like it should be: the wait is not what makes a synthesized click expensive. +/// A click costs ~410ms with this installed and ~440ms without, against an app +/// that answers in tens of milliseconds; the rest is inside XCTest's pointer +/// path and out of reach from here. Do not expect a second one of these to turn +/// up. +/// +/// Safe because nothing in this suite leans on the wait. Every assertion waits +/// on a condition of its own through ``AppUnderTest/wait(for:)``, which is +/// faster and specific about what it is waiting for; an implicit settle after +/// each event only hides where a real one is missing. A test that starts +/// failing after a change here is a test that was relying on it — give it the +/// wait it actually needs rather than putting this one back. +/// +/// Private API, reached by replacing two method implementations. It lives in +/// the test bundle and nothing ships it. It is version-fragile: the selector +/// this replaces was one argument in 2016, is two now, and picked up a third in +/// a variant along the way. So ``install()`` checks every assumption it makes +/// and reports rather than guessing, and ``AppUnderTest/launch(against:)`` +/// fails the run when it reports. A silent no-op would put the second back and +/// tell nobody. +enum Quiescence { + /// What went wrong installing this, or `nil` if it took. + /// + /// A `let`, so the work happens once however many apps a run launches. + static let installation: String? = install() + + /// The class that does the waiting. + private static let className = "XCUIApplicationProcess" + + /// Replace both waits, or say why not. + /// + /// Both, not either: XCTest calls the plain one and the one that opens an + /// activity around the wait, and leaving one in place leaves its share of + /// the cost in place with it. + /// + /// `shouldSkipPreEventQuiescence` and `shouldSkipPostEventQuiescence` look + /// like the better target — no arguments, `BOOL` return, nothing to get + /// wrong — and forcing both to `true` measurably changes nothing. XCTest + /// does not consult them on the path that costs. + /// + /// The encodings are checked rather than assumed, because a replacement is + /// called through a signature the runtime does not police: a method that + /// gained an argument, or that returns something other than `void`, would + /// be called with the wrong frame and go wrong somewhere unrelated. `v` is + /// void, `@0:8` the receiver and selector every method takes, and each `B` + /// a `_Bool` argument. `B` rather than `c` also pins this to a machine + /// where `BOOL` is `_Bool` — the Swift `Bool` the blocks below are written + /// with matches that and not the `signed char` an Intel Mac would want. + private static func install() -> String? { + guard let process: AnyClass = NSClassFromString(className) else { + return "XCTest no longer has a class named \(className)." + } + + let two: @convention(block) (AnyObject, Bool, Bool) -> Void = { _, _, _ in } + let three: @convention(block) (AnyObject, Bool, Bool, Bool) -> Void = { _, _, _, _ in } + + let replacements = [ + ( + name: "waitForQuiescenceIncludingAnimationsIdle:isPreEvent:", + encoding: "v24@0:8B16B20", + imp: imp_implementationWithBlock(two) + ), + ( + name: "waitForQuiescenceIncludingAnimationsIdle:usingActivity:isPreEvent:", + encoding: "v28@0:8B16B20B24", + imp: imp_implementationWithBlock(three) + ), + ] + + for replacement in replacements { + let selector = NSSelectorFromString(replacement.name) + guard let method = class_getInstanceMethod(process, selector) else { + return "\(className) no longer answers \(replacement.name)." + } + + let found = method_getTypeEncoding(method).map { String(cString: $0) } ?? "(none)" + guard found == replacement.encoding else { + return """ + \(className).\(replacement.name) is \(found), \ + expected \(replacement.encoding). + """ + } + + method_setImplementation(method, replacement.imp) + } + + return nil + } +} diff --git a/apps/macos/UITests/SharedApp.swift b/apps/macos/UITests/SharedApp.swift new file mode 100644 index 000000000..2b357be73 --- /dev/null +++ b/apps/macos/UITests/SharedApp.swift @@ -0,0 +1,107 @@ +import Testing +import XCTest + +/// Runs a suite's tests against one launched app instead of one each. +/// +/// Launching costs seconds and the work under test costs milliseconds, so a +/// suite that launches per test spends almost all of its time starting and +/// stopping the app. This launches once, hands the same instance to every test +/// in the suite, and terminates it when the suite finishes. +/// +/// The trade is that tests share what the app remembers. A suite using this has +/// to leave the app as it found it, or order its tests so that what one leaves +/// behind is what the next one expects. A test that cannot work that way asks +/// for its own instance with ``AppUnderTest/launch(against:)`` and terminates +/// it itself. +/// +/// Safe despite the shared mutable state because ``UISuite`` is serialized: +/// only one test runs at a time, and all of this is main-actor isolated. +struct SharedApp: SuiteTrait, TestScoping { + /// The fixture the app is launched against. + let fixture: @Sendable () throws -> WorkspaceFixture + + func provideScope( + for test: Test, + testCase: Test.Case?, + performing function: () async throws -> Void + ) async throws { + let fixture = try fixture() + let driven = await AppUnderTest.launch(against: fixture) + await SharedAppBox.shared.set(driven, fixture: fixture) + + // Torn down on both paths rather than in a `defer`, so terminating is + // awaited: a `defer` would have to spawn a task to reach the main actor, + // and a fire-and-forget task can lose the race with the process exiting + // — leaving the app on the developer's screen. + do { + try await function() + } catch { + await Self.teardown(driven, fixture) + throw error + } + + await Self.teardown(driven, fixture) + } + + @MainActor + private static func teardown(_ driven: AppUnderTest, _ fixture: WorkspaceFixture) { + SharedAppBox.shared.clear() + driven.terminate() + fixture.remove() + } +} + +extension Trait where Self == SharedApp { + /// One app for the whole suite, launched against `fixture`. + static func sharedApp( + _ fixture: @escaping @Sendable () throws -> WorkspaceFixture + ) -> Self { + SharedApp(fixture: fixture) + } +} + +/// Where the suite's app is kept between the trait that launches it and the +/// tests that use it. +/// +/// A global rather than a property on the suite, because swift-testing builds a +/// fresh suite value for every test: anything stored on the suite is gone by +/// the time the next test runs. +@MainActor +final class SharedAppBox { + static let shared = SharedAppBox() + + private var driven: AppUnderTest? + private var fixture: WorkspaceFixture? + + private init() {} + + func set(_ driven: AppUnderTest, fixture: WorkspaceFixture) { + self.driven = driven + self.fixture = fixture + } + + func clear() { + driven = nil + fixture = nil + } + + /// The app the suite is running against. + /// + /// Traps rather than returning an optional every caller has to unwrap: a + /// test reaching for this without ``SharedApp`` on its suite is a mistake in + /// the test, and every assertion after it would be meaningless anyway. + var app: AppUnderTest { + guard let driven else { + fatalError("no shared app: put `.sharedApp(...)` on the suite") + } + return driven + } + + /// The workspace the suite's app was launched against. + var workspace: WorkspaceFixture { + guard let fixture else { + fatalError("no shared app: put `.sharedApp(...)` on the suite") + } + return fixture + } +} diff --git a/apps/macos/UITests/TranscriptReflowTests.swift b/apps/macos/UITests/TranscriptReflowTests.swift new file mode 100644 index 000000000..8aaae0069 --- /dev/null +++ b/apps/macos/UITests/TranscriptReflowTests.swift @@ -0,0 +1,126 @@ +import Foundation +import Testing +import XCTest + +/// Whether the transcript re-wraps while a window is being dragged. +/// +/// Its own app rather than the shared one: it needs a conversation tall enough +/// to scroll, and it resizes and scrolls the window it is given, which is not a +/// state to hand the next suite. +extension UISuite { + @Suite("TranscriptReflow") + @MainActor + struct TranscriptReflowTests { + /// The interval the app writes once per window drag. + private static let drag = "transcript.liveresize" + + /// How far the drag moves the window's right edge, in points. + /// + /// Outwards. A window opened fresh sits at its minimum width, because the + /// scene names no default size and SwiftUI takes the smallest its content + /// allows — so a drag inwards has nowhere to go, moves the pointer, resizes + /// nothing, and delivers no frames at all. + private static let dragBy: CGFloat = 220 + + /// The whole point: text re-wraps on every frame of a window drag, not + /// once the mouse comes up. + /// + /// A window resize reaches the text through the text container, whose width + /// the text view is supposed to keep in step with its own. AppKit does not + /// do that while a resize is in progress, so nothing changes the + /// container's geometry, nothing invalidates layout, and the view redraws + /// lines wrapped to a width the window no longer has. The app sets the + /// container's width itself for exactly this reason. + /// + /// Asserted through the app's own trace rather than off the screen, because + /// the defect leaves nothing behind: on mouse-up the container catches up + /// and the text is correct either way. Only what happened *during* the drag + /// tells the two apart. + @Test("re-wraps the transcript during a window drag, scrolled away from the top") + func reflowsWhileDragging() throws { + let fixture = try ConversationFixtures.makeLongRead() + defer { fixture.remove() } + + let driven = AppUnderTest.launch(against: fixture) + defer { driven.terminate() } + + driven.row(ConversationFixtures.longRead).click() + guard driven.expectAppears(driven.transcriptText, "the transcript's text") + else { return } + + scrollToTheEnd(of: driven) + dragTheWindowEdge(of: driven, fixture) + + let record = try #require( + fixture.lastTracedInterval(named: Self.drag), + "the app traced no window drag, so the gesture never reached it" + ) + + // Two preconditions before the assertion, because each of them failing + // would leave a test that passes without having tried anything. + #expect( + (record["visible_from_y"] ?? 0) > 1000, + """ + the transcript was still near the top of the document, where the \ + defect does not show: \(record) + """ + ) + #expect( + (record["width_changes"] ?? 0) > 4, + """ + the drag delivered almost no width changes, so it was a jump rather \ + than a gesture: \(record) + """ + ) + + // The assertion. Zero is what the defect produces: the view resized + // hundreds of times and the container was told nothing. + #expect( + (record["container_changes"] ?? 0) > 0, + """ + the text container's width never changed while the window was being \ + dragged, so the text on screen stayed wrapped to the old width \ + until the mouse came up: \(record) + """ + ) + } + + /// Put the transcript at the end of the document. + /// + /// Through the text view's own Command-Down rather than a synthesized + /// scroll wheel: `scroll(byDeltaX:deltaY:)` reported synthesizing an event + /// and left the transcript where it was. The end is used rather than a + /// measured fraction because it is a position the view can be asked for + /// exactly, and anywhere past the first fifth of the document is equally + /// good for what is being tested. + private func scrollToTheEnd(of driven: AppUnderTest) { + driven.transcriptText.click() + driven.app.typeKey(.downArrow, modifierFlags: .command) + } + + /// Drag the window's right edge outwards, once. + /// + /// One direction and no attempt to put the window back. A coordinate is + /// resolved against its element's frame at the moment it is *used*, not + /// when it is made, so a second gesture written against the same two + /// coordinates re-resolves both against the window the first one just + /// narrowed: the return drag starts inside the window body and pulls a + /// stretch of empty transcript instead of the edge. + /// + /// Nothing needs the width restored. This suite launches its own app and + /// terminates it, and the assertion is about the frames during the drag + /// rather than the size it ended on. + /// + /// The window is raised by the click that preceded this, so the edge is + /// where the tree says it is. + private func dragTheWindowEdge(of driven: AppUnderTest, _ fixture: WorkspaceFixture) { + let window = driven.workspaceWindow(fixture) + let edge = window.coordinate(withNormalizedOffset: CGVector(dx: 1, dy: 0.5)) + + edge.press( + forDuration: 0.1, + thenDragTo: edge.withOffset(CGVector(dx: Self.dragBy, dy: 0)) + ) + } + } +} diff --git a/apps/macos/UITests/Transcripts.swift b/apps/macos/UITests/Transcripts.swift new file mode 100644 index 000000000..e0bed3e98 --- /dev/null +++ b/apps/macos/UITests/Transcripts.swift @@ -0,0 +1,36 @@ +/// What each fixture conversation looks like once the app has drawn it. +/// +/// Written out in full rather than assembled from the fixture's messages, so a +/// reader sees exactly what is on screen and a change to the transcript's shape +/// shows up here as a diff. Building these from ``ConversationFixtures`` would +/// follow a change in the app's formatting instead of catching one. +/// +/// The shape: each message is its speaker's name on one line, then the message, +/// with nothing between one message and the next but a newline. The spacing a +/// reader sees is paragraph spacing, which is not in the text. +enum Transcripts { + static let readingList = """ + Jean + What is on the reading list? + Assistant + Three books and a paper. + """ + + static let configPipeline = """ + Jean + How does the config pipeline layer? + Assistant + Later layers win, field by field. + """ + + static let releaseNotes = """ + Jean + Draft the release notes. + Assistant + Drafted, with one open question. + Jean + Answer it yourself. + Assistant + Answered. + """ +} diff --git a/apps/macos/UITests/UISuite.swift b/apps/macos/UITests/UISuite.swift new file mode 100644 index 000000000..e90984386 --- /dev/null +++ b/apps/macos/UITests/UISuite.swift @@ -0,0 +1,13 @@ +import Testing + +/// The suite every UI test belongs to. +/// +/// Serialized, and serialized *together*: `XCUIApplication` addresses the app +/// under test by bundle identifier, so two tests running side by side would +/// drive one process between them. Nesting is what puts sibling suites under +/// the same ordering — `.serialized` orders a suite's own tests and its nested +/// suites, while suites declared alongside each other still run in parallel. +/// +/// The `extension UISuite` declarations in the sibling files are that nesting. +@Suite("UI", .serialized) +struct UISuite {} diff --git a/apps/macos/UITests/WorkspaceFixture.swift b/apps/macos/UITests/WorkspaceFixture.swift new file mode 100644 index 000000000..414fb504e --- /dev/null +++ b/apps/macos/UITests/WorkspaceFixture.swift @@ -0,0 +1,273 @@ +import AppKit +import Foundation + +/// A workspace on disk for one UI test, and the scratch directories the app +/// under test writes into. +/// +/// The layout is JP's storage format: `.jp/.id` names the workspace, and each +/// conversation is a directory holding `metadata.json`, `base_config.json` and +/// `events.json`. A UI test runs outside the app's process and cannot reach the +/// Rust library that would otherwise write them, so they are written here by +/// hand. `crates/jp_ffi/src/lib_tests.rs` pins the same shape from the Rust +/// side; a change to one needs the other. +/// +/// Paired with ``remove()`` through `defer` rather than released by a `deinit`: +/// ARC may drop an object right after its last mention, which can be while the +/// app is still reading the directory. +struct WorkspaceFixture { + /// Everything the fixture owns. + let root: URL + + /// The workspace directory the app is told to open. + let workspacePath: String + + /// The pasteboard the app under test copies to. + /// + /// A real pasteboard that nobody is looking at, so Copy Link can be checked + /// without destroying whatever the person at the keyboard last copied. + /// Named per fixture, so a stale value from an earlier run cannot be read + /// back as this one's. + let pasteboardName: String + + /// The workspace's directory name, which the window shows as its title. + var name: String { + URL(fileURLWithPath: workspacePath).lastPathComponent + } + + /// Create a fixture holding `conversations`. + /// + /// The workspace ID is written rather than left for JP to mint, because JP + /// derives one from the current millisecond. A fixed one keeps the + /// user-local store path stable across runs. + static func make( + named name: String = "my-workspace", + conversations: [FixtureConversation] = [] + ) throws -> WorkspaceFixture { + let root = URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent("jp-uitests-\(UUID().uuidString)") + let workspace = root.appendingPathComponent(name) + let store = workspace.appendingPathComponent(".jp") + + let files = FileManager.default + try files.createDirectory(at: store, withIntermediateDirectories: true) + + // `Id::load` reads the last line, and rejects anything that is not five + // characters of `[0-9a-z]`. + let preamble = "DO NOT EDIT THIS FILE! IT IS AUTO-GENERATED BY JP." + try "\(preamble)\nuitst\n" + .write(to: store.appendingPathComponent(".id"), atomically: true, encoding: .utf8) + + for conversation in conversations { + try conversation.write(into: store) + } + + // The app resolves `HOME` for anything it keeps in the home directory, + // so the directory has to exist before it looks. + for scratch in ["user-data", "state", "home"] { + try files.createDirectory( + at: root.appendingPathComponent(scratch), + withIntermediateDirectories: true + ) + } + + return WorkspaceFixture( + root: root, + workspacePath: workspace.path, + pasteboardName: "computer.jp.jean-pierre.uitest.\(UUID().uuidString)" + ) + } + + /// What to launch the app with, so nothing it writes reaches the state the + /// developer shares with it. + /// + /// - `JP_WORKSPACE` names the workspace to open. The app prefers it over + /// both its stored path and its most recent workspace. + /// - `JP_USER_DATA_DIR` moves the user-local conversation store, which + /// opening a workspace creates. + /// - `JP_DEBUG_STATE_DIR` moves the recent-workspace list into a file here, + /// instead of the list the app shares with the system. That list needs + /// Full Disk Access to read back, so a test could neither inspect nor + /// restore it. + /// - `HOME` moves whatever else the app resolves from the home directory. + /// - `JP_DEBUG_PASTEBOARD` moves Copy Link off the system pasteboard. Read + /// only by a debug build; see `DebugState.pasteboard`. + /// - `JP_DEBUG_DISABLE_ANIMATIONS` stops the app animating. XCUITest waits + /// for the app to stop moving before every action it synthesizes, so an + /// animation is time added to every test that triggers one. + /// + /// Window state saved by `@SceneStorage` reaches none of these, because it + /// is keyed by bundle identifier. ``AppUnderTest`` handles that with a + /// launch argument. + var environment: [String: String] { + [ + "JP_WORKSPACE": workspacePath, + "JP_USER_DATA_DIR": root.appendingPathComponent("user-data").path, + "JP_DEBUG_STATE_DIR": root.appendingPathComponent("state").path, + "HOME": root.appendingPathComponent("home").path, + "JP_DEBUG_PASTEBOARD": pasteboardName, + "JP_DEBUG_DISABLE_ANIMATIONS": "1", + ] + } + + /// The process id of the app launched against this fixture. + /// + /// Written by the app itself, into the state directory it was pointed at. + /// Exact rather than matched on a name or a bundle identifier, which is + /// what makes it safe to act on: the developer's own copy of JP shares both + /// of those and must never be touched. + var appProcessID: String? { + let file = root.appendingPathComponent("state/pid") + guard let text = try? String(contentsOf: file, encoding: .utf8) else { return nil } + + return text.trimmingCharacters(in: .whitespacesAndNewlines) + } + + /// The fields of the last interval the app traced under `name`. + /// + /// The app writes one JSON object per line into the state directory it was + /// pointed at, which is how a test reaches a fact about the app that leaves no + /// mark on screen. Live re-wrapping is one: whether text re-wrapped *during* a + /// window drag or only once it ended is invisible afterwards, because both end + /// with the text correct. + /// + /// Numbers come back as `Double` whatever the app wrote, since JSON does not + /// distinguish them and a caller comparing counts does not care. + /// + /// `nil` when the app has traced nothing under that name. + func lastTracedInterval(named name: String) -> [String: Double]? { + let file = root.appendingPathComponent("state/trace.jsonl") + guard let text = try? String(contentsOf: file, encoding: .utf8) else { return nil } + + for line in text.split(separator: "\n").reversed() { + guard + let data = line.data(using: .utf8), + let object = try? JSONSerialization.jsonObject(with: data) as? [String: Any], + let fields = object["fields"] as? [String: Any], + fields["message"] as? String == name + else { continue } + + return fields.compactMapValues { $0 as? Double ?? ($0 as? Int).map(Double.init) } + } + + return nil + } + + /// The text the app last copied, or `nil` if it has copied nothing. + /// + /// Reads the fixture's own pasteboard, never the system one. That is what + /// makes checking Copy Link safe, and it is enforced rather than trusted: + /// `ClipboardPolicyTests` fails on any mention of the system pasteboard in + /// this directory. + func copiedText() -> String? { + NSPasteboard(name: NSPasteboard.Name(pasteboardName)).string(forType: .string) + } + + func remove() { + // A named pasteboard outlives the process that made one, so this run's + // is handed back rather than left for the pasteboard server to keep. + NSPasteboard(name: NSPasteboard.Name(pasteboardName)).releaseGlobally() + + try? FileManager.default.removeItem(at: root) + } +} + +/// One conversation to write into a fixture. +/// +/// Every value is fixed by the test that builds it, including the ID: JP mints +/// one from the wall clock, and a test that did the same could not name the row +/// it wanted afterwards. +struct FixtureConversation { + /// The decisecond timestamp identifying the conversation. + /// + /// Also its directory name. JP writes `-`, but the loader + /// finds a conversation by the ID prefix, so the bare ID is enough and saves + /// reproducing the slug rule here. + let id: String + + /// The title, shown as the row's first line. + let title: String + + /// When the conversation was last activated, in JP's stored spelling. + /// + /// This is what the list sorts on, most recent first. + let lastActivatedAt: String + + /// When the conversation was pinned, in JP's stored spelling, or `nil` for a + /// conversation that is not pinned. + /// + /// Left out of `metadata.json` entirely when `nil`, which is how JP stores an + /// unpinned conversation and what the app's decoder reads as "not pinned". + var pinnedAt: String? + + /// The stored event stream, oldest first. + let events: [[String: String]] + + /// A message from the user, as storage holds one. + static func userMessage( + at timestamp: String, from author: String, _ text: String + ) -> [String: String] { + ["timestamp": timestamp, "type": "chat_request", "author": author, "content": text] + } + + /// A message from the assistant, as storage holds one. + static func assistantMessage(at timestamp: String, _ text: String) -> [String: String] { + ["timestamp": timestamp, "type": "chat_response", "message": text] + } + + /// The `jp://` URI the app copies and drags for this conversation. + var uri: String { + "jp://\(id)" + } + + /// What the row's second line reads, pluralized the way the app does. + var eventCountLabel: String { + events.count == 1 ? "1 event" : "\(events.count) events" + } + + /// Write the conversation into a workspace's `.jp` store. + fileprivate func write(into store: URL) throws { + let directory = + store + .appendingPathComponent("conversations") + .appendingPathComponent(id) + try FileManager.default.createDirectory( + at: directory, withIntermediateDirectories: true) + + var metadata = ["title": title, "last_activated_at": lastActivatedAt] + if let pinnedAt { + metadata["pinned_at"] = pinnedAt + } + + try Self.writeJSON(metadata, to: directory.appendingPathComponent("metadata.json")) + + try Self.baseConfig.write( + to: directory.appendingPathComponent("base_config.json"), + atomically: true, + encoding: .utf8 + ) + + try Self.writeJSON(events, to: directory.appendingPathComponent("events.json")) + } + + /// The smallest `base_config.json` a conversation can be stored with. + /// + /// Its presence tells the loader the conversation is in the current storage + /// format, and its contents have to finalize into a whole config: a + /// conversation whose base config is empty fails to load, and the app shows + /// "Could Not Read Conversation" where the transcript belongs. These two + /// settings are the ones with no default to fall back on. + /// + /// The same string is pinned in `crates/jp_ffi/src/lib_tests.rs`, which + /// reads this exact layout back through the library the app calls. That + /// test is what names a newly required setting, in seconds; here the same + /// breakage looks like a UI test waiting on a pane that never fills. + private static let baseConfig = """ + {"assistant":{"model":{"id":{"provider":"anthropic","name":"test"}}},\ + "conversation":{"tools":{"*":{"run":"ask"}}}} + """ + + private static func writeJSON(_ value: Any, to url: URL) throws { + let data = try JSONSerialization.data(withJSONObject: value) + try data.write(to: url) + } +} diff --git a/apps/macos/project.yml b/apps/macos/project.yml index 9105ade0b..ff3045a04 100644 --- a/apps/macos/project.yml +++ b/apps/macos/project.yml @@ -135,12 +135,34 @@ targets: TEST_HOST: $(BUILT_PRODUCTS_DIR)/JP.app/Contents/MacOS/JP BUNDLE_LOADER: $(TEST_HOST) + # The regression half of `QA.md`: the app is launched, acted on, and read back + # through its accessibility tree, which is what reaches menu enablement, the + # pasteboard, and terminate-and-relaunch. + # + # A UI test runs in its own process, so `@testable import JP` is not available + # here and must not be reached for. Anything that can be checked in-process + # belongs in JPTests, where it runs in milliseconds. + JPUITests: + type: bundle.ui-testing + platform: macOS + sources: + - path: UITests + dependencies: + - target: JP + settings: + base: + PRODUCT_BUNDLE_IDENTIFIER: computer.jp.jean-pierre.uitests + GENERATE_INFOPLIST_FILE: YES + # Which app `XCUIApplication()` addresses when constructed without one. + TEST_TARGET_NAME: JP + schemes: JP: build: targets: JP: all JPTests: [test] + JPUITests: [test] run: config: Debug # The workspace to open. Phase 2 has no file chooser, so the path comes @@ -156,3 +178,4 @@ schemes: gatherCoverageData: false targets: - JPTests + - JPUITests diff --git a/justfile b/justfile index 9981f9d3f..707a634ed 100644 --- a/justfile +++ b/justfile @@ -357,6 +357,36 @@ test-app: gen-app (build-ffi "debug") xcodebuild test -project apps/macos/JP.xcodeproj -scheme JP \ -destination platform=macOS -only-testing:JPTests -quiet +# Run every one of the macOS app's UI tests. +# +# Takes over the screen for the length of the run. This is the CI job; while +# writing a test, run it by name through the `swift_test_ui` tool instead, which +# stops at the first failure. +# +# Every test runs here even after one fails, which is what `CI` means to that +# tool and what a run nobody is watching should do. +[group('test')] +[macos] +test-app-ui: gen-app (build-ffi "debug") + CI=1 xcodebuild test -project apps/macos/JP.xcodeproj -scheme JP \ + -destination platform=macOS -only-testing:JPUITests -quiet + +# Format the macOS app's Swift sources. +[group('fmt')] +[macos] +fmt-app: + swift format --in-place --recursive --parallel \ + apps/macos/Sources apps/macos/Tests apps/macos/UITests \ + apps/macos/Tools/jpdrive/Sources apps/macos/Tools/jpdrive/Tests + +# Check Swift formatting and lints without rewriting anything. +[group('check')] +[macos] +lint-app: + swift format lint --strict --recursive --parallel \ + apps/macos/Sources apps/macos/Tests apps/macos/UITests \ + apps/macos/Tools/jpdrive/Sources apps/macos/Tools/jpdrive/Tests + [group('profile')] [positional-arguments] profile-heap *ARGS: From a4d15885ad803a23c0d64a4324d2e5c988183f72 Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Fri, 21 Aug 2026 23:45:11 +0200 Subject: [PATCH 2/2] review feedback Signed-off-by: Jean Mertz --- apps/macos/Sources/FixedWindowFrame.swift | 98 +++++++++++++++++++ apps/macos/Sources/WorkspaceWindow.swift | 4 + apps/macos/Tests/FixedWindowFrameTests.swift | 19 ++++ apps/macos/UITests/AppUnderTest.swift | 74 +++++++++++++- .../macos/UITests/ConversationListTests.swift | 26 ++++- apps/macos/UITests/PointerCursorTests.swift | 40 +++++--- apps/macos/UITests/Quiescence.swift | 8 +- .../macos/UITests/TranscriptReflowTests.swift | 19 ++-- justfile | 93 ++++++++++++------ 9 files changed, 323 insertions(+), 58 deletions(-) create mode 100644 apps/macos/Sources/FixedWindowFrame.swift create mode 100644 apps/macos/Tests/FixedWindowFrameTests.swift diff --git a/apps/macos/Sources/FixedWindowFrame.swift b/apps/macos/Sources/FixedWindowFrame.swift new file mode 100644 index 000000000..2479e5c46 --- /dev/null +++ b/apps/macos/Sources/FixedWindowFrame.swift @@ -0,0 +1,98 @@ +import AppKit +import SwiftUI + +/// Pins the window to a known frame, when the environment asks for one. +/// +/// For tests, and inert otherwise: without `JP_WINDOW_FRAME` set, nothing here +/// runs and the window behaves as any other, remembering where it was left. +/// +/// That memory is the problem it exists for. A window's frame is autosaved into +/// user defaults, which is a different mechanism from the saved application +/// state `-ApplePersistenceIgnoreState` disables — so a UI test suite can turn +/// off state restoration, as this one does, and still inherit the size the last +/// run left behind. A test that resizes the window then hands the next run a +/// different starting point, and one that grows the window eventually hands it +/// a window with nowhere left to grow. +struct FixedWindowFrame: ViewModifier { + /// The frame to pin to, as `x`. + /// + /// `nonisolated` because a name is not UI state: a `ViewModifier` is + /// main-actor isolated, which would otherwise follow this string everywhere + /// it is read. + nonisolated static let environmentKey = "JP_WINDOW_FRAME" + + /// The requested frame, if the environment names a usable one. + private static var requested: CGSize? { + guard let value = ProcessInfo.processInfo.environment[environmentKey] else { + return nil + } + + let parts = value.split(separator: "x") + guard parts.count == 2, + let width = Double(parts[0]), + let height = Double(parts[1]), + width > 0, + height > 0 + else { + return nil + } + + return CGSize(width: width, height: height) + } + + func body(content: Content) -> some View { + guard let size = Self.requested else { + return AnyView(content) + } + + return AnyView(content.background(WindowPinner(size: size))) + } +} + +extension View { + /// Pin the window to the frame `JP_WINDOW_FRAME` names, if it names one. + func fixedWindowFrame() -> some View { + modifier(FixedWindowFrame()) + } +} + +/// Reaches the `NSWindow` behind a SwiftUI scene, to set its frame once. +private struct WindowPinner: NSViewRepresentable { + let size: CGSize + + func makeNSView(context: Context) -> NSView { + let view = WindowPinningView() + view.pin = size + return view + } + + func updateNSView(_ view: NSView, context: Context) {} +} + +/// A view that acts the moment it is put in a window. +/// +/// `viewDidMoveToWindow` rather than `updateNSView`, which is called when SwiftUI +/// decides to and can run before the view has a window at all — and then not +/// again, if nothing else changes. +private final class WindowPinningView: NSView { + var pin: CGSize? + + override func viewDidMoveToWindow() { + super.viewDidMoveToWindow() + + guard let window, let pin else { return } + + // Emptying the autosave name is what stops this run from writing its + // size back over the default the next one reads. + _ = window.setFrameAutosaveName("") + window.setContentSize(pin) + + // Placed toward the left of the screen rather than centred, so a test + // dragging the right edge outwards has room whatever the screen size. + if let visible = window.screen?.visibleFrame { + window.setFrameOrigin( + CGPoint(x: visible.minX + 40, y: visible.maxY - window.frame.height - 40) + ) + } + } +} diff --git a/apps/macos/Sources/WorkspaceWindow.swift b/apps/macos/Sources/WorkspaceWindow.swift index 7074ab00a..da1498f3b 100644 --- a/apps/macos/Sources/WorkspaceWindow.swift +++ b/apps/macos/Sources/WorkspaceWindow.swift @@ -140,6 +140,10 @@ struct WorkspaceWindow: View { // to the scene, because the scene's `defaultSize` applies to a window // opened fresh and not to one restored into a saved frame. .frame(minWidth: Self.minimumWindowWidth, minHeight: Self.minimumWindowHeight) + // Inert unless `JP_WINDOW_FRAME` is set, which only a test does. Frame + // autosave outlives `-ApplePersistenceIgnoreState`, so without this a + // suite inherits the window size its last run left behind. + .fixedWindowFrame() // Carried but not displayed: the window has no title bar to show it in. // It is still what the Window menu lists the window under, and what an // external driver addresses it by. diff --git a/apps/macos/Tests/FixedWindowFrameTests.swift b/apps/macos/Tests/FixedWindowFrameTests.swift new file mode 100644 index 000000000..2171dc745 --- /dev/null +++ b/apps/macos/Tests/FixedWindowFrameTests.swift @@ -0,0 +1,19 @@ +import Testing + +@testable import JP + +/// The window frame a UI test pins the app to. +@Suite("FixedWindowFrame") +struct FixedWindowFrameTests { + /// A UI test bundle drives the app from another process and cannot import + /// it, so `AppUnderTest` spells this key out as a literal. Renaming the app's + /// constant without changing that literal would leave every launch inheriting + /// the previous run's window size again — silently, because an unset variable + /// means "behave normally". + /// + /// This is the only thing holding the two spellings together. + @Test("is read from the variable the UI tests set") + func keyMatchesTheOneUITestsSet() { + #expect(FixedWindowFrame.environmentKey == "JP_WINDOW_FRAME") + } +} diff --git a/apps/macos/UITests/AppUnderTest.swift b/apps/macos/UITests/AppUnderTest.swift index c884fcd80..a59d68b2a 100644 --- a/apps/macos/UITests/AppUnderTest.swift +++ b/apps/macos/UITests/AppUnderTest.swift @@ -86,6 +86,20 @@ struct AppUnderTest { /// covers slower work rather than raising it here. static let timeout: TimeInterval = 1 + /// The frame every launched app starts at, as `x`. + /// + /// Small enough to leave room on any screen worth running tests on, for a + /// test that drags the window wider. Large enough to show a conversation. + static let windowFrame = "1000x700" + + /// The variable the app reads that frame from. + /// + /// Spelled out rather than referenced: a UI test bundle drives the app from + /// another process and cannot import it. `FixedWindowFrameTests` in the unit + /// tests pins the app's own constant to this string, so renaming one without + /// the other fails there. + static let windowFrameKey = "JP_WINDOW_FRAME" + /// Launch against `fixture` and wait until the conversation list is on /// screen. /// @@ -107,10 +121,28 @@ struct AppUnderTest { } let app = XCUIApplication() - app.launchEnvironment = fixture.environment + + // The menu titles AppKit supplies — Edit, View, Window, Enter Full + // Screen, Merge All Windows — are localized, and this suite addresses + // them by their English names. Unpinned, they are whatever the machine + // running the tests prefers, and every menu test fails on a Mac set to + // another language. + app.launchArguments = ["-AppleLanguages", "(en)", "-AppleLocale", "en_US"] + + var environment = fixture.environment if !keepingWindowState { - app.launchArguments = ["-ApplePersistenceIgnoreState", "YES"] + app.launchArguments += ["-ApplePersistenceIgnoreState", "YES"] + + // Saved state and autosaved window frames are two mechanisms, and the + // launch argument above only disables the first. Without this the + // window opens at whatever size the previous run left it, so a suite + // that resizes hands the next one a different starting point — and one + // that grows the window eventually hands it a window against the edge + // of the screen with nowhere left to grow. + environment[Self.windowFrameKey] = Self.windowFrame } + + app.launchEnvironment = environment app.launch() let driven = AppUnderTest(app: app) @@ -517,6 +549,44 @@ struct AppUnderTest { return false } + /// Wait until `fixture`'s pasteboard holds `text`. + /// + /// Polled rather than read once. The quiescence waits are off, so a + /// synthesized click returns before the app has finished handling it — + /// measured at 71ms of app work after `click()` had already come back — and + /// reading the pasteboard on the next statement races that work. + /// + /// Slept between reads, unlike the element waits: a pasteboard read is cheap + /// enough to spin thousands of times a second, and the loop would compete + /// with the app for the core it is waiting on. + @discardableResult + func expectCopied( + _ text: String, + to fixture: WorkspaceFixture, + timeout: TimeInterval = AppUnderTest.timeout, + sourceLocation: SourceLocation = #_sourceLocation + ) -> Bool { + let deadline = Date().addingTimeInterval(timeout) + var last: String? + + repeat { + last = fixture.copiedText() + if last == text { + return true + } + Thread.sleep(forTimeInterval: 0.01) + } while Date() < deadline + + record( + """ + the pasteboard never held \(text.debugDescription) within \(timeout)s. \ + It held \(last.map { $0.debugDescription } ?? "nothing"). + """, + sourceLocation: sourceLocation + ) + return false + } + /// Record a failure, in both places a reader might look. /// /// `Issue.record` alone is not enough under `xcodebuild`, which prints the diff --git a/apps/macos/UITests/ConversationListTests.swift b/apps/macos/UITests/ConversationListTests.swift index 8bc1908e5..3353c170e 100644 --- a/apps/macos/UITests/ConversationListTests.swift +++ b/apps/macos/UITests/ConversationListTests.swift @@ -79,6 +79,21 @@ extension UISuite { driven.filter.click() driven.filter.typeText("Release") + // The suite shares one app, and every test after this one addresses + // a row by identifier. A filter left set hides most of them, so a + // failure here would fail the rest of the suite for a reason that + // has nothing to do with what they check — and the guard below is + // exactly the path a filtering regression takes. + // + // Cleared by keyboard rather than through the button, because the + // button is the other half of what this test is checking. Costs a + // few hundred milliseconds on the passing path, which is worth it. + defer { + driven.filter.click() + driven.app.typeKey("a", modifierFlags: .command) + driven.app.typeKey(.delete, modifierFlags: []) + } + guard driven.expectDisappears(hidden, "the Reading list row, once filtered") else { return } @@ -168,7 +183,7 @@ extension UISuite { driven.row(ConversationFixtures.readingList).rightClick() driven.chooseContextMenuItem("Copy Link") - #expect(fixture.copiedText() == ConversationFixtures.readingList.uri) + driven.expectCopied(ConversationFixtures.readingList.uri, to: fixture) } /// Edit ▸ Copy Link acts on the sidebar selection, so it is greyed out @@ -192,14 +207,21 @@ extension UISuite { driven.expectDisappears(driven.transcript, "the transcript, after Escape") else { return } + // Presence and enablement asserted separately. `menuItemIsEnabled` + // is false for an item that is absent as well as one that is greyed + // out, so on its own it would pass a regression that leaves the item + // out entirely while nothing is selected and inserts it once + // something is — the copy below would then succeed and the whole + // test would agree. let edit = driven.openMenu("Edit") + #expect(driven.menuItemExists("Copy Link", in: edit)) #expect(driven.menuItemIsEnabled("Copy Link", in: edit) == false) driven.closeMenu() driven.row(ConversationFixtures.configPipeline).click() driven.chooseMenuItem("Copy Link", in: "Edit") - #expect(fixture.copiedText() == ConversationFixtures.configPipeline.uri) + driven.expectCopied(ConversationFixtures.configPipeline.uri, to: fixture) } /// The window holds its two panes itself rather than in a diff --git a/apps/macos/UITests/PointerCursorTests.swift b/apps/macos/UITests/PointerCursorTests.swift index 498e12c1c..4bb32fca2 100644 --- a/apps/macos/UITests/PointerCursorTests.swift +++ b/apps/macos/UITests/PointerCursorTests.swift @@ -21,12 +21,20 @@ extension UISuite { @Suite("PointerCursor") @MainActor struct PointerCursorTests { + /// How long to let the window server settle before reading the cursor for + /// the baseline. + /// + /// Shorter than the usual timeout because the baseline is a negative: a + /// healthy run spends the whole of it confirming that nothing happened. + private static let baselineSettle: TimeInterval = 1 + /// The pointer becomes the horizontal-resize cursor over the strip that /// resizes the sidebar. /// - /// Dragging that strip works, and the view asks for the right cursor over - /// the right area, and the pointer still does not change — the request is - /// made and not delivered. Until this passes, that is unfixed. + /// This was believed unfixed — the view asking for a cursor it never got. + /// It was not: the baseline below could never be met, so the run always + /// stopped before reaching the assertion, and nobody had seen the result + /// it was reporting. `pointerStyle(.columnResize)` delivers. @Test("shows the horizontal-resize cursor over the pane divider") func showsResizeCursorOverTheDivider() { let fixture = try? ConversationFixtures.make() @@ -44,25 +52,32 @@ extension UISuite { // The baseline, and it is not optional. `NSCursor.currentSystem` reads // the cursor for the whole machine, so a column-resize cursor left // showing by anything at all would pass the assertion below without - // this app having done a thing. Establishing the arrow first turns "it - // is the right cursor" into "it changed to the right cursor". + // this app having done a thing. + // + // Asked as "not already the resize cursor" rather than "is the arrow", + // because the arrow cannot be recognised: `NSCursor.arrow.image` does + // not match the bytes the system reports while showing it, so + // `waitForCursor(.arrow)` never returns true and the baseline could + // never be met. See `cursorIs`. // // The conversation list rather than the transcript: the transcript does // not exist until something is selected, and hovering a missing element // fails without stopping the test — which is how an earlier version of // this passed with no baseline at all. driven.sidebar.hover() - let arrowFirst = driven.waitForCursor(.arrow) + let alreadyResizing = driven.waitForCursor( + .columnResize, + timeout: Self.baselineSettle + ) #expect( - arrowFirst, + !alreadyResizing, """ - over the conversation list the pointer was \(driven.describeCursor()) \ - rather than the arrow, so this run cannot say whether the divider \ - changed anything. + the column-resize cursor was already showing over the conversation \ + list, so this run cannot say whether the divider changed anything. """ ) - guard arrowFirst else { return } + guard !alreadyResizing else { return } driven.divider.hover() @@ -75,8 +90,7 @@ extension UISuite { changed, """ the pointer over the pane divider did not become the column-resize \ - cursor. It stayed \(driven.describeCursor()). The strip drags \ - correctly, so the gesture reaches it and the pointer does not. + cursor. It stayed \(driven.describeCursor()). """ ) } diff --git a/apps/macos/UITests/Quiescence.swift b/apps/macos/UITests/Quiescence.swift index d864cefc3..afc97f77b 100644 --- a/apps/macos/UITests/Quiescence.swift +++ b/apps/macos/UITests/Quiescence.swift @@ -50,9 +50,11 @@ enum Quiescence { /// gained an argument, or that returns something other than `void`, would /// be called with the wrong frame and go wrong somewhere unrelated. `v` is /// void, `@0:8` the receiver and selector every method takes, and each `B` - /// a `_Bool` argument. `B` rather than `c` also pins this to a machine - /// where `BOOL` is `_Bool` — the Swift `Bool` the blocks below are written - /// with matches that and not the `signed char` an Intel Mac would want. + /// a `_Bool` argument. + /// + /// `B` rather than `c` is not an architecture assumption: these parameters + /// are `_Bool`, not `BOOL`, so they encode as `B` under x86_64 as well. + /// Verified by reading the encoding out of the x86_64 slice under Rosetta. private static func install() -> String? { guard let process: AnyClass = NSClassFromString(className) else { return "XCTest no longer has a class named \(className)." diff --git a/apps/macos/UITests/TranscriptReflowTests.swift b/apps/macos/UITests/TranscriptReflowTests.swift index 8aaae0069..efb601811 100644 --- a/apps/macos/UITests/TranscriptReflowTests.swift +++ b/apps/macos/UITests/TranscriptReflowTests.swift @@ -16,10 +16,9 @@ extension UISuite { /// How far the drag moves the window's right edge, in points. /// - /// Outwards. A window opened fresh sits at its minimum width, because the - /// scene names no default size and SwiftUI takes the smallest its content - /// allows — so a drag inwards has nowhere to go, moves the pointer, resizes - /// nothing, and delivers no frames at all. + /// Outwards, which is always possible: every launch pins the window to + /// ``AppUnderTest/windowFrame`` at the left of the screen, so there is room + /// to the right of it whatever the last run did. private static let dragBy: CGFloat = 220 /// The whole point: text re-wraps on every frame of a window drag, not @@ -100,16 +99,18 @@ extension UISuite { /// Drag the window's right edge outwards, once. /// - /// One direction and no attempt to put the window back. A coordinate is + /// One gesture, and no attempt to put the window back. A coordinate is /// resolved against its element's frame at the moment it is *used*, not /// when it is made, so a second gesture written against the same two /// coordinates re-resolves both against the window the first one just - /// narrowed: the return drag starts inside the window body and pulls a + /// moved: the return drag starts inside the window body and pulls a /// stretch of empty transcript instead of the edge. /// - /// Nothing needs the width restored. This suite launches its own app and - /// terminates it, and the assertion is about the frames during the drag - /// rather than the size it ended on. + /// Nothing needs the width restored, and nothing depends on where the last + /// run left it: every launch pins the frame. Before that, the runs walked + /// the window rightwards until it met the screen, after which this drag + /// moved the pointer, resized nothing, delivered no frames, and failed with + /// "the app traced no window drag". /// /// The window is raised by the click that preceded this, so the edge is /// where the tree says it is. diff --git a/justfile b/justfile index 707a634ed..2266443f1 100644 --- a/justfile +++ b/justfile @@ -131,6 +131,9 @@ build-changelog: (_install "jilu@" + jilu_version) # Build the static library and C header that the macOS app links against, and # stage both where the Xcode project expects them. # +# Universal. The app declares no `ARCHS`, so Xcode builds it for +# `ARCHS_STANDARD` — arm64 and x86_64 — and a Release build links both slices. +# # Xcode runs this from a build phase, so `just` stays the single entry point for # building the Rust side rather than Xcode growing a competing one. # @@ -153,29 +156,38 @@ build-ffi PROFILE="debug": (_install "cbindgen@" + cbindgen_version) build_profile="{{PROFILE}}" fi - # Ask cargo which file it wrote rather than reconstructing the path. A - # configured build target (`[build] target` in `.cargo/config.toml`, or - # `CARGO_BUILD_TARGET`) inserts the triple into it, and `cargo metadata` - # reports only the outer target directory. The directory is redirectable - # too: sibling git worktrees here share one outside the checkout entirely. - # - # `json-render-diagnostics` and not `json`: the latter would send compiler - # errors down the pipe into `jq` instead of to the terminal. - # - # Deliberately not `{{quiet_flag}}`: a staticlib links the whole dependency - # graph, so a cold build runs long enough that silence reads as a hang. - # Cargo's status lines go to stderr and the JSON to stdout, so letting them - # through costs the pipe nothing. - lib=$(cargo build --package jp_ffi --profile "$build_profile" \ - --message-format=json-render-diagnostics | - jq -r 'select(.reason == "compiler-artifact" and .target.name == "jp_ffi") - | .filenames[] | select(endswith(".a"))' | - tail -n 1) - - if [ -z "$lib" ] || [ ! -f "$lib" ]; then - echo "cargo did not produce a jp_ffi static library" >&2 - exit 1 - fi + # Both slices, every time. A host-only library satisfies the Debug build on + # the machine that produced it and nothing else, so the gap stays invisible + # until somebody cuts a release or runs the UI suite under Rosetta — at + # which point it is a link error a long way from its cause. + slices="" + for target in aarch64-apple-darwin x86_64-apple-darwin; do + rustup target add "$target" >/dev/null + + # Ask cargo which file it wrote rather than reconstructing the path. The + # target directory is redirectable: sibling git worktrees here share one + # outside the checkout entirely. + # + # `json-render-diagnostics` and not `json`: the latter would send + # compiler errors down the pipe into `jq` instead of to the terminal. + # + # Deliberately not `{{quiet_flag}}`: a staticlib links the whole + # dependency graph, so a cold build runs long enough that silence reads + # as a hang. Cargo's status lines go to stderr and the JSON to stdout, + # so letting them through costs the pipe nothing. + slice=$(cargo build --package jp_ffi --profile "$build_profile" \ + --target "$target" --message-format=json-render-diagnostics | + jq -r 'select(.reason == "compiler-artifact" and .target.name == "jp_ffi") + | .filenames[] | select(endswith(".a"))' | + tail -n 1) + + if [ -z "$slice" ] || [ ! -f "$slice" ]; then + echo "cargo did not produce a jp_ffi static library for $target" >&2 + exit 1 + fi + + slices="$slices $slice" + done # Stage into a fixed, checkout-local directory. Xcode's search paths are # static build settings, so they need one location that does not move with @@ -183,15 +195,15 @@ build-ffi PROFILE="debug": (_install "cbindgen@" + cbindgen_version) out="apps/macos/.build/{{PROFILE}}" mkdir -p "$out/include" - # A debug staticlib bundles every dependency, so skip the copy when the - # staged one is already current. - if [ ! -f "$out/libjp_ffi.a" ] || [ "$lib" -nt "$out/libjp_ffi.a" ]; then - cp "$lib" "$out/libjp_ffi.a" + # A debug staticlib bundles every dependency, so joining the slices is worth + # skipping when what is staged is already newer than both of them. + if [ ! -f "$out/libjp_ffi.a" ] || [ -n "$(find $slices -newer "$out/libjp_ffi.a")" ]; then + lipo -create -output "$out/libjp_ffi.a" $slices fi cbindgen --config crates/jp_ffi/cbindgen.toml --crate jp_ffi --output "$out/include/jp_ffi.h" - echo "library: $out/libjp_ffi.a" >&2 + echo "library: $out/libjp_ffi.a ($(lipo -archs "$out/libjp_ffi.a"))" >&2 echo "header: $out/include/jp_ffi.h" >&2 # Build the `jpdrive` accessibility driver that the `debug_app_*` tools shell out @@ -365,11 +377,34 @@ test-app: gen-app (build-ffi "debug") # # Every test runs here even after one fails, which is what `CI` means to that # tool and what a run nobody is watching should do. +# +# The result bundle is written into the checkout rather than left in derived +# data, so a failing run leaves its evidence somewhere a reader or a CI artifact +# step can reach without deriving a container path. `swift_test_ui` writes to +# the same place for the same reason. [group('test')] [macos] test-app-ui: gen-app (build-ffi "debug") + #!/usr/bin/env sh + set -eu + + # Not tidying up: `xcodebuild` refuses to write over an existing bundle, so + # without this the second run in a checkout fails before it starts. + rm -rf tmp/uitests/run.xcresult + mkdir -p tmp/uitests + + # Captured rather than propagated, so the bundle is still reported on the + # failing run — which is the only run anybody opens it for. + status=0 CI=1 xcodebuild test -project apps/macos/JP.xcodeproj -scheme JP \ - -destination platform=macOS -only-testing:JPUITests -quiet + -destination platform=macOS -only-testing:JPUITests \ + -resultBundlePath tmp/uitests/run.xcresult -quiet || status=$? + + if [ -d tmp/uitests/run.xcresult ]; then + echo "result bundle: tmp/uitests/run.xcresult" >&2 + fi + + exit $status # Format the macOS app's Swift sources. [group('fmt')]