Conversation
a3887da to
648cca0
Compare
26a1a8c to
743877d
Compare
There was a problem hiding this comment.
Pull request overview
This PR upgrades the functional test suite to Playwright 1.61.1 and adjusts Sync/OAuth functional tests to account for Firefox’s keys_optional/decoupled-Sync routing changes, including more robust navigation + WebChannel mocking for browser-driven Sync handshakes.
Changes:
- Upgrades
@playwright/test(and Playwright deps) and bumps CircleCI runner images to-v10to ship matching browsers. - Refactors Sync-entry navigation across many specs to use
/pairvia a newgotoSyncSession()helper, and updates several tests for new signin routing. - Improves test stability with new/updated helpers (e.g., settled
/oauthwait, repeated WebChannel responder, email-first fill retry, QR decode retry).
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Updates Playwright-related dependency resolutions/checksums. |
| packages/functional-tests/package.json | Bumps @playwright/test to 1.61.1. |
| packages/functional-tests/playwright.config.ts | Makes retries always-on (1) to absorb rare flakes. |
| packages/functional-tests/lib/sync-helpers.ts | Adds gotoSyncSession() helper to centralize Sync session entry via /pair. |
| packages/functional-tests/pages/signin.ts | Makes email-first form fill more resilient via expect().toPass() retry. |
| packages/functional-tests/pages/settings/totp.ts | Retries QR screenshot+decode to avoid blank-canvas race. |
| packages/functional-tests/pages/relier.ts | Adds a stricter wait for the settled /oauth page after redirects. |
| packages/functional-tests/pages/layout.ts | Adds respondToWebChannelMessageAlways() and refactors responder install logic. |
| packages/functional-tests/tests/syncV3/signIn.spec.ts | Uses gotoSyncSession() instead of hardcoded Sync URLs. |
| packages/functional-tests/tests/syncV3/settings.spec.ts | Uses gotoSyncSession() and minor fixture formatting adjustments. |
| packages/functional-tests/tests/syncV3/fxDesktopV3ForceAuth.spec.ts | Updates force-auth flow to handle chained OAuth re-auth/token steps. |
| packages/functional-tests/tests/settings/recoveryPhone.spec.ts | Starts Sync handshake via /pair before recovery-phone assertions. |
| packages/functional-tests/tests/react-conversion/signup.spec.ts | Initiates Sync v3 signup flow via /pair. |
| packages/functional-tests/tests/react-conversion/oauthSignup.spec.ts | Initiates Sync OAuth flows via /pair and updates send-tab flow assertions. |
| packages/functional-tests/tests/passkeyAuth/passkeySetPassword.spec.ts | Uses /pair-initiated Sync handshake for passkey set-password flows. |
| packages/functional-tests/tests/passkeyAuth/passkeyPasswordFallback.spec.ts | Refactors setup + fallback helpers and uses capability-based FxAStatus mock. |
| packages/functional-tests/tests/pairing/pairChoice.spec.ts | Adds explicit sign-in steps since /pair redirects for fresh browsers. |
| packages/functional-tests/tests/oauth/syncSignIn.spec.ts | Uses gotoSyncSession() and updates expectations for cached-signin behavior. |
| packages/functional-tests/tests/misc/vpnIntegration.spec.ts | Switches to capability-based “Sync not decoupled” mock and always-respond WebChannel handler. |
| packages/functional-tests/tests/misc/recoveryKeyPromoInline.spec.ts | Uses gotoSyncSession() and adjusts expectations when session persists. |
| packages/functional-tests/tests/key-stretching-v2/signInTokenCode.spec.ts | Uses gotoSyncSession() while preserving stretch param behavior. |
| packages/functional-tests/tests/cms/cms.spec.ts | Re-enters Sync CMS entrypoint after /pair handshake; updates flow steps. |
| packages/functional-tests/tests/cms/cms-2fa.spec.ts | Enrolls TOTP out-of-band and reworks Sync CMS 2FA test to fit new routing. |
| .circleci/README.md | Documents image rebuild/tag bump process for Playwright upgrades. |
| .circleci/config.yml | Bumps CI images -v9 → -v10 across executors/build/push steps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await expect(connectAnotherDevice.fxaConnected).toBeVisible(); | ||
| await connectAnotherDevice.clickNotNowPair(); |
| import { BaseTarget } from './targets/base'; | ||
|
|
||
| /** | ||
| * Initiate a desktop Sync sign-in/up session in the test browser. |
There was a problem hiding this comment.
I dont really like this, but it is the "cleanest" way to get the Firefox desktop browser to initate a login session. We can't fake the OAuth params because Firefox checks these and will not log a user in completely because of that. I've pull this into a helper so that we can change it later.
TLDR; goto /pair to Sign into Sync. The FxA sends webchannel messages and browser returns a valid login url that we direct to.
| await expect(this.step1QRCode).toBeVisible(); | ||
|
|
||
| // Retry the screenshot+decode: in the change-2FA flow the canvas can repaint | ||
| // with the new secret a beat after the heading appears, so a single early |
There was a problem hiding this comment.
Unrelated, but ran into this error a few times locally, figured I would just fix it.
| syncDesktopOAuthQueryParams.set('entrypoint', ENTRYPOINT_SYNC); | ||
| await signin.goto('/authorization', syncDesktopOAuthQueryParams); | ||
|
|
||
| await assertCmsCustomization(page, { |
There was a problem hiding this comment.
This was already asserted below, removed duplicate.
| const { email, password } = await testAccountTracker.signUpPasswordless(); | ||
| // Android (Fenix) has no keys_optional (Sync not decoupled), so signin needs | ||
| // a password and routes to /set_password; fxa_status repeats, so answer all. | ||
| const syncNotDecoupledStatus: FxAStatusResponse = { |
There was a problem hiding this comment.
@LZoog Mind a santify check on this test case?
83ea11b to
0d6ccd5
Compare
…th/CMS tests Because: * Playwright 1.44 -> 1.61.1 (Firefox 151) and decoupled Sync (keys_optional) broke several Sync/OAuth/CMS functional tests. * Hardcoded fake OAuth params (keys_jwk) no longer complete key derivation on FF151, causing password re-prompt loops in Sync flows. This commit: * Upgrades Playwright to 1.61.1 and bumps the CI images. * Routes Sync sign-in/up tests through the real /pair handshake (gotoSyncSession) instead of hardcoded fake OAuth params. * Restructures the CMS 2FA TOTP-Sync test to enroll TOTP out-of-band so a single sign-in exercises the branded signin_totp_code flow (no unusable second /pair). * Reworks the SmartWindow-after-Sync test into a two-phase flow that asserts the real fxa_oauth_login WebChannel scope instead of only reaching a signed-in view. * Converts the Sync passkey password-fallback test to the real Firefox Sync browser (syncOAuthBrowserPages + /pair), dropping the unnecessary no-keys_optional mock since the passkey simply cannot derive Sync keys. * Tightens passkey assertions: wrong-password now asserts the Incorrect password alert (not any 'password' text), asserts the passkey button on prompt=login, and corrects a misleading 'passkey-only account' test name. * Bounds email-first waits (waitForURL(action=email), fillOutEmailFirstForm toPass) so broken flows fail fast instead of hitting the 60s test timeout. * Retries once locally and on CI to absorb rare parallel-worker flakes.
|
@vbudhram just want to note that in #20817 I had to temporarily add a couple of lines to fill out the "repeat password" form since we can't test the |
Because
keys_optional) changed signin routing, breaking several Sync and OAuth functional tests.This pull request
@playwright/test1.44.1 → 1.61.1 (packages/functional-tests/package.json,yarn.lock).fxa-circleciCI images v9 → v10 (.circleci/config.yml) so the functional-test-runner ships the matching browsers.keys_optionalmocks to be capability-based (Sync not decoupled) rather than browser-version-based invpnIntegration.spec.ts,passkeyPasswordFallback.spec.ts, and thesyncV3/*specs.waitForOauthPage()inrelier.tsso email-first/AAL2 nav waits for the settled/oauthpage, not the intermediate/authorizationredirect.respondToWebChannelMessageAlways()inlayout.tsfor flows that re-dispatchfxa_status.gotoSyncSession()inlib/sync-helpers.tsto abstract/pairnavigation across Sync tests.Issue that this pull request solves
Closes: https://mozilla-hub.atlassian.net/browse/FXA-13647
Checklist
Other information
Requires the
-v10CI image (ci-functional-test-runner-v10) to be published before functional tests pass — built via theupdate-ci-imagebranch /force-deploy-fxa-ci-images. Verify by re-running the "Firefox Functional Tests - Playwright (PR)" job once-v10is in the registry.