feat(W-23721537): speed up AuthFlowTester UI tests via launch-arg driven SDK reset - #4121
Merged
wmathurin merged 19 commits intoAug 6, 2026
Conversation
…ven SDK reset Add SalesforceSDKManager.resetForUITesting() (#if DEBUG) which logs out all users (including async server refresh-token revocation), resets the selected login host to login.salesforce.com, removes persisted custom login servers, and restores all auth flags to their post-init defaults. AppDelegate calls it when --resetSDKForUITesting is in launch arguments. BaseAuthFlowTester.launch() passes the flag so every test gets a clean slate in-process at startup. tearDown is now a no-op (no UI logout, no DPoP toggle navigation). restart() creates a fresh XCUIApplication without the reset arg so session-persistence tests (LoginWithRestartTests, RefreshTokenMigration- WithRestartTests) continue to work correctly.
Clang Static Analysis Issues
Generated by 🚫 Danger |
|
||||||||||||||
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your patch status has failed because the patch coverage (34.78%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## dev #4121 +/- ##
==========================================
+ Coverage 71.05% 71.41% +0.35%
==========================================
Files 254 254
Lines 22480 22497 +17
==========================================
+ Hits 15973 16066 +93
+ Misses 6507 6431 -76
🚀 New features to boost your workflow:
|
…browser login() computed advancedAuthEnabled = forceAdvancedAuthentication != false, which evaluated nil as true (browser). Tests that omit the flag therefore logged in via the browser, registering the BW marker — but restartAndValidateUser, switchToUserAndValidate, and launchLoginAndValidate all used the same != false logic for expectAdvancedAuth. On restart, the BW flag is not rehydrated from disk (per-user feature-flag persistence is a separate unmerged story), so the assertion fires. Fix: change all four sites from != false to == true so nil and false both mean "use the in-app WebView". Also pass forceAdvancedAuthentication ?? false to configureLoginOptions to explicitly clear any residual nAuthentication=true from a prior test. Update two ForceAdvancedAuthTests cases that relied on nil=ON to pass forceAdvancedAuthentication: true explicitly. Port of fix from browser-login-telemetry-markers branch (commit 92a5823).
… restartAndValidateUser switchToUserAndValidate calls in testBothDynamic, testFirstStatic/Dynamic, and testMigrateOneUserOnly were missing isMultiUser: true — the MU flag is still set while both users are logged in, so the assertion fires even though the switch itself doesn't change the account count. restartAndValidateUser gains an isRtr parameter (default false). When true, it runs assertRevokeAndRefreshWorks with the correct expectAdvancedAuth derived from its own params, replacing the standalone assertRevokeAndRefreshWorks call that callers (RTRLoginTests) were making separately with defaulted params. Port of fix from browser-login-telemetry-markers branch (commit 87abbf7).
…rateAndValidate RT flag (testECAJwtRtr_NoHybrid_WithRestart): validateUser had no isRtr param, so it always called validateUserAgent with isRtr=false. After launchLoginAndValidate runs an RTR cycle the RT flag is set and persists through the non-resetting restart — restartAndValidateUser's validateUser call then asserts RT must be absent while it's present. Fix: add isRtr to validateUser and forward it to validateUserAgent; thread it from restartAndValidateUser. MU flag (testMigrateOneUserOnly): migrateAndValidate had no isMultiUser param. In testMigrateOneUserOnly User B is still logged in when migrateAndValidate runs on User A, so MU is set in the UA but validate() asserted it must not be there. Fix: add isMultiUser to migrateAndValidate and forward it to validate(); pass isMultiUser: true at the call site in testMigrateOneUserOnly.
SFSDKPerUserFeatureMarkersMap is a static singleton that survives logoutAllUsers. Add #if DEBUG resetPerUserFeaturesForUITesting and call it from resetForUITesting so RT/DP flags from one test's RTR cycle don't bleed into the next test's initial UA assertion. Fixes testECAOpaqueRtr_Hybrid and any other non-RTR test that ran after an RTR test with the same user key.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
1. assertRevokeAndRefreshWorks default expectAdvancedAuth: true → false The old default assumed nil forceAdvancedAuthentication meant browser (BW) login. After the forceAdvancedAuthentication != false → == true fix, nil means in-app WebView (no BW). DPoP and RTR callers that omit expectAdvancedAuth were getting spurious BW assertions. 2. setForceAdvancedAuthentication staticAppConfigName now optional testForceAdvancedAuth_Disabled_StandardServer_UsesInAppWebView was importing ecaOpaque (a My-Domain ECA) as the consumer key while testing against login.salesforce.com, producing invalid_client_id and no login form. Default bootconfig.plist key is valid on the standard server; no config override needed for this test.
wmathurin
requested review from
brandonpage and
sfdctaka
and removed request for
brandonpage
August 5, 2026 19:38
restartAndValidateUser's job is to restart and validate session persistence. The revoke/refresh RTR check is a separate concern; callers that need it pass assertRevokeAndRefreshWorks(isRtr: true) explicitly, consistent with DPoP tests and the skipped testECAJwtRtr_Hybrid_WithRestart.
…= true) Matches Android's AuthFlowTest where forceAdvancedAuthentication is a non-nullable Boolean defaulting to true (the SDK default). Removes the nil path that was silently overriding sdk_forceAdvancedAuthentication to false via configureLoginOptions (?? false). Now nil is impossible: callers either rely on the true default (browser/BW) or pass false explicitly to exercise the WebView path. Also restores assertRevokeAndRefreshWorks default expectAdvancedAuth back to true, consistent with the forceAdvancedAuthentication default.
…cope tests
Three new tests mirror testCAOpaque_{Default,Subset,All}Scopes_WebServerFlow
but with forceAdvancedAuthentication: false to exercise the web server
OAuth flow through the in-app WebView rather than the external browser.
Contributor
|
This file is not included in the PR. |
sfdctaka
reviewed
Aug 5, 2026
Both -init and resetForUITesting set the same auth flags. Extract a private resetAuthFlags instance method so the values are defined once and can't diverge. simulatedDomainDiscoveryResult is reset only in resetForUITesting (it's a test-only property absent from -init).
Contributor
Author
Good catch. The assert was there before this PR. Fixed PR description. |
…dValidate Migration is a silent token exchange — the browser is never opened, so BW is absent from the post-migration UA regardless of how the initial login was done. Defaulting to false avoids spurious BW assertions in all migration tests. Remove now-redundant explicit false from two DPoPLoginTests callers.
SFOAuthTypeRefreshTokenMigration was hitting the generic else branch in finalizeAuthCompletion that unregisters BW. Migration exchanges the consumer key/token but does not change how the user originally authenticated, so the existing per-user BW flag should be preserved. Add an explicit guard for SFOAuthTypeRefreshTokenMigration that is a no-op, leaving the flag intact. Normal token refreshes (SFOAuthTypeRefresh) and other non-browser auth types continue to clear BW as designed. Update migrateAndValidate default to forceAdvancedAuthentication: true (BW is now carried through) and add explicit false for the two user-agent-flow migration tests where BW was never registered.
brandonpage
reviewed
Aug 5, 2026
Comment on lines
+2185
to
+2187
| } else if (completedAuthType == SFOAuthTypeRefreshTokenMigration) { | ||
| // Migration exchanges the token but does not change how the user originally | ||
| // authenticated. Preserve the existing per-user BW flag rather than clearing it. |
brandonpage
approved these changes
Aug 5, 2026
…heck The RT flag persists on disk after launchLoginAndValidate completes an RTR cycle. restartAndValidateUser must tell validateUser to expect RT in the post-restart UA; otherwise validateUserAgent asserts RT absent and fails. isRtr is forwarded to validateUser only — the separate assertRevokeAndRefreshWorks(isRtr: true) call is unchanged.
…, for WebView modality testForceAdvancedAuth_Disabled_StandardServer_UsesInAppWebView was asserting isShowingInAppLoginForm() (waits 30s for a text field inside the WKWebView) after disabling advanced auth and restarting auth against login.salesforce.com. The consumer key in bootconfig.plist is an org-specific test CA that triggers invalid_client_id on the standard server, so the login page never renders its username text field — the assertion timed out. The test's intent is modality detection: confirm the SDK chose the in-app WebView (SFLoginViewController) over the external browser (ASWebAuthenticationSession). For that it is sufficient to observe the "Log In" navigation bar, which SFLoginViewController presents immediately — before the WKWebView has finished loading any page. Add isShowingLoginViewController() (checks the "Log In" nav bar) to LoginPageObject and expose it in BaseAuthFlowTester. Switch the test to use it instead of isShowingInAppLoginForm(), which remains available for callers that load a real page.
…ncedAuthUser_HasBWFlag_RegularAuthUser_DoesNot launchLoginAndValidate defaults to forceAdvancedAuthentication: true, so User A was logging in via the browser (BW registered). The test then asserted BW absent for User A while two users are logged in, causing XCTAssertFalse to fail. Pass forceAdvancedAuthentication: false for User A so the SDK uses the in-app WebView — the .regularAuth host does not opt into native browser auth via its auth config, so disabling the process-global flag is sufficient to use the WebView.
Contributor
Author
|
A lot of tests fixed. Will further validate in the other PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SalesforceSDKManager.resetForUITesting()(#if DEBUG) — logs out all users (including async server refresh-token revocation), clears per-user in-memory feature flags, resets the selected login host tologin.salesforce.com, removes persisted custom login servers, and restores all auth flags to theirinitdefaults.AppDelegatecalls it when--resetSDKForUITestingis in launch arguments.BaseAuthFlowTester.launch()passes--resetSDKForUITestingso every test gets a clean slate in-process at startup, replacing 10–20 s of UI-driven tearDown/launch cleanup per test.tearDownis now a no-op (no UI logout, no DPoP toggle navigation).restart()creates a freshXCUIApplicationwithout the reset arg so session-persistence tests continue to work.SDK fix
SFUserAccountManager.m— preserve BW flag through refresh token migrationfinalizeAuthCompletion:had an unconditionalelsethat unregistered the BW (kSFAppFeatureSafariBrowserForLogin) per-user flag for any auth type other thanSFOAuthTypeAdvancedBrowser. Refresh token migration completes withSFOAuthTypeRefreshTokenMigration— a silent token exchange that does not change how the user originally authenticated — so BW was being cleared even when the user had logged in via the external browser. Added an explicit no-op guard forSFOAuthTypeRefreshTokenMigrationthat leaves the existing per-user flag intact. Normal token refreshes and other non-browser auth types continue to clear BW as designed.Test fixes (found while running the suite after the main change)
Several pre-existing assertion bugs surfaced because the launch-arg reset makes every test start from a known clean state — previously these were masked by state leaking between tests.
tokenFormatin credentials JSON was"Opaque"instead of""UserCredentialsView.tokenFormatapplied a display substitution (""→"Opaque") that was flowing into the exported JSON. Split intotokenFormatRaw(export) andtokenFormat(display).restartAndValidateUserwiped the session on restartappstill carried--resetSDKForUITestinginlaunchArguments; fixed by creating a freshXCUIApplicationinrestart().hasHost()used a 10 s timeout to probe for a custom host thatresetForUITestingalways removes. Reduced to 2 s (host list is synchronously in-memory).forceAdvancedAuthentication: nilwas overriding SDK default to offconfigureLoginOptionswas called withforceAdvancedAuthentication ?? false, actively writingfalseinto the Login Options JSON and switching the app to WebView mode even when the caller meant "use the SDK default (browser on)". Fixed by makingforceAdvancedAuthenticationa non-nullableBool = trueacross all helpers (matching Android'sAuthFlowTest), removing the optional path entirely.switchToUserAndValidatecalls intestFirstStatic/Dynamic_DifferentApps,testBothDynamic_DifferentApps, andtestMigrateOneUserOnly(viamigrateAndValidate) were missingisMultiUser: true. AddedisMultiUserparam tomigrateAndValidateand fixed all call sites.validateUserhad noisRtrparam, always callingvalidateUserAgentwithisRtr: false. AfterlaunchLoginAndValidateruns an RTR cycle the RT flag persists through the non-resetting restart. AddedisRtrtovalidateUserandvalidateUserAgent.testECAOpaqueRtr_Hybrid)SFSDKPerUserFeatureMarkersMapwas never cleared bylogoutAllUsers. RT flag from a previous test's RTR cycle survived into the next test's initial UA check. Added#if DEBUG resetPerUserFeaturesForUITestingtoSFSDKAppFeatureMarkersand called it fromresetForUITesting.invalid_client_iderror on standard server in ForceAdvancedAuth disabled testsetForceAdvancedAuthenticationwas importing a My-Domain ECA consumer key (ecaOpaque) when testing againstlogin.salesforce.com, triggeringinvalid_client_id. MadestaticAppConfigNameoptional (defaults tonil); the app's defaultbootconfig.plistkey is valid on the standard server.Refactors
forceAdvancedAuthentication: Bool? = nil→Bool = truenilwas ambiguous and masked the?? falsebug. Non-nullable default makes the intent explicit: browser on unless the test explicitly opts out withfalse.restartAndValidateUserno longer hasisRtrparamassertRevokeAndRefreshWorks(isRtr: true)explicitly, consistent with DPoP tests and the skippedtestECAJwtRtr_Hybrid_WithRestart.resetAuthFlagshelper inSalesforceSDKManager-initandresetForUITestingset the same auth flags. Extracted into a private instance method so the values are defined once and can't diverge.New tests
LegacyLoginTests: addedtestCAOpaque_{Default,Subset,All}Scopes_WebServerFlow_InAppWebView— mirrors the existing browser-path web server flow tests withforceAdvancedAuthentication: falseto cover the in-app WebView path under the same scope variations.Files changed
SalesforceSDKCore/Classes/Common/SalesforceSDKManager.h+ (void)resetForUITestingunder#if DEBUGSalesforceSDKCore/Classes/Common/SalesforceSDKManager.mresetForUITesting; extractresetAuthFlags; callresetPerUserFeaturesForUITesting; importSFSDKLoginHostStorage.hSalesforceSDKCore/Classes/Common/SFSDKAppFeatureMarkers.h+ (void)resetPerUserFeaturesForUITestingunder#if DEBUGSalesforceSDKCore/Classes/Common/SFSDKAppFeatureMarkers.mresetPerUserFeaturesForUITesting— clearsSFSDKPerUserFeatureMarkersMapSalesforceSDKCore/Classes/UserAccount/SFUserAccountManager.mSFOAuthTypeRefreshTokenMigrationinfinalizeAuthCompletion:AuthFlowTester/Classes/AppDelegate.swiftresetForUITesting()when launch arg presentAuthFlowTester/Views/UserCredentialsView.swifttokenFormatinto raw (export) and display variantsAuthFlowTesterUITests/Util/BaseAuthFlowTester.swifttearDown; fixrestart(); makeforceAdvancedAuthenticationnon-nullable (Bool = true); removeisRtrfromrestartAndValidateUser; addisRtr/isMultiUserparams where needed; makesetForceAdvancedAuthenticationstaticAppConfigNameoptional; restoremigrateAndValidatedefault totrueAuthFlowTesterUITests/PageObjects/LoginPageObject.swifthasHosttimeout from 10 s to 2 sAuthFlowTesterUITests/Tests/ForceAdvancedAuthTests.swiftstaticAppConfigNamefrom disabled-auth testAuthFlowTesterUITests/Tests/MultiUserLoginTests.swiftisMultiUser: truetoswitchToUserAndValidatewhile two users are logged inAuthFlowTesterUITests/Tests/RefreshTokenMigrationTests.swiftisMultiUser: truetomigrateAndValidate/switchToUserAndValidate; passforceAdvancedAuthentication: falsefor user-agent-flow migration testsAuthFlowTesterUITests/Tests/LegacyLoginTests.swift_InAppWebViewvariants withforceAdvancedAuthentication: false