test(presence): dangling connections #37710

cardoso · 2025-12-06T21:21:45Z

[WIP] Tests for #37551

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

Summary by CodeRabbit

Tests
- Introduced end-to-end test for stale user session removal, validating proper presence status transitions in chat channels
- Enhanced testing infrastructure with database connectivity capabilities, enabling direct database operations during e2e test execution

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Fixes user status inaccuracy by refreshing active connections and filtering out the stale ones.

codecov · 2025-12-11T13:53:58Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.69%. Comparing base (5896e3b) to head (5b2a5ce).
⚠️ Report is 19 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #37710      +/-   ##
===========================================
- Coverage    67.69%   67.69%   -0.01%     
===========================================
  Files         3452     3452              
  Lines       113983   113982       -1     
  Branches     20943    20943              
===========================================
- Hits         77163    77156       -7     
- Misses       34693    34695       +2     
- Partials      2127     2131       +4

Flag	Coverage Δ
e2e	`57.25% <ø> (+<0.01%)`	⬆️
e2e-api	`42.25% <ø> (-0.06%)`	⬇️

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.

cardoso · 2025-12-12T13:26:42Z

@coderabbitai review

coderabbitai · 2025-12-12T13:26:48Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (3)

apps/meteor/tests/e2e/presence-stale.spec.ts (2)

26-27: Prefer semantic locators over CSS class selectors.

Per coding guidelines, avoid page.locator() in favor of semantic locators like getByRole(), getByLabel(), etc. The status bullet locator uses a CSS class selector which is more brittle and less accessible.

Consider using an approach similar to lines 60-68 which uses getByRole('button', { name: 'user2' }).getByRole('img') for better consistency.

29-72: Consider restoring database state after test for isolation.

The test mutates user2's status and session timestamps in the database (lines 33-57) but doesn't restore the original state. Per coding guidelines, tests should maintain isolation and ensure clean state for each execution.

If other tests depend on user2's presence status, this mutation could cause flakiness. Consider adding cleanup logic in a try/finally block or using test.afterEach() to restore the original user status and session data.
apps/meteor/tests/e2e/utils/db.ts (1)
6-17: Consider making properties readonly.

The client, users, and usersSessions properties are assigned once in the constructor and shouldn't change. Marking them as readonly prevents accidental reassignment and communicates intent.
 export class DatabaseClient {
-	client: MongoClient;
+	readonly client: MongoClient;

-	users: UsersRaw;
+	readonly users: UsersRaw;

-	usersSessions: UsersSessionsRaw;
+	readonly usersSessions: UsersSessionsRaw;

📜 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 93eedcc and 5b2a5ce.

📒 Files selected for processing (3)

apps/meteor/tests/e2e/presence-stale.spec.ts (1 hunks)
apps/meteor/tests/e2e/utils/db.ts (1 hunks)
apps/meteor/tests/e2e/utils/test.ts (2 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

**/*.{ts,tsx,js}

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

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

apps/meteor/tests/e2e/presence-stale.spec.ts
apps/meteor/tests/e2e/utils/db.ts
apps/meteor/tests/e2e/utils/test.ts

**/*.spec.ts

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

**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use .spec.ts extension for test files (e.g., login.spec.ts)

Files:

apps/meteor/tests/e2e/presence-stale.spec.ts

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

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

apps/meteor/tests/e2e/**/*.spec.ts: All test files must be created in apps/meteor/tests/e2e/ directory
Avoid using page.locator() in Playwright tests - always prefer semantic locators such as page.getByRole(), page.getByLabel(), page.getByText(), or page.getByTitle()
Use test.beforeAll() and test.afterAll() for setup/teardown in Playwright tests
Use test.step() for complex test scenarios to improve organization in Playwright tests
Group related tests in the same file
Utilize Playwright fixtures (test, page, expect) for consistency in test files
Prefer web-first assertions (toBeVisible, toHaveText, etc.) in Playwright tests
Use expect matchers for assertions (toEqual, toContain, toBeTruthy, toHaveLength, etc.) instead of assert statements in Playwright tests
Use page.waitFor() with specific conditions instead of hardcoded timeouts in Playwright tests
Implement proper wait strategies for dynamic content in Playwright tests
Maintain test isolation between test cases in Playwright tests
Ensure clean state for each test execution in Playwright tests
Ensure tests run reliably in parallel without shared state conflicts

Files:

apps/meteor/tests/e2e/presence-stale.spec.ts

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

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

apps/meteor/tests/e2e/**/*.{ts,spec.ts}: Store commonly used locators in variables/constants for reuse
Follow Page Object Model pattern consistently in Playwright tests

Files:

apps/meteor/tests/e2e/presence-stale.spec.ts
apps/meteor/tests/e2e/utils/db.ts
apps/meteor/tests/e2e/utils/test.ts

🧠 Learnings (14)

📓 Common learnings

Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.

Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests

Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files

Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests