Conversation
There was a problem hiding this comment.
Pull request overview
Adds a macOS “test” host app plus XCUITest UI automation to exercise the RxAuthSwift sign-in flow end-to-end, and wires it into CI.
Changes:
- Introduces a
test/Xcode project containing a SwiftUI app (test) and a UI test target (testUITests) with helpers for app launch, dotenv loading, and sign-in automation. - Updates RxAuthSwift/RxAuthSwiftUI to better support E2E automation (new in-memory token storage, UI accessibility identifier change, macOS web auth window URL display).
- Adds a new CI job intended to execute the Xcode test plan on macOS.
Reviewed changes
Copilot reviewed 19 out of 21 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/testUITests/testUITests.swift | Adds a basic UI test that launches the app and performs sign-in. |
| test/testUITests/helper/signIn.swift | Implements the UI automation flow for OAuth sign-in using dotenv/env credentials. |
| test/testUITests/helper/launch.swift | Launch helper that passes --reset-auth for deterministic E2E runs. |
| test/testUITests/helper/dotenv.swift | Adds a lightweight .env loader with CI environment fallback. |
| test/test/testApp.swift | Adds a macOS SwiftUI host app configured to reset auth and optionally use in-memory token storage. |
| test/test/ContentView.swift | Adds a minimal UI that renders RxAuthSwiftUI sign-in and an authenticated “home” state. |
| test/test/Assets.xcassets/** | Adds default app assets required by Xcode app target. |
| test/test.xcodeproj/** | Adds Xcode project, schemes, SwiftPM resolved file, and workspace metadata for the test app and UI tests. |
| test/TestPlan.xctestplan | Adds an Xcode test plan targeting the UI tests. |
| Sources/RxAuthSwiftUI/RxSignInView.swift | Minor import ordering change. |
| Sources/RxAuthSwiftUI/Components/PrimaryAuthButton.swift | Changes the sign-in button accessibility identifier for UI testing. |
| Sources/RxAuthSwift/Platform/MacOSWebAuthSession.swift | Adds a URL display field above the WKWebView and updates it during navigation. |
| Sources/RxAuthSwift/InMemoryTokenStorage.swift | Adds an in-memory token storage for test/ephemeral sessions. |
| .gitignore | Ignores .env, .netrc, and common Xcode/SwiftPM generated files. |
| .github/workflows/ci.yml | Adds a self-hosted macOS job to run the TestPlan via xcodebuild. |
Files not reviewed (1)
- test/test.xcodeproj/project.xcworkspace/contents.xcworkspacedata: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Use a longer timeout and provide better error message | ||
| let emailFieldExists = emailField.waitForExistence(timeout: 30) | ||
| NSLog("✅ Email field found, entering credentials...") | ||
| logger.info("✅ Email field found, entering credentials...") | ||
|
|
||
| // Fill in credentials from environment | ||
| // WebView elements need extra handling for keyboard focus in CI | ||
| emailField.tap() | ||
| sleep(1) // Give WebView time to establish keyboard focus | ||
| // Type the email | ||
| emailField.typeText(testEmail) | ||
| NSLog("✅ Email entered") |
There was a problem hiding this comment.
In the iOS flow, emailFieldExists is computed but never asserted/used, and the code proceeds to tap/type into the email field regardless. If the OAuth page doesn’t load, this will fail with a less-informative element interaction error; assert emailFieldExists (and similarly for passwordField) before interacting.
| let emailFieldExists = emailField.waitForExistence(timeout: 30) | ||
| XCTAssertTrue(emailFieldExists, "Failed to sign in and reach dashboard") | ||
|
|
||
| let passwordField = self/*@START_MENU_TOKEN@*/ .secureTextFields["Enter your password"].firstMatch/*[[".groups",".secureTextFields[\"Password\"].firstMatch",".secureTextFields[\"Enter your password\"].firstMatch",".secureTextFields",".containing(.group, identifier: nil).firstMatch",".firstMatch"],[[[-1,2],[-1,1],[-1,3,2],[-1,0,1]],[[-1,2],[-1,1]],[[-1,5],[-1,4]]],[0]]@END_MENU_TOKEN@*/ |
There was a problem hiding this comment.
The macOS passwordField locator still contains an Xcode recorder @START_MENU_TOKEN@...@END_MENU_TOKEN@ artifact and unusual spacing (self/*...*/ .secureTextFields...). This is hard to maintain and can break easily; replace it with a stable query (preferably using accessibility identifiers) and remove the recorder token markup.
| let passwordField = self/*@START_MENU_TOKEN@*/ .secureTextFields["Enter your password"].firstMatch/*[[".groups",".secureTextFields[\"Password\"].firstMatch",".secureTextFields[\"Enter your password\"].firstMatch",".secureTextFields",".containing(.group, identifier: nil).firstMatch",".firstMatch"],[[[-1,2],[-1,1],[-1,3,2],[-1,0,1]],[[-1,2],[-1,1]],[[-1,5],[-1,4]]],[0]]@END_MENU_TOKEN@*/ | |
| let passwordField = secureTextFields["Enter your password"].firstMatch |
| .foregroundStyle(.green) | ||
|
|
||
| Text("Welcome!") | ||
| .accessibilityLabel("home-view-title") |
There was a problem hiding this comment.
The UI test waits for staticTexts["home-view-title"], but the view sets this as an accessibilityLabel, not an accessibilityIdentifier. This can cause the test to fail depending on how XCTest resolves queries. Set accessibilityIdentifier("home-view-title") on the element used for the dashboard assertion (and keep the user-facing label as "Welcome!").
| .accessibilityLabel("home-view-title") | |
| .accessibilityIdentifier("home-view-title") |
| .accessibilityIdentifier("sign-in-button") | ||
| .accessibilityHint(isLoading ? "Authentication in progress" : "Double tap to sign in") |
There was a problem hiding this comment.
This changes the accessibility identifier from primaryAuthButton to sign-in-button on a public UI component. That’s effectively a breaking change for any downstream UI automation relying on the old identifier. Consider keeping the existing identifier (or making it configurable) and updating the UI tests to use it, or document this as a breaking change in release notes.
| // Give Safari time to launch | ||
| sleep(2) | ||
|
|
||
| // Wait for Safari OAuth page to appear | ||
| #if os(iOS) | ||
| let safariViewServiceApp = XCUIApplication(bundleIdentifier: "com.apple.SafariViewService") | ||
| NSLog("⏱️ Waiting for Safari OAuth page to load...") | ||
| logger.info("⏱️ Waiting for Safari OAuth page to load...") | ||
|
|
||
| // Wait for email field to appear (OAuth page loaded) | ||
| let emailField = safariViewServiceApp.textFields["you@example.com"].firstMatch | ||
| let passwordField = safariViewServiceApp.secureTextFields["Enter your password"].firstMatch | ||
|
|
||
| // Use a longer timeout and provide better error message | ||
| let emailFieldExists = emailField.waitForExistence(timeout: 30) | ||
| NSLog("✅ Email field found, entering credentials...") | ||
| logger.info("✅ Email field found, entering credentials...") | ||
|
|
||
| // Fill in credentials from environment | ||
| // WebView elements need extra handling for keyboard focus in CI | ||
| emailField.tap() | ||
| sleep(1) // Give WebView time to establish keyboard focus | ||
| // Type the email | ||
| emailField.typeText(testEmail) | ||
| NSLog("✅ Email entered") | ||
| logger.info("✅ Email entered") | ||
|
|
||
| // Small delay before pressing Enter | ||
| sleep(1) | ||
| emailField.typeText("\n") // Press Enter to move to next field | ||
| #elseif os(macOS) | ||
|
|
||
| let emailField = textFields["you@example.com"].firstMatch | ||
| let emailFieldExists = emailField.waitForExistence(timeout: 30) | ||
| XCTAssertTrue(emailFieldExists, "Failed to sign in and reach dashboard") | ||
|
|
||
| let passwordField = self/*@START_MENU_TOKEN@*/ .secureTextFields["Enter your password"].firstMatch/*[[".groups",".secureTextFields[\"Password\"].firstMatch",".secureTextFields[\"Enter your password\"].firstMatch",".secureTextFields",".containing(.group, identifier: nil).firstMatch",".firstMatch"],[[[-1,2],[-1,1],[-1,3,2],[-1,0,1]],[[-1,2],[-1,1]],[[-1,5],[-1,4]]],[0]]@END_MENU_TOKEN@*/ | ||
|
|
||
| emailField.click() | ||
| emailField.typeText(testEmail) | ||
| #endif | ||
| // WebView password field also needs focus handling | ||
| passwordField.tap() | ||
| sleep(1) // Give WebView time to establish keyboard focus | ||
|
|
||
| passwordField.typeText(testPassword) | ||
| NSLog("✅ Password entered, submitting...") | ||
| logger.info("✅ Password entered, submitting...") | ||
| sleep(1) | ||
| passwordField.typeText("\n") // Press Enter to submit |
There was a problem hiding this comment.
There are multiple fixed sleep(...) calls used for synchronization. These tend to make UI tests slower and flaky across machines/CI. Prefer waiting on specific UI conditions (e.g., waitForExistence, XCTWaiter/expectations, or polling for hittable state) instead of hard sleeps.
|
|
||
| import XCTest | ||
|
|
||
| final class testUITests: XCTestCase { |
There was a problem hiding this comment.
XCTestCase subclass names should be UpperCamelCase in Swift. final class testUITests deviates from Swift naming conventions and makes test discovery/debug output harder to read. Rename to TestUITests (and keep the file name aligned).
| final class testUITests: XCTestCase { | |
| final class TestUITests: XCTestCase { |
| import SwiftUI | ||
|
|
||
| @main | ||
| struct testApp: App { |
There was a problem hiding this comment.
Swift type names should be UpperCamelCase. struct testApp: App deviates from standard conventions and will read oddly in crash logs / debugging. Rename to TestApp (and consider matching module/target naming if this is a sample app).
| struct testApp: App { | |
| struct TestApp: App { |
353c885 to
b8d3217
Compare
Autopilot PR Check IssuesThe following potential issues were detected in this PR:
|
No description provided.