test: flaky should reset e2e password from the modal

[dougfabris]

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

FLAKY-1542

Summary by CodeRabbit

• Tests
  • Updated end-to-end login checks to wait for the main application area to become visible after the login button disappears. This adds an explicit post-login validation to the flow and makes the success condition stricter. No changes to public interfaces or runtime behavior; impact is limited to the automated test suite.

[dionisio-bot]

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

[changeset-bot]

⚠️ No Changeset found

Latest commit: 6380bd9

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.45%. Comparing base (e82cfaa) to head (6380bd9).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #36888      +/-   ##
===========================================
+ Coverage    66.42%   66.45%   +0.03%     
===========================================
  Files         3341     3343       +2     
  Lines       114284   114244      -40     
  Branches     21072    21181     +109     
===========================================
+ Hits         75913    75926      +13     
+ Misses       35686    35643      -43     
+ Partials      2685     2675      -10     

Flag	Coverage Δ
e2e	57.76% <ø> (+0.06%)	⬆️
unit	71.13% <ø> (-0.03%)	⬇️

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]

Walkthrough

Added a visibility assertion in LoginPage.waitForLogin to ensure the main landmark is visible after the login button disappears, strengthening the post-login wait condition in the E2E test.

Changes

Cohort / File(s)	Summary
E2E Login Page Object apps/meteor/tests/e2e/page-objects/login.ts	Added await expect(this.page.getByRole('main')).toBeVisible() after hiding of login button to confirm main content visibility post-login. No exports/signatures changed.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant T as Test Runner
  participant LP as LoginPage (page-object)
  participant P as Playwright Page

  T->>LP: waitForLogin()
  LP->>P: wait for login button to be hidden
  Note over LP,P: New step added
  LP->>P: assert <main> landmark is visible
  P-->>LP: visible
  LP-->>T: done

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title Check	⚠️ Warning	The changeset adds an assertion in LoginPage.waitForLogin to wait for the main landmark to be visible after login, but the PR title "test: flaky should reset e2e password from the modal" refers to resetting an e2e password via a modal which is not present in the diff, so the title is misleading and does not summarize the primary change.	Rename the PR to clearly summarize the actual change (for example: "test(e2e): require main landmark to be visible after login") and update the PR description to state how this change addresses FLAKY-1542 and include brief steps to reproduce/verify.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

Poem

I thump my paw—“log in, proceed!”
Now main appears, just what we need.
The button fades, the content gleams,
A tidy check to guard our dreams.
Hop-safe tests, in burrows bright,
Ensure our apps wake up just right. 🐇✨

✨ 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/e2e-waitForLogin

---

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

apps/meteor/tests/e2e/page-objects/login.ts (1)

21-24: Good deflake; tighten the readiness signal to reduce residual flakiness.

Adding the main landmark check is sensible. Consider also asserting that we’ve actually left the /login route and give CI more headroom with an explicit timeout.

Proposed tweak:

 protected async waitForLogin() {
-  await expect(this.loginButton).not.toBeVisible();
-  await expect(this.page.getByRole('main')).toBeVisible();
+  await expect(this.loginButton).not.toBeVisible();
+  // Ensure navigation/state left the login route
+  await expect(this.page).not.toHaveURL(/\/login\b/);
+  // Main content fully rendered (more generous in CI)
+  await expect(this.page.getByRole('main')).toBeVisible({ timeout: 30_000 });
 }

If the login screen also contains a

region in some flows (e.g., SSO, e2e-reset modal), please confirm this remains unambiguous; otherwise switch the assertion to a more specific, stable post-login element (e.g., sidebar navigation or rooms list).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e82cfaa and 6380bd9.

📒 Files selected for processing (1)

• apps/meteor/tests/e2e/page-objects/login.ts (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: CodeQL-Build
• GitHub Check: CodeQL-Build