test(functional-tests): decouple smoke locators from CMS-overridable copy - #20973
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes the Playwright functional-test suite resilient to relying-party CMS overrides by removing locators that match default English copy and replacing them with structural, route-, or telemetry-based hooks. It also shifts copy assertions into fxa-settings unit tests and documents the locator convention for future changes.
Changes:
- Add reusable structural helpers for functional-test POMs (
formSubmitButton,pageHeading) and migrate affected page objects/specs off CMS-overridable copy. - Introduce a stable
data-testid="cached-signin-submit"hook for the cached sign-in step (shared/signinroute) and update functional tests to use it. - Expand
fxa-settingscomponent tests to assert CMS-provided headlines and primary CTA text; constrainMOCK_CMS_INFOwithsatisfies RelierCmsInfo; document the convention in.clauderules and POM README.
Reviewed changes
Copilot reviewed 54 out of 54 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/fxa-settings/src/pages/Signup/SignupConfirmedSync/index.test.tsx | Adds unit test coverage for CMS headline + primary CTA text on confirmed-sync page. |
| packages/fxa-settings/src/pages/Signup/index.test.tsx | Asserts CMS primaryButtonText renders on signup set-password page. |
| packages/fxa-settings/src/pages/Signin/SigninUnblock/index.test.tsx | Adds unit test asserting CMS headline + primary CTA text on unblock page. |
| packages/fxa-settings/src/pages/Signin/SigninTotpCode/index.test.tsx | Adds CMS primary CTA assertion; removes unnecessary optional chaining from mock usage. |
| packages/fxa-settings/src/pages/Signin/SigninTokenCode/index.test.tsx | Adds unit test asserting CMS headline rendering (button text noted as out of scope). |
| packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/index.test.tsx | Adds CMS primary CTA assertion; formatting-only test wrapper changes. |
| packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/index.test.tsx | Adds CMS primary CTA assertion for recovery-code page. |
| packages/fxa-settings/src/pages/Signin/SigninRecoveryChoice/index.test.tsx | Adds CMS primary CTA assertion for recovery-choice page. |
| packages/fxa-settings/src/pages/Signin/SigninPasswordlessCode/index.test.tsx | Adds local CMS fixture to assert CMS headline + primary CTA for passwordless code page. |
| packages/fxa-settings/src/pages/Signin/components/SigninCached/index.tsx | Adds data-testid="cached-signin-submit" to cached-step submit button (prod behavior unchanged). |
| packages/fxa-settings/src/pages/Signin/components/SigninCached/index.test.tsx | Asserts testid contract and adds CMS headline + primary CTA coverage for cached step. |
| packages/fxa-settings/src/pages/mocks.tsx | Exports MOCK_CMS_INFO with satisfies RelierCmsInfo to keep literal types and tighten shape. |
| packages/functional-tests/tests/syncV3/signinCached.spec.ts | Replaces heading-based waits with field/testid-based waits. |
| packages/functional-tests/tests/syncV3/oauthSignIn.spec.ts | Replaces cached/email-first heading asserts with structural locators. |
| packages/functional-tests/tests/syncV3/fxDesktopHandshakeNonSync.spec.ts | Replaces cached/password heading asserts with structural locators. |
| packages/functional-tests/tests/syncV3/fxDesktopHandshake.spec.ts | Replaces sync/email/password heading asserts with field-based waits. |
| packages/functional-tests/tests/signin/signinCached.spec.ts | Migrates cached-step gating from heading to submit testid. |
| packages/functional-tests/tests/signin/signIn.spec.ts | Migrates email-first heading gating to email field gating. |
| packages/functional-tests/tests/signin/redirect.spec.ts | Uses email field hidden/visible checks instead of heading. |
| packages/functional-tests/tests/settings/redirect.spec.ts | Fixes missing await on navigation; uses email field gate on redirect. |
| packages/functional-tests/tests/settings/changeEmail.spec.ts | Uses email field gating after sign-out instead of heading. |
| packages/functional-tests/tests/react-conversion/syncV3SignIn.spec.ts | Migrates sync-signin heading gate to email field gate. |
| packages/functional-tests/tests/react-conversion/signup.spec.ts | Migrates signup heading gate to password field gate. |
| packages/functional-tests/tests/react-conversion/signInCached.spec.ts | Migrates cached heading gate to submit testid. |
| packages/functional-tests/tests/react-conversion/signIn.spec.ts | Migrates email-first heading gate to email field gate. |
| packages/functional-tests/tests/react-conversion/oauthSignup.spec.ts | Migrates email/signup heading gates to email/password field gates. |
| packages/functional-tests/tests/react-conversion/oauthSignin.spec.ts | Migrates cached/signup heading gates to submit testid / password field gates. |
| packages/functional-tests/tests/react-conversion/oauthPromptNone.spec.ts | Migrates signup/password heading gates to password textbox gating. |
| packages/functional-tests/tests/react-conversion/forceAuth.spec.ts | Adds URL guard for signup vs signin ambiguity; switches gating to structural locators. |
| packages/functional-tests/tests/passwordless/signinPasswordless.spec.ts | Migrates cached/password heading gates to submit testid / password textbox. |
| packages/functional-tests/tests/passkeyAuth/passkey-signin.spec.ts | Migrates cached/password heading gates to submit testid / password textbox. |
| packages/functional-tests/tests/oauth/syncSignIn.spec.ts | Migrates cached/password heading gates to submit testid / password textbox. |
| packages/functional-tests/tests/oauth/signin.spec.ts | Migrates cached heading gates to submit testid. |
| packages/functional-tests/tests/oauth/serverSideScopeResolution.spec.ts | Migrates cached heading gate to submit testid. |
| packages/functional-tests/tests/oauth/loginHintCachedCredentials.spec.ts | Migrates password heading gate to password textbox gate. |
| packages/functional-tests/tests/oauth/loginHint.spec.ts | Migrates email/signup/password heading gates to field-based gating. |
| packages/functional-tests/tests/misc/vpnIntegration.spec.ts | Migrates cached heading gate to submit testid. |
| packages/functional-tests/tests/misc/relayIntegration.spec.ts | Makes Relay copy assertion resilient to Fluent bidi isolates via partial regex match. |
| packages/functional-tests/tests/misc/errorViews.spec.ts | Migrates email-first heading gates to email textbox gates. |
| packages/functional-tests/tests/key-stretching-v2/signInTokenCode.spec.ts | Migrates email-first heading gate to email textbox gate. |
| packages/functional-tests/pages/signupConfirmedSync.ts | Locates pair CTA via data-glean-id instead of copy. |
| packages/functional-tests/pages/signup.ts | Routes submit actions through structural submit helper; replaces heading-gated waits with field-gated waits. |
| packages/functional-tests/pages/signinUnblock.ts | Uses pageHeading helper instead of headline copy for h1-only pages. |
| packages/functional-tests/pages/signinTokenCode.ts | Uses pageHeading helper instead of headline copy for h1-only pages. |
| packages/functional-tests/pages/signinRecoveryPhone.ts | Uses structural submit button instead of CMS-overridable “Confirm” label. |
| packages/functional-tests/pages/signinRecoveryChoice.ts | Uses formSubmitButton helper for Continue action and removes unused import. |
| packages/functional-tests/pages/signinPasswordlessCode.ts | Uses pageHeading helper; removes CMS-overridable submit-button copy match. |
| packages/functional-tests/pages/signin.ts | Replaces cached/email/password/sync heading gating with field/testid-based locators; submits via structural helper. |
| packages/functional-tests/pages/README.md | Documents “never match CMS-overridable string” locator strategy and points to the detailed rule. |
| packages/functional-tests/pages/layout.ts | Adds shared formSubmitButton helper for stable CTA selection. |
| packages/functional-tests/pages/confirmSignupCode.ts | Uses pageHeading helper instead of headline copy for h1-only pages. |
| packages/functional-tests/pages/baseTokenCode.ts | Adds pageHeading helper and routes submit via formSubmitButton instead of copy match. |
| CLAUDE.md | Updates test-rule auto-load documentation to include functional-tests locator rules. |
| .claude/rules/testing/functional-pages.md | Adds documented convention for CMS-safe locators and strict-mode hazards. |
vpomerleau
marked this pull request as ready for review
August 6, 2026 17:57
…copy Because: - Page copy is CMS-overridable per relying party, so page objects matching on default English text break whenever a relying party customizes it. This took down the confirm-signup-code step of the Sync send-tab smoke test in production, where the CTA renders as "Continue". - Matching on copy also asserts the opposite of the contract: it fails precisely when CMS is working as intended. - CMS overrides content but never routes, so page identity belongs on the URL, on form fields whose labels come from FTL, or on an explicit test id. - The convention was recorded nowhere, so the pattern keeps being reintroduced. This commit: - Adds BaseLayout.formSubmitButton and BaseTokenCodePage.pageHeading, matching the submit button and heading structurally instead of by label, and routes the CTA getters across nine page objects through them. - Scopes formSubmitButton to `form` so the getter stays single-match on pages that render more than one submit button, since every page object inherits it. - Drops the redundant submitButton overrides on signinUnblock and signinPasswordlessCode, and the confirmButton override on signinRecoveryPhone. - Replaces the copy-matched heading getters with the elements that actually identify each step: the email and password fields, whose labels are FTL-only, and a new `cached-signin-submit` test id for the cached step, which shares /signin with the password step and has no distinguishing field. - Applies pageHeading only to the code pages, which render a single h1 behind a checkPath() guard. Level matching is unsafe elsewhere: a strict-mode violation throws instead of retrying, so such assertions stop working as waits for a navigation. - Locates the signup-confirmed-sync pair CTA by its Glean id, since that button renders outside a form with no type attribute. - Matches only the brand-free portion of the Relay set-password copy, which Fluent renders with bidi isolation marks around the brand term. - Deletes the unused fillOutAuthenticationForm and the getters only it reached. - Covers CMS headline and primaryButtonText in the component unit tests, which is the layer that should own copy assertions, guards the new test id there too, and constrains MOCK_CMS_INFO with `satisfies RelierCmsInfo`. - Records the locator rules, including the strict-mode hazard, in .claude/rules/testing/functional-pages.md, which loads whenever a page object or spec is edited.
vbudhram
approved these changes
Aug 6, 2026
vbudhram
left a comment
vbudhram
left a comment
Contributor
There was a problem hiding this comment.
@vpomerleau I started CI for playwright tests. Have you ran the previous failing tests against stage? Assuming those pass this should be good to merge 👍🏽
Contributor
Author
Good call, just ran the |
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
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.
Because
This pull request
formSubmitButtonandpageHeadinghelpers inpackages/functional-tests/pages/layout.tsandbaseTokenCode.ts, and routes the CTA getters through them.data-testid="cached-signin-submit"for the cached step, which shares/signinwith the password step.type.fxa-settingscomponent tests and constrainsMOCK_CMS_INFOwithsatisfies RelierCmsInfo..claude/rules/testing/functional-pages.md.Issue that this pull request solves
Closes: FXA-13790
Checklist
Put an
xin the boxes that applyHow to review (Optional)
pages/layout.tsandpages/baseTokenCode.tsfor the shared getters, andSigninCached/index.tsxfor the only production change..claude/rules/testing/functional-pages.mdfor the reasoning, then the page objects, then the spec sweep.Screenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
Full local functional run is green, including the two specs that previously failed deterministically. Not yet validated against a CMS-configured relying party on stage or prod.
SigninTokenCodenever passesprimaryButtonTextto its CTA, so CMS button text is silently ignored there. Pre-existing and out of scope; worth its own ticket.