test: Decouple `setupE2EEPassword` and use in flaky #36969

dougfabris · 2025-09-17T13:12:18Z

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

FLAKY-1542

Summary by CodeRabbit

Tests
- Consolidated E2E passphrase setup into a reusable helper to reduce duplication and simplify flows.
- Updated encryption/decryption and passphrase management tests to use the new helper, improving readability and consistency.
- Removed fragile UI-step dependencies in tests, increasing reliability and reducing flakiness.
- Maintained verification of passphrase usage to ensure end-to-end coverage remains intact.
- Overall, improves test stability and speeds up CI without affecting user-facing behavior.

dionisio-bot · 2025-09-17T13:12:23Z

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

changeset-bot · 2025-09-17T13:12:23Z

⚠️ No Changeset found

Latest commit: 86c35e0

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

coderabbitai · 2025-09-17T13:12:29Z

Walkthrough

Introduces a new Playwright helper setupE2EEPassword(page) and updates two E2E test suites to use it instead of directly interacting with SaveE2EEPasswordBanner/Modal. The helper encapsulates banner click, modal interaction, password retrieval, confirmation, and banner disappearance. Tests now call the helper to initialize passphrase and proceed with existing verification steps.

Changes

Cohort / File(s)	Summary
E2E passphrase setup helper `apps/meteor/tests/e2e/e2e-encryption/setupE2EEPassword.ts`	New helper exported: setupE2EEPassword(page: Page): Promise. Clicks SaveE2EEPasswordBanner, reads generated password from SaveE2EEPasswordModal, confirms, waits for banner to disappear, returns password.
E2E tests updated to use helper `apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts`, `apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts`	Replaced explicit banner/modal interactions with setupE2EEPassword(page). Tests retain existing password entry/verification flows; some tests call the helper to initialize/reset state.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor T as Test
  participant B as SaveE2EEPasswordBanner
  participant M as SaveE2EEPasswordModal

  rect rgba(200,230,255,0.3)
  note over T: setupE2EEPassword(page)
  T->>B: click()
  B-->>M: open modal
  T->>M: getPassword()
  M-->>T: generated password
  T->>M: confirm()
  T->>B: wait for hidden
  end
  T-->>T: returns password

sequenceDiagram
  autonumber
  actor T as Test
  participant Helper as setupE2EEPassword
  participant EnterBanner as EnterE2EEPasswordBanner
  participant EnterModal as EnterE2EEPasswordModal

  T->>Helper: setupE2EEPassword(page)
  Helper-->>T: password
  T->>EnterBanner: click()
  EnterBanner-->>EnterModal: open
  T->>EnterModal: fill(password) + confirm
  EnterModal-->>T: proceed

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

test: modularize e2e encryption tests and extract common utilities #36890 — Touches the same E2EE test area and introduces/uses the same setupE2EEPassword helper in related specs.

Suggested labels

stat: ready to merge, stat: QA assured

Suggested reviewers

cardoso
tassoevan

Poem

A hop and a click, the banner slips away,
I nibble a passphrase, fresh as hay.
Modal blinks—“confirmed,” I cheer,
Tests bound tight, with secrets clear.
In burrows of code, I stash the key,
Thump-thump—E2EE! 🐇🔐

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "test: Decouple `setupE2EEPassword` and use in flaky" concisely describes the primary change: extracting a reusable setupE2EEPassword helper and applying it to stabilize a flaky e2e test. The diff adds apps/meteor/tests/e2e/e2e-encryption/setupE2EEPassword.ts and updates e2e tests to call this helper instead of directly manipulating the banner/modal, which aligns with the title. The phrasing is focused and relevant to the test-focused change and intent to address flakiness.
Linked Issues Check	✅ Passed	The linked issue FLAKY-1542 targets the "should reset e2e password from the modal" scenario; the PR centralizes E2EE password setup into setupE2EEPassword and inserts calls to it in e2e-passphrase-management.spec.ts, including before the test that should reset the password from the modal. The raw summary shows the helper returns the generated password and replaces direct banner/modal interactions, which directly addresses the coding objective to make the modal-based reset flow deterministic. Based on the provided changes, the implementation aligns with the linked issue's coding requirements.
Out of Scope Changes Check	✅ Passed	All modifications in the raw summary are confined to test files under apps/meteor/tests/e2e/e2e-encryption and the new setupE2EEPassword helper; there are no edits to production code or unrelated directories. The changes consolidate duplicated UI interactions into a helper and are directly connected to the linked flaky-test objective, so I find no out-of-scope changes. This PR appears scoped to the stated testing improvement.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch test/saveE2EE-flaky

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2025-09-17T16:35:03Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.21%. Comparing base (f139c0d) to head (86c35e0).

Additional details and impacted files

@@           Coverage Diff            @@
##           develop   #36969   +/-   ##
========================================
  Coverage    66.21%   66.21%           
========================================
  Files         3384     3384           
  Lines       115027   115027           
  Branches     21064    21065    +1     
========================================
  Hits         76161    76161           
- Misses       36260    36262    +2     
+ Partials      2606     2604    -2

Flag	Coverage Δ
e2e	`56.96% <ø> (-0.04%)`	⬇️
unit	`71.19% <ø> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (3)

apps/meteor/tests/e2e/e2e-encryption/setupE2EEPassword.ts (1)
8-8: Annotate explicit return type for public helper.

Make the API contract clear and prevent widening if getPassword() changes.
-export const setupE2EEPassword = async (page: Page) => {
+export const setupE2EEPassword = async (page: Page): Promise<string> => {
apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts (2)
44-44: Optional: wrap setup in a named step for clearer traces.

Improves Playwright trace readability.
- await setupE2EEPassword(page);
+ await test.step('setup E2EE password', async () => {
+   await setupE2EEPassword(page);
+ });
44-44: Move setupE2EEPassword(page) to immediately after creating the encrypted channel.

In apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts the helper is called before createEncryptedChannel (lines 44 and 84); if the SaveE2EE banner only appears after opening/creating an encrypted room, call setupE2EEPassword(page) immediately after await sidenav.createEncryptedChannel(channelName).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between b1cefb0 and 7ff1f80.

📒 Files selected for processing (3)

apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts (3 hunks)
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (4 hunks)
apps/meteor/tests/e2e/e2e-encryption/setupE2EEPassword.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}: Write concise, technical TypeScript/JavaScript with accurate typing
Follow DRY by extracting reusable logic into helper functions or page objects
Avoid code comments in the implementation

Files:

apps/meteor/tests/e2e/e2e-encryption/setupE2EEPassword.ts
apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts

apps/meteor/tests/e2e/**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

apps/meteor/tests/e2e/**/*.{ts,tsx}: Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Store commonly used locators in variables/constants for reuse
Use page.waitFor() with specific conditions and avoid hardcoded timeouts
Implement proper wait strategies for dynamic content
Follow the Page Object Model pattern consistently

Files:

apps/meteor/tests/e2e/e2e-encryption/setupE2EEPassword.ts
apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts

apps/meteor/tests/e2e/**/*.spec.ts

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

apps/meteor/tests/e2e/**/*.spec.ts: All Playwright test files must be located under apps/meteor/tests/e2e/ and use the .spec.ts extension (e.g., login.spec.ts)
Use descriptive test names that clearly communicate expected behavior
Use test.beforeAll() and test.afterAll() for setup and teardown
Use test.step() to organize complex test scenarios
Group related tests in the same file
Utilize Playwright fixtures (test, page, expect) consistently
Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Use expect matchers (toEqual, toContain, toBeTruthy, toHaveLength, etc.) instead of assert statements
Maintain test isolation between test cases
Ensure a clean state for each test execution
Ensure tests run reliably in parallel without shared state conflicts

Files:

apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts

🧠 Learnings (14)

📓 Common learnings

Learnt from: CR
PR: RocketChat/Rocket.Chat#0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (test, page, expect) consistently