From 4da61465b0aa23aebca1492c8d57c7665324d33d Mon Sep 17 00:00:00 2001 From: Storm Dragon Date: Tue, 3 Nov 2026 11:37:36 -0500 Subject: [PATCH] Fix document retention and helper retry regressions Keep failed cursor trials and native recapture anchored to the original document, and restore the requested cached root before LRU eviction. Base braille retry resets on established connection time rather than stalled startup. Poll private AT-SPI daemon ownership without triggering service activation and clean up partial startup failures. Add focused regressions and update browser testing notes. --- docs/testing/browser-ticking-pages.md | 19 ++++- internal/app/braille_helper_test.go | 16 ++-- internal/app/braille_session.go | 46 ++++++----- internal/atspi/adapter.go | 45 ++++++---- internal/atspi/adapter_references_native.go | 52 ++---------- internal/atspi/adapter_tree_test.go | 56 +++++++++++++ internal/review/document_worker.go | 4 + internal/review/document_worker_test.go | 32 ++++++++ internal/version/version.go | 2 +- tests/native_browser.py | 91 +++++++++++---------- tests/test_native_browser.py | 51 ++++++++++++ 11 files changed, 282 insertions(+), 132 deletions(-) create mode 100644 tests/test_native_browser.py diff --git a/docs/testing/browser-ticking-pages.md b/docs/testing/browser-ticking-pages.md index 83ad07e..72cc8eb 100644 --- a/docs/testing/browser-ticking-pages.md +++ b/docs/testing/browser-ticking-pages.md @@ -69,8 +69,9 @@ before a caret move commits, and the tests now prove the live provider text is what the move reports. Structural navigation keeps strict validation everywhere. Capture root selection also learned to resolve a focus identity from retained document sources (a document root after a -load may have no focus-event history) and to keep scanning focus history -past chrome entries when recovering a dead browse focus. Provider-side +load may have no focus-event history). A dead browse focus recovers only +through the root of the capture that retained that source; other tabs on +the same application bus cannot supply its recovery anchor. Provider-side stalls remain possible: one log ends with two 15 s capture deadlines from an unresponsive game tab, which no validation change can fix. @@ -109,7 +110,11 @@ stashed), events keep invalidating stashed trees while inactive, and the worker retains a browse cursor per retained tree. A move first trials retained cursors (cheap in-memory syncs) before capturing, so an unchanged document resumes with no capture at all and a changed one recaptures warm -(only dirty nodes re-read). Tab switches suspend instead of hard-resetting +(only dirty nodes re-read). Failed trials preserve the original cursor for +recapture, including when its focused child was removed. Restoring the +oldest retained slot at capacity removes it before stashing the active +document, so restoration cannot evict its own target. +Tab switches suspend instead of hard-resetting documents; page loads still hard-reset, and requests carry a load epoch so a cursor captured before a same-path load is never reused afterwards (`TestWorkerDocumentLoadRetiresOutstandingResult` still guards this). @@ -161,6 +166,14 @@ retention. touched. Observed but not addressed: empty fragment stops between the fixture's `
`-separated spans, a pre-existing line-assembly quirk. +Retention boundaries have focused regressions: +`TestDocumentSlotsRestoreOldestAtCapacity`, +`TestRetainedCursorTrialKeepsDeadFocusInOriginalDocument`, and +`TestRetainedDocumentRootDoesNotCrossTabsOnSameBus`. The browser harness +waits for daemon bus-name ownership without activating services, bounds +each readiness query, and joins both daemons on startup failure. +Run its startup checks with `python3 tests/test_native_browser.py -v`. + ## Live acceptance (pending, maintainer) Do not replace the running reader without approval; build to a side path diff --git a/internal/app/braille_helper_test.go b/internal/app/braille_helper_test.go index c3b4741..7812f84 100644 --- a/internal/app/braille_helper_test.go +++ b/internal/app/braille_helper_test.go @@ -55,8 +55,8 @@ func TestBrailleWorkerReportsHelperStartupFailure(t *testing.T) { worker := &brailleWorker{path: path, logger: slog.New(slog.DiscardHandler)} ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) defer cancel() - if err := worker.session(ctx); err == nil || !strings.Contains(err.Error(), "braille test startup failure") { - t.Fatalf("helper startup error = %v", err) + if connectedFor, err := worker.session(ctx); connectedFor != 0 || err == nil || !strings.Contains(err.Error(), "braille test startup failure") { + t.Fatalf("helper startup duration = %v, error = %v", connectedFor, err) } } @@ -110,7 +110,13 @@ func TestBrailleWorkerSessionWaitsForLateDisplay(t *testing.T) { worker := &brailleWorker{path: path, logger: slog.New(slog.DiscardHandler), wake: make(chan struct{}, 1)} ctx, cancel := context.WithCancel(context.Background()) done := make(chan error, 1) - go func() { done <- worker.session(ctx) }() + go func() { + connectedFor, err := worker.session(ctx) + if connectedFor != 0 { + err = fmt.Errorf("unready helper counted as connected for %v", connectedFor) + } + done <- err + }() select { case err := <-done: t.Fatalf("session gave up on a stalled connection attempt: %v", err) @@ -141,8 +147,8 @@ func TestBrailleWorkerSessionRecyclesWedgedHelper(t *testing.T) { worker := &brailleWorker{path: path, logger: slog.New(slog.DiscardHandler), wake: make(chan struct{}, 1), readyGiveUp: 300 * time.Millisecond} ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) defer cancel() - if err := worker.session(ctx); err == nil || !strings.Contains(err.Error(), "connection timed out") { - t.Fatalf("wedged helper error = %v", err) + if connectedFor, err := worker.session(ctx); connectedFor != 0 || err == nil || !strings.Contains(err.Error(), "connection timed out") { + t.Fatalf("wedged helper duration = %v, error = %v", connectedFor, err) } } diff --git a/internal/app/braille_session.go b/internal/app/braille_session.go index 0f836d4..8536ec7 100644 --- a/internal/app/braille_session.go +++ b/internal/app/braille_session.go @@ -45,8 +45,13 @@ func (worker *brailleWorker) run(ctx context.Context, done chan struct{}) { backoff := time.Duration(0) lastError := "" for ctx.Err() == nil { - started := time.Now() - err := worker.session(ctx) + connectedFor, err := worker.session(ctx) + // Only a healthy, established connection resets retry history. Waiting + // for the helper's ready message can itself take several minutes. + if connectedFor > 10*time.Second { + backoff = 0 + lastError = "" + } if err != nil && ctx.Err() == nil { if err.Error() != lastError { worker.logger.Warn("braille helper disconnected", "error", err) @@ -55,12 +60,6 @@ func (worker *brailleWorker) run(ctx context.Context, done chan struct{}) { worker.logger.Debug("braille helper still unavailable", "error", err) } } - // A session that stayed up a while was a real connection; do not - // keep growing the delay after it ends. - if time.Since(started) > 10*time.Second { - backoff = 0 - lastError = "" - } backoff = nextBrailleBackoff(backoff) select { case <-ctx.Done(): @@ -70,23 +69,31 @@ func (worker *brailleWorker) run(ctx context.Context, done chan struct{}) { } } -func (worker *brailleWorker) session(ctx context.Context) (result error) { +// connectedFor excludes startup waits so the supervisor can distinguish an +// established connection from a slow failed handshake. +func (worker *brailleWorker) session(ctx context.Context) (connectedFor time.Duration, result error) { + var connectedAt time.Time + defer func() { + if !connectedAt.IsZero() { + connectedFor = time.Since(connectedAt) + } + }() defer func() { worker.mu.Lock(); worker.connected = false; worker.mu.Unlock() }() childCtx, cancel := context.WithCancel(ctx) defer cancel() command := exec.CommandContext(childCtx, worker.path, "--client") stdin, err := command.StdinPipe() if err != nil { - return err + return 0, err } stdout, err := command.StdoutPipe() if err != nil { - return err + return 0, err } var stderr bytes.Buffer command.Stderr = &stderr if err := command.Start(); err != nil { - return err + return 0, err } waitDone := make(chan struct{}) var waitErr error @@ -154,17 +161,18 @@ readyWait: select { case err := <-ready: if err != nil { - return err + return 0, err } break readyWait case <-notice.C: worker.logger.Info("waiting for braille display") case <-giveUpTimer.C: - return errors.New("braille helper connection timed out") + return 0, errors.New("braille helper connection timed out") case <-ctx.Done(): - return nil + return 0, nil } } + connectedAt = time.Now() worker.logger.Info("braille helper connected") worker.mu.Lock() worker.connected = true @@ -198,15 +206,15 @@ readyWait: for { select { case <-waitDone: - return waitErr + return 0, waitErr case err := <-writeError: - return err + return 0, err case err := <-readErrors: - return err + return 0, err case event := <-routes: worker.route(childCtx, event) case <-ctx.Done(): - return nil + return 0, nil } } } diff --git a/internal/atspi/adapter.go b/internal/atspi/adapter.go index a9c4416..5015fe3 100644 --- a/internal/atspi/adapter.go +++ b/internal/atspi/adapter.go @@ -468,6 +468,22 @@ type stashedDocument struct { sources map[ObjectID]*C.AtspiAccessible } +// Source membership survives tree invalidation until the next capture, so it +// can identify the owner of a removed focus without guessing from its bus. +func (adapter *Adapter) retainedDocumentRoot(id ObjectID) ObjectID { + adapter.treeMutex.Lock() + defer adapter.treeMutex.Unlock() + if _, retained := adapter.nativeDocument[id]; retained { + return adapter.documentSlotRoot + } + for _, root := range adapter.stashedDocumentIDs { + if _, retained := adapter.stashedDocuments[root].sources[id]; retained { + return root + } + } + return ObjectID{} +} + // rotateDocumentSlots makes root's retained trees and native sources the // active document slots, stashing the previously active document. Capture // calls this on the GLib thread once the capture root is known, before @@ -481,6 +497,16 @@ func (adapter *Adapter) rotateDocumentSlots(root ObjectID) { if root == adapter.documentSlotRoot { return } + // Remove the requested slot before stashing the active one: at capacity, + // stashing first could evict the very document we are returning to. + stashed := adapter.stashedDocuments[root] + delete(adapter.stashedDocuments, root) + for index, id := range adapter.stashedDocumentIDs { + if id == root { + adapter.stashedDocumentIDs = append(adapter.stashedDocumentIDs[:index], adapter.stashedDocumentIDs[index+1:]...) + break + } + } if adapter.documentSlotRoot.Valid() && (adapter.documentTree != nil || adapter.documentTextTree != nil) { adapter.stashDocumentLocked(adapter.documentSlotRoot, stashedDocument{ @@ -492,22 +518,9 @@ func (adapter *Adapter) rotateDocumentSlots(root ObjectID) { C.g_object_unref(C.gpointer(source)) } } - if stashed, ok := adapter.stashedDocuments[root]; ok { - delete(adapter.stashedDocuments, root) - for index, id := range adapter.stashedDocumentIDs { - if id == root { - adapter.stashedDocumentIDs = append(adapter.stashedDocumentIDs[:index], adapter.stashedDocumentIDs[index+1:]...) - break - } - } - adapter.documentTree = stashed.tree - adapter.documentTextTree = stashed.textTree - adapter.nativeDocument = stashed.sources - } else { - adapter.documentTree = nil - adapter.documentTextTree = nil - adapter.nativeDocument = nil - } + adapter.documentTree = stashed.tree + adapter.documentTextTree = stashed.textTree + adapter.nativeDocument = stashed.sources adapter.documentSlotRoot = root } diff --git a/internal/atspi/adapter_references_native.go b/internal/atspi/adapter_references_native.go index 2703ae5..8640415 100644 --- a/internal/atspi/adapter_references_native.go +++ b/internal/atspi/adapter_references_native.go @@ -21,52 +21,12 @@ func (adapter *Adapter) captureDesktop(ctx context.Context, reviewFocus ObjectID trace := capture_diagnostics(ctx) trace.begin_stage("root-selection", reviewFocus, "") root, err := adapter.nativeDocumentRoot(ctx, reviewFocus) - adapter.treeMutex.Lock() - retainedRoots := make(map[ObjectID]bool, 1+len(adapter.stashedDocumentIDs)) - if adapter.documentSlotRoot.Valid() && adapter.documentTree != nil { - retainedRoots[adapter.documentSlotRoot] = true - } - for _, id := range adapter.stashedDocumentIDs { - retainedRoots[id] = true - } - adapter.treeMutex.Unlock() - if (errors.Is(err, ErrObjectNotFound) || errors.Is(err, ErrStaleObject)) && len(retainedRoots) != 0 && len(adapter.nativeFocus) != 0 { - // Dynamic pages can remove the browse cursor without sending a new - // focus event. Recover only from the newest retained native focus in - // the same document. A chrome or other-tab focus is not a fallback. - for i := len(adapter.nativeFocus) - 1; i >= 0; i-- { - latest := adapter.nativeFocus[i] - if latest.id.BusName != reviewFocus.BusName { - break - } - C.atspi_accessible_clear_cache_single(latest.source) - states := C.atspi_accessible_get_state_set(latest.source) - if states == nil { - continue - } - dead := C.atspi_state_set_contains(states, C.ATSPI_STATE_DEFUNCT) != 0 - C.g_object_unref(C.gpointer(states)) - if dead { - continue - } - // A removed browse target can retain a non-defunct proxy after - // Chromium has dropped its accessible object. Its role is then - // invalid; try the preceding real focus instead. - if C.atspi_accessible_get_role(latest.source, nil) == C.ATSPI_ROLE_INVALID { - continue - } - candidate, candidateErr := adapter.nativeDocumentRoot(ctx, latest.id) - if candidateErr != nil { - continue - } - if retainedRoots[nativeObjectID(candidate)] { - root, err = candidate, nil - break - } - // The newest live focus can sit in browser chrome or a tab - // whose document was never retained; older focus history in - // the retained document may still resolve. - C.g_object_unref(C.gpointer(candidate)) + if errors.Is(err, ErrObjectNotFound) || errors.Is(err, ErrStaleObject) { + // A removed focus may still belong to a retained capture. Recover + // through that document's root, never through another tab's focus + // history merely because it shares the application's bus. + if owner := adapter.retainedDocumentRoot(reviewFocus); owner.Valid() && owner != reviewFocus { + root, err = adapter.nativeDocumentRoot(ctx, owner) } } if err != nil { diff --git a/internal/atspi/adapter_tree_test.go b/internal/atspi/adapter_tree_test.go index 186e907..3edde71 100644 --- a/internal/atspi/adapter_tree_test.go +++ b/internal/atspi/adapter_tree_test.go @@ -583,6 +583,62 @@ func TestDocumentSlotsRetainTreesAcrossRoots(t *testing.T) { } } +func TestDocumentSlotsRestoreOldestAtCapacity(t *testing.T) { + adapter := New() + var first *AccessibleTree + for index := range maxStashedDocuments + 1 { + root := NewObjectID(":1.42", fmt.Sprintf("/doc%d", index)) + adapter.rotateDocumentSlots(root) + snapshot := NewSnapshot(root.Path, "Document", "document web", "", "") + snapshot.BusName = root.BusName + tree, err := adapter.installCapturedTree(root, []TreeNode{{Snapshot: snapshot}}, true, false) + if err != nil { + t.Fatal(err) + } + if index == 0 { + first = tree + } + } + adapter.rotateDocumentSlots(NewObjectID(":1.42", "/doc0")) + if adapter.documentTree != first { + t.Fatal("returning to oldest retained document evicted it before restoration") + } + if len(adapter.stashedDocuments) != maxStashedDocuments { + t.Fatalf("stashed documents = %d, want %d", len(adapter.stashedDocuments), maxStashedDocuments) + } +} + +// These fixtures describe source membership without owning native handles. +func documentSourceKeys[T any](_ map[ObjectID]T, ids ...ObjectID) map[ObjectID]T { + sources := make(map[ObjectID]T, len(ids)) + var value T + for _, id := range ids { + sources[id] = value + } + return sources +} + +func TestRetainedDocumentRootDoesNotCrossTabsOnSameBus(t *testing.T) { + adapter := New() + active, inactive := testID("/active"), testID("/inactive") + adapter.documentSlotRoot = active + adapter.nativeDocument = documentSourceKeys(adapter.nativeDocument, testID("/active_child")) + adapter.stashedDocumentIDs = []ObjectID{inactive} + adapter.stashedDocuments = map[ObjectID]stashedDocument{ + inactive: {sources: documentSourceKeys(adapter.nativeDocument, testID("/removed_child"))}, + } + for _, item := range []struct{ focus, want ObjectID }{ + {testID("/active_child"), active}, + {testID("/removed_child"), inactive}, + {testID("/chrome"), ObjectID{}}, + {NewObjectID(":1.99", "/removed_child"), ObjectID{}}, + } { + if got := adapter.retainedDocumentRoot(item.focus); got != item.want { + t.Fatalf("owner of %v = %v, want %v", item.focus, got, item.want) + } + } +} + // A provider whose capture just timed out is probably still stuck. The // next document capture for that bus must fail fast with ErrProviderHung // instead of queueing another multi-second native stall, and the mark diff --git a/internal/review/document_worker.go b/internal/review/document_worker.go index 2b7d853..251dc12 100644 --- a/internal/review/document_worker.go +++ b/internal/review/document_worker.go @@ -121,6 +121,7 @@ func (worker *Worker) trialRetainedCursors(cursor **DocumentCursor, loadEpoch ui if !documentNeedsCapture(result.Err) { return } + originalCursor, originalErr := *cursor, result.Err for _, retained := range worker.retainedCursorsExcept(*cursor, loadEpoch) { *cursor = retained result.Err = move() @@ -128,6 +129,9 @@ func (worker *Worker) trialRetainedCursors(cursor **DocumentCursor, loadEpoch ui return } } + // Failed trials do not own the focus. Keep recapture anchored to the + // original document if its focused child disappeared in the meantime. + *cursor, result.Err = originalCursor, originalErr } // warmDocument captures and syncs the focused document so the first diff --git a/internal/review/document_worker_test.go b/internal/review/document_worker_test.go index b04c324..bb6db41 100644 --- a/internal/review/document_worker_test.go +++ b/internal/review/document_worker_test.go @@ -526,6 +526,38 @@ func TestDocumentCaretRecapturesThroughDocumentWhenFocusAnchorDies(t *testing.T) } } +func TestRetainedCursorTrialKeepsDeadFocusInOriginalDocument(t *testing.T) { + treeA := retainedDocumentTree(t, "/docA", "/a1", "/a2") + treeB := retainedDocumentTree(t, "/docB", "/b1", "/b2") + cursorA, _ := NewDocumentCursor(treeA) + cursorB, _ := NewDocumentCursor(treeB) + ctx := context.Background() + if _, err := cursorA.SyncFocus(ctx, webID("/a1")); err != nil { + t.Fatal(err) + } + if _, err := cursorB.SyncFocus(ctx, webID("/b1")); err != nil { + t.Fatal(err) + } + treeA.Invalidate(webID("/a1")) + capturer := &retainedDocumentCapturer{trees: map[atspi.ObjectID]*atspi.AccessibleTree{ + webID("/docA"): retainedDocumentTree(t, "/docA", "/a2"), + webID("/docB"): treeB, + }} + worker, err := NewWorker(capturer, time.Second) + if err != nil { + t.Fatal(err) + } + worker.SyncFocus(webID("/a1")) + worker.rememberDocumentCursor(cursorA, 0) + worker.rememberDocumentCursor(cursorB, 0) + result, _ := worker.moveDocument(ctx, cursorA, DocumentHeading, 0, false, 0, 0) + if result.Err != nil || !result.Moved || result.DocumentPosition.Document.ID() != webID("/docA") || + result.DocumentPosition.Object.ID() != webID("/a2") { + t.Fatalf("dead focus recovery: document=%v object=%v moved=%v error=%v captures=%v", + result.DocumentPosition.Document.ID(), result.DocumentPosition.Object.ID(), result.Moved, result.Err, capturer.captures) + } +} + func (reader *waitingCaretReader) ReadDocumentCaret(ctx context.Context, _ atspi.ObjectReference, _ int, _ atspi.DocumentCaretUnit) (atspi.DocumentCaretText, error) { reader.deadline, _ = ctx.Deadline() <-ctx.Done() diff --git a/internal/version/version.go b/internal/version/version.go index 24ae986..9a21679 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -22,7 +22,7 @@ import "strings" const Name = "Cthulhu" const Version = "2026.11.03" -const Channel = "testing" +const Channel = "hunter" var Build = Version diff --git a/tests/native_browser.py b/tests/native_browser.py index f006040..5dccbfc 100644 --- a/tests/native_browser.py +++ b/tests/native_browser.py @@ -59,6 +59,34 @@ def selected_proofs(): return selected +def stop_atspi_bus(processes): + for process in reversed(processes): + process.terminate() + try: + process.wait(timeout=5) + except subprocess.TimeoutExpired: + process.kill() + process.wait(timeout=5) + + +def wait_for_bus_name(process, busArgs, name): + deadline = time.monotonic() + 10 + while time.monotonic() < deadline: + if process.poll() is not None: + raise RuntimeError(name + " daemon exited before the probe") + # Query the bus itself: contacting the service can activate another + # daemon (or a user-manager unit) before our child owns its name. + result = subprocess.run( + ["gdbus", "call", *busArgs, "--dest", "org.freedesktop.DBus", + "--object-path", "/org/freedesktop/DBus", "--method", + "org.freedesktop.DBus.NameHasOwner", name], + capture_output=True, text=True, timeout=2) + if result.returncode == 0 and result.stdout.strip() == "(true,)": + return + time.sleep(0.1) + raise RuntimeError(name + " did not become ready") + + def start_atspi_bus(): # D-Bus activation of org.a11y.Bus and of the AT-SPI registry may # delegate to systemd user units, which cannot work in this private @@ -77,44 +105,29 @@ def start_atspi_bus(): launcher = locate("at-spi-bus-launcher", ("/usr/lib/at-spi-bus-launcher", "/usr/libexec/at-spi-bus-launcher")) - process = subprocess.Popen([launcher, "--launch-immediately"], - stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) - address = None - deadline = time.monotonic() + 10 - while time.monotonic() < deadline: - if process.poll() is not None: - raise RuntimeError("at-spi-bus-launcher exited before the probe") + registry = locate("at-spi2-registryd", + ("/usr/lib/at-spi2-registryd", "/usr/libexec/at-spi2-registryd")) + processes = [] + try: + process = subprocess.Popen([launcher, "--launch-immediately"], + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + processes.append(process) + wait_for_bus_name(process, ["--session"], "org.a11y.Bus") result = subprocess.run( ["gdbus", "call", "--session", "--dest", "org.a11y.Bus", "--object-path", "/org/a11y/bus", "--method", "org.a11y.Bus.GetAddress"], - capture_output=True, text=True) - if result.returncode == 0 and "unix:" in result.stdout: - address = result.stdout.split("'", 2)[1] - break - time.sleep(0.1) - if address is None: - process.terminate() - raise RuntimeError("AT-SPI bus did not become ready") - registry = locate("at-spi2-registryd", - ("/usr/lib/at-spi2-registryd", "/usr/libexec/at-spi2-registryd")) - registryd = subprocess.Popen([registry], - stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) - deadline = time.monotonic() + 10 - while time.monotonic() < deadline: - if registryd.poll() is not None: - process.terminate() - raise RuntimeError("at-spi2-registryd exited before the probe") - result = subprocess.run( - ["gdbus", "call", "--address", address, "--dest", "org.a11y.atspi.Registry", - "--object-path", "/org/a11y/atspi/registry", - "--method", "org.freedesktop.DBus.Peer.Ping"], - capture_output=True, text=True) - if result.returncode == 0: - return [process, registryd] - time.sleep(0.1) - registryd.terminate() - process.terminate() - raise RuntimeError("AT-SPI registry did not become ready") + capture_output=True, text=True, timeout=2, check=True) + address = result.stdout.split("'", 2)[1] + registryEnv = os.environ.copy() + registryEnv["AT_SPI_BUS_ADDRESS"] = address + registryd = subprocess.Popen([registry], env=registryEnv, + stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + processes.append(registryd) + wait_for_bus_name(registryd, ["--address", address], "org.a11y.atspi.Registry") + return processes + except BaseException: + stop_atspi_bus(processes) + raise def run_inside(): @@ -131,13 +144,7 @@ def run_inside(): # the same browser state whether run alone or after editing. run_browser_proof(browser, proof) finally: - for process in reversed(atspi): - process.terminate() - try: - process.wait(timeout=5) - except subprocess.TimeoutExpired: - process.kill() - process.wait(timeout=5) + stop_atspi_bus(atspi) def run_browser_proof(browser, proof): diff --git a/tests/test_native_browser.py b/tests/test_native_browser.py new file mode 100644 index 0000000..5b0a34b --- /dev/null +++ b/tests/test_native_browser.py @@ -0,0 +1,51 @@ +#!/usr/bin/env python3 +"""Check private browser daemon startup without contacting a desktop bus.""" +import subprocess +import unittest +from unittest.mock import Mock, patch + +import native_browser + + +class AtspiStartupTests(unittest.TestCase): + def test_readiness_queries_bus_without_activating_service(self): + process = Mock() + process.poll.return_value = None + replies = [Mock(returncode=0, stdout="(false,)"), + Mock(returncode=0, stdout="(true,)")] + with patch("native_browser.subprocess.run", side_effect=replies) as run, \ + patch("native_browser.time.sleep"): + native_browser.wait_for_bus_name(process, ["--session"], "org.a11y.Bus") + self.assertEqual(run.call_count, 2) + for call in run.call_args_list: + args = call.args[0] + self.assertEqual(args[args.index("--dest") + 1], "org.freedesktop.DBus") + self.assertEqual(args[-2:], ["org.freedesktop.DBus.NameHasOwner", "org.a11y.Bus"]) + self.assertEqual(call.kwargs["timeout"], 2) + + def test_registry_failure_joins_both_daemons(self): + launcher, registry = Mock(), Mock() + with patch("native_browser.shutil.which", side_effect=lambda name: "/private/" + name), \ + patch("native_browser.subprocess.Popen", side_effect=[launcher, registry]) as popen, \ + patch("native_browser.wait_for_bus_name", side_effect=[None, RuntimeError("registry failed")]), \ + patch("native_browser.subprocess.run", return_value=Mock(stdout="('unix:path=/private/bus',)")): + with self.assertRaisesRegex(RuntimeError, "registry failed"): + native_browser.start_atspi_bus() + self.assertEqual(popen.call_args.kwargs["env"]["AT_SPI_BUS_ADDRESS"], "unix:path=/private/bus") + for process in (launcher, registry): + process.terminate.assert_called_once_with() + process.wait.assert_called_once_with(timeout=5) + + def test_probe_timeout_joins_launcher(self): + launcher = Mock() + with patch("native_browser.shutil.which", return_value="/private/daemon"), \ + patch("native_browser.subprocess.Popen", return_value=launcher), \ + patch("native_browser.wait_for_bus_name", side_effect=subprocess.TimeoutExpired("gdbus", 2)): + with self.assertRaises(subprocess.TimeoutExpired): + native_browser.start_atspi_bus() + launcher.terminate.assert_called_once_with() + launcher.wait.assert_called_once_with(timeout=5) + + +if __name__ == "__main__": + unittest.main()