Skip to content

test: isolate hosted camera permission - #1117

Merged
ovitrif merged 1 commit into
masterfrom
test/isolate-hosted-camera-permission
Oct 24, 2026
Merged

ovitrif merged 1 commit into
masterfrom
test/isolate-hosted-camera-permission

Conversation

@ben-kaufman

Copy link
Copy Markdown
Contributor

Refs #1110

This PR prevents hosted send-navigation fixtures from leaving a system camera-permission alert behind for subsequent UI tests.

Description

  • Snapshot the shared camera permission value before hosting either send fixture, suppress system permission requests during the test, and restore the original value on the main actor during teardown. All 21 existing test methods and assertions remain unchanged.

Out of Scope

Design

N/A — no UI changes.

Preview

N/A — no user-visible change.

QA Notes

Setup

The controlled comparison used separate newly created, empty iPhone 17 simulators on iOS 26.5 with Xcode 26.1/SDK 26.1 and unchanged Package.resolved. Camera authorization began undecided; no permission response or privacy reset was issued. Build/test through XcodeBuildMCP with serial testing. Select SendQuickpayLnurlRemountTests and SendSheetLifecycleTests, then BitkitUITests/testExample; inspect the simulator and TCC logs between stages. No funded wallet is required.

Journeys

N/A — no user-visible behaviour change.

Manual Tests

N/A

Automated Checks

  • updated SendQuickpayLnurlRemountTests.swift — isolate the camera permission state used by six hosted remount cases.
  • updated SendSheetLifecycleTests.swift — isolate the same state used by fifteen hosted lifecycle cases.
  • ran controlled fresh-simulator comparison against e4bf44a2: base leaves a real camera alert after the triggering test and the subsequent passing launch; candidate passes all 21 cases and launch with no prompt request or leaked alert. TCC records only preflight queries for the candidate and no persisted camera authorization.

Evidence

Validated 1891f128 (the unchanged reviewed diff) against master e4bf44a2: 2,871 unit tests passed; 43 UI executions finished with 39 passes, four existing prerequisite skips and zero failures, using the six CI integration exclusions. SwiftFormat 0.64: 0/843 files require formatting; git diff --check passed. Complete diagnostics were inspected, including four SDK/Pubky constructor QoS stacks and retained simulator/fault-fixture messages; 99 emitting/context sources were compared with the base.

Both base and candidate 19-target builds passed. Their focused 21-case suites passed; all 39 runtime diagnostic records match the same five normalized groups/counts, including appearance-transition and socket notices. Production app dylib and UI test executable hashes match across the two builds. Independent review of the frozen two-file diff found no actionable issue.

Gaps

Local Xcode/SDK/runtime differ from failing CI's 16.4/18.5/26.2. This validates fixture isolation, not the daemon assertion-delay cause or a fully repaired #1054 run. The first build failed before compilation because cached VSS metadata pointed to an obsolete path; a private cache copy with corrected artifact paths, identical VSS files and verified pinned revisions built successfully. Runtime diagnostics are retained; no warning-free claim. The later canonical USDT scanner test requested camera normally and XCTest’s default interruption handler allowed it; the no-prompt/undecided-state result above is specifically the controlled fixture comparison. No financial authorization/payment, latency or hardware validation.

Models used

  • Planning/scoping: gpt-6-astra (reasoning: xhigh)
  • Implementation: gpt-6-astra (reasoning: xhigh)
  • Review: gpt-6-astra (reasoning: xhigh)

Restore shared camera state after hosted navigation fixtures so unit tests do not leave a system permission alert for subsequent UI launches.

Refs #1110

@talosmachina talosmachina left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No findings. Forces CameraManager.shared.hasPermission = true for the hosted Send fixtures in SendQuickpayLnurlRemountTests and SendSheetLifecycleTests, so Scanner's onAppear no longer calls requestPermissionIfNeeded() and leaves a system camera alert for later UI tests, and restores the original value in a teardown block. Reviewed 1891f12, light tier (12 changed lines, test-only).

What I checked, and 4 candidates I ruled out

Read in full: Bitkit/Managers/CameraManager.swift, the Scanner view in Bitkit/Components/Scanner.swift, both makeHost fixtures.
Not built here: iOS does not build on Linux, so this review is reasoned from the code; the unit test job was still pending when I read CI.

Ruled out

  • A prompt from another path: AVCaptureDevice.requestAccess is only called from requestPermissionIfNeeded(), and the only automatic caller is Scanner.onAppear, which returns early when hasPermission is true. requestPermission() runs only from the permission-request button, which is not rendered once hasPermission is true.
  • The forced value overwritten mid-test: refreshPermission() is called only from the private init, so nothing re-reads the system status after the fixture sets the flag.
  • A real capture session started: with hasPermission true on the simulator Scanner renders Color.black under #if targetEnvironment(simulator), not ScannerCamera.
  • Teardown actor mismatch: both test classes are @MainActor and the teardown block is annotated @MainActor, matching CameraManager's isolation; no other test in BitkitTests reads CameraManager.hasPermission.

Merge confidence: 4/5, no findings, but the unit test job had not finished and the change could not be run here.

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: ✅ Approve

Findings: None.

QA for 1891f12:

Check 1: app and test-target build ✅ I built the app and test targets successfully with Xcode 27.0 for the arm64 iOS Simulator.

build-for-testing compiled both changed fixtures with the unchanged Package.resolved. This was a compilation check; it did not execute tests.

Check 2: hosted fixture isolation ✅ Code inspection found the permission override and restoration correctly scoped to the hosted tests.

Both fixtures capture the shared value before hosting, set it before Scanner.onAppear, and restore it on the main actor after the tests remove their windows. Repeated hosts unwind in reverse teardown order. The simulator scanner uses its non-camera branch. Existing test methods, assertions and production code are unchanged.

Check 3: formatting and patch integrity ✅ SwiftFormat 0.64.0 lint and `git diff --check` passed.

Lint covered both changed Swift files using the repository configuration.

Runtime evidence: I relied on @ben-kaufman's controlled comparison at this exact head. It reports the base leaking a camera alert, the candidate completing both hosted suites and the subsequent launch without a prompt, and a passing canonical suite with its existing prerequisite skips. I did not repeat those unchanged cases or run a manual device journey; the QA notes list manual tests and journeys as N/A.

The separate bitkit-ios#1110 Investigate repeated XCTest app-launch background-assertion timeout remains unexplained; the reported base launch passed despite the alert. This approval covers fixture isolation. It does not establish that timeout's cause or resolution.

Coverage: full e4bf44a..1891f12, both changed files and the affected scanner, permission and teardown callers. No new runtime case was driven in this review. The other current-head review also reports no findings.

Android twin: none declared; this changes only iOS hosted XCTest fixtures.

Reviewer model: gpt-6.1-sol
Reasoning effort: high


Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

@ovitrif ovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utAck

@ovitrif
ovitrif merged commit f7074d5 into master Oct 24, 2026
19 checks passed
@ovitrif

ovitrif commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

Merged, as it was approved and green.

@ovitrif
ovitrif deleted the test/isolate-hosted-camera-permission branch October 24, 2026 07:10
@ovitrif

ovitrif commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

@ben-kaufman Merged, as it was approved and green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants