test: refactor e2ee encrypted channels tests #36987
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughAdds many focused Playwright E2EE end-to-end specs, new page-object fragments and utilities (resolve-room-id, room-toolbar, pinned/starred tabs, E2EE actions), removes a large monolithic spec, and centralizes created-channel tracking and API cleanup across tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Test Runner
participant API as Admin API
participant B as Browser/UI
participant S as Server
T->>API: enable E2E settings
API-->>T: 200 OK
T->>B: open /home (storageState)
B-->>T: home ready
T->>B: create private channel (CreateE2EEChannel.createAndStore)
B->>S: create room
S-->>B: room created → toast, navigate to /group/{name}
alt enable encryption via modal
T->>B: Open kebab → EnableRoomEncryptionModal.enable()
B->>S: enable E2EE for room
S-->>B: header lock icon visible
end
T->>B: send message
B->>S: post (encrypted/unencrypted based on state)
S-->>B: message saved (encrypted icon shown/hidden)
T->>API: delete created channels (deleteRoom)
API-->>T: 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jessicaschelly
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #36987 +/- ##
===========================================
- Coverage 67.37% 66.34% -1.03%
===========================================
Files 3330 3382 +52
Lines 113482 115625 +2143
Branches 20598 21247 +649
===========================================
+ Hits 76454 76712 +258
- Misses 34418 36293 +1875
- Partials 2610 2620 +10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
jessicaschelly
changed the title
jessicaschelly
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/tests/e2e/page-objects/fragments/home-content.ts (1)
493-498: Bug: setChecked called on a label, not the checkbox (will fail).
Locator.setCheckedmust target the checkbox element (or role=checkbox), not the label.- async toggleAlsoSendThreadToChannel(isChecked: boolean): Promise<void> { - await this.page - .getByRole('dialog') - .locator('label', { has: this.page.getByRole('checkbox', { name: 'Also send to channel' }) }) - .setChecked(isChecked); - } + async toggleAlsoSendThreadToChannel(isChecked: boolean): Promise<void> { + const cb = this.page.getByRole('dialog').getByRole('checkbox', { name: 'Also send to channel' }); + await cb.setChecked(isChecked); + }
🧹 Nitpick comments (15)
apps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts (3)
31-31: Fix grammar in test title.Use “an encrypted” instead of “a encrypted”.
-test('expect create a private encrypted channel and send a encrypted thread message', async ({ page }) => { +test('expect create a private encrypted channel and send an encrypted thread message', async ({ page }) => {
57-61: Prefer the page-object API to send thread messages (more reliable).Leverage
sendMessageInThreadinstead of filling and pressing Enter. Keeps logic DRY and consistent with other flows.- await test.step('send encrypted reply on thread message', async () => { - await poHomeChannel.content.toggleAlsoSendThreadToChannel(true); - await poHomeChannel.content.inputThreadMessage.fill('This is an encrypted thread message also sent in channel'); - await page.keyboard.press('Enter'); - }); + await test.step('send encrypted reply on thread message', async () => { + await poHomeChannel.content.toggleAlsoSendThreadToChannel(true); + await poHomeChannel.content.sendMessageInThread('This is an encrypted thread message also sent in channel'); + });
22-25: Optional: navigate via page-object for consistency.Use
poHomeChannel.goto()to centralize navigation behavior.- await page.goto('/home'); + await poHomeChannel.goto();apps/meteor/tests/e2e/e2e-encryption/e2ee-sidebar.spec.ts (3)
35-38: Fix typos in test title."unecrypted" → "unencrypted".
-test('expect create a private channel, send unecrypted messages, encrypt the channel and delete the last message and check the last message in the sidebar', async ({ +test('expect create a private channel, send unencrypted messages, encrypt the channel and delete the last message and check the last message in the sidebar', async ({
42-44: Consider restoring sidebar display mode after the test.Changing user UI preferences can leak across tests that reuse the same storage state.
If other specs assume a different display mode, add a small helper to capture current mode and restore it in
test.afterEach.
63-66: Avoid forcing clicks unless necessary.Try waiting for the element to be visible and enabled before clicking; fall back to
{ force: true }only if unavoidable.- await poHomeChannel.tabs.kebab.click({ force: true }); + await expect(poHomeChannel.tabs.kebab).toBeVisible(); + await expect(poHomeChannel.tabs.kebab).toBeEnabled(); + await poHomeChannel.tabs.kebab.click(); - await poHomeChannel.tabs.btnEnableE2E.click({ force: true }); + await expect(poHomeChannel.tabs.btnEnableE2E).toBeVisible(); + await poHomeChannel.tabs.btnEnableE2E.click();apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts (1)
53-65: Use explicit hidden assertion for clarity.
toBeHidden()communicates intent better than negating visibility.- await expect(poHomeChannel.content.encryptedRoomHeaderIcon).not.toBeVisible(); + await expect(poHomeChannel.content.encryptedRoomHeaderIcon).toBeHidden();Also consider the same for the per-message encrypted icon in Line 64.
apps/meteor/tests/e2e/page-objects/fragments/home-content.ts (3)
70-72: Tighten “More” button locator with exact match.Prevents accidental matches with similarly-named controls (e.g., “More actions”).
- getMessageMoreButton(messageLocator: Locator): Locator { - return messageLocator.getByRole('button', { name: 'More' }); - } + getMessageMoreButton(messageLocator: Locator): Locator { + return messageLocator.getByRole('button', { name: 'More', exact: true }); + }
74-80: Prefer exact matching for mention locators.Reduces false positives when names are substrings.
- getUserMention(username: string): Locator { - return this.page.getByRole('button', { name: username }); - } + getUserMention(username: string): Locator { + return this.page.getByRole('button', { name: username, exact: true }); + } - getChannelMention(channelName: string): Locator { - return this.page.getByRole('button', { name: channelName }); - } + getChannelMention(channelName: string): Locator { + return this.page.getByRole('button', { name: channelName, exact: true }); + }
477-481: Use semantic getByRole and reuse helper to open message menu.Avoid selector strings and duplicate logic.
- async openLastMessageMenu(): Promise<void> { - await this.page.locator('[data-qa-type="message"]').last().hover(); - await this.page.locator('[data-qa-type="message"]').last().locator('role=button[name="More"]').waitFor(); - await this.page.locator('[data-qa-type="message"]').last().locator('role=button[name="More"]').click(); - } + async openLastMessageMenu(): Promise<void> { + const msg = this.page.locator('[data-qa-type="message"]').last(); + await msg.hover(); + await this.getMessageMoreButton(msg).waitFor(); + await this.getMessageMoreButton(msg).click(); + }apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.ts (3)
31-31: Fix grammar in test title.-test('expect create a encrypted private channel and mention user', async ({ page }) => { +test('expect create an encrypted private channel and mention user', async ({ page }) => {
47-47: Fix grammar in test title.-test('expect create a encrypted private channel, mention a channel and navigate to it', async ({ page }) => { +test('expect create an encrypted private channel, mention a channel and navigate to it', async ({ page }) => {
68-68: Fix grammar in test title.-test('expect create a encrypted private channel, mention a channel and user', async ({ page }) => { +test('expect create an encrypted private channel, mention a channel and user', async ({ page }) => {apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (2)
49-55: Stronger disabled-state assertions.Prefer ARIA/state attributes over class names for robustness.
- await expect(poHomeChannel.content.btnForwardMessageDisabled).toBeDisabled(); + await expect(poHomeChannel.content.btnForwardMessageDisabled).toHaveAttribute(/(disabled|aria-disabled)/, /^(true|)$/); … - await expect(poHomeChannel.content.btnOptionReplyInDirectMessage).toHaveClass(/disabled/); - await expect(poHomeChannel.content.btnOptionCopyLink).toHaveClass(/disabled/); + await expect(poHomeChannel.content.btnOptionReplyInDirectMessage).toHaveAttribute('aria-disabled', 'true'); + await expect(poHomeChannel.content.btnOptionCopyLink).toHaveAttribute('aria-disabled', 'true');If the menu uses classes only, keep the class checks as fallback.
66-68: Fix typos: “stared” → “starred”.- await poHomeChannel.content.sendMessage('This message should be pinned and stared.'); - await expect(poHomeChannel.content.lastUserMessageBody).toHaveText('This message should be pinned and stared.'); + await poHomeChannel.content.sendMessage('This message should be pinned and starred.'); + await expect(poHomeChannel.content.lastUserMessageBody).toHaveText('This message should be pinned and starred.'); … - await expect(poHomeChannel.content.lastPinnedMessage).toContainText('This message should be pinned and stared.'); + await expect(poHomeChannel.content.lastPinnedMessage).toContainText('This message should be pinned and starred.'); … - await expect(poHomeChannel.content.lastStarredMessage).toContainText('This message should be pinned and stared.'); + await expect(poHomeChannel.content.lastStarredMessage).toContainText('This message should be pinned and starred.');Also applies to: 90-91, 102-104
📜 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.
📒 Files selected for processing (11)
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.ts(0 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts(0 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts(0 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts(0 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.ts(0 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-sidebar.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/home-content.ts(1 hunks)
💤 Files with no reviewable changes (5)
- apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts
- apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.ts
- apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
- apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
- apps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.ts
🧰 Additional context used
📓 Path-based instructions (4)
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-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-sidebar.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts
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/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-sidebar.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.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/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-sidebar.spec.tsapps/meteor/tests/e2e/page-objects/fragments/home-content.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts
apps/meteor/tests/e2e/page-objects/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Files:
apps/meteor/tests/e2e/page-objects/fragments/home-content.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
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-sidebar.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-sidebar.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Group related tests in the same file
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-sidebar.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Maintain test isolation between test cases
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-sidebar.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Ensure a clean state for each test execution
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-sidebar.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-sidebar.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Write concise, technical TypeScript/JavaScript with accurate typing
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-sidebar.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-sidebar.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : All Playwright test files must be located under apps/meteor/tests/e2e/ and use the .spec.ts extension (e.g., login.spec.ts)
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-sidebar.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Avoid code comments in the implementation
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use test.step() to organize complex test scenarios
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use test.beforeAll() and test.afterAll() for setup and teardown
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-sidebar.spec.ts
🧬 Code graph analysis (6)
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (2)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)
apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.ts (2)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts (3)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (2)
EnableRoomEncryptionModal(118-135)DisableRoomEncryptionModal(137-154)
apps/meteor/tests/e2e/e2e-encryption/e2ee-sidebar.spec.ts (3)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (1)
EnableRoomEncryptionModal(118-135)
apps/meteor/tests/e2e/page-objects/fragments/home-content.ts (1)
apps/meteor/tests/e2e/page-objects/auth.ts (1)
username(50-52)
apps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts (2)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)
⏰ 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). (4)
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 📦 Meteor Build - coverage
🔇 Additional comments (2)
apps/meteor/tests/e2e/page-objects/fragments/home-content.ts (1)
66-68: LGTM: encrypted icon accessor is clear and reusable.apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts (1)
21-24: Global E2E setting toggles — isolate or restore
Multiple specs mutate /settings/E2E_Enable and /settings/E2E_Allow_Unencrypted_Messages, which can cause cross‑spec flakiness. Either run the E2E-encryption suite isolated/serial (dedicated worker) or ensure every spec restores the prior values in test.afterAll.Observed files that set these settings: apps/meteor/tests/e2e/account-login.spec.ts; apps/meteor/tests/e2e/e2e-encryption/{e2ee-channel.spec.ts, e2ee-pdf-export.spec.ts, e2ee-mentions.spec.ts, e2ee-legacy-format.spec.ts, e2ee-message-actions.spec.ts, e2ee-key-reset.spec.ts, e2ee-file-encryption.spec.ts, e2ee-passphrase-management.spec.ts (restores original settings in afterAll lines 40–41), e2ee-encryption-decryption.spec.ts, e2ee-server-settings.spec.ts, e2ee-sidebar.spec.ts, e2ee-threads.spec.ts, e2ee-otr.spec.ts}.
There was a problem hiding this comment.
what do you think if we start to refactor these page objects as we're doing for the encrypted classes.
e.g: EnableRoomEncryptionModal, DisableRoomEncryptionModal
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/meteor/tests/e2e/page-objects/fragments/message.ts (1)
18-20: Expose-and-reuse the Message.moreButton to de-duplicate across tabsGetter looks good and uses an accessible role. Please update PinnedMessagesTab/StarredMessagesTab to reuse this (
new Message(this.lastMessage).moreButton) instead of re-implementing their own "More" locator.apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts (1)
16-18: DRY: reuse Message.moreButton + prefer web-first assertions over waitForAvoid duplicating the "More" locator and replace
waitFor()withexpect(...).toBeVisible()to follow our Playwright guidelines and reduce flakiness.Apply:
@@ -import type { Locator, Page } from '@playwright/test'; +import type { Locator, Page } from '@playwright/test'; +import { Message } from './message'; @@ - private getMessageMoreButton(messageLocator: Locator): Locator { - return messageLocator.getByRole('button', { name: 'More' }); - } @@ async openLastMessageMenu(): Promise<void> { await this.lastMessage.hover(); - await this.getMessageMoreButton(this.lastMessage).waitFor(); - await this.getMessageMoreButton(this.lastMessage).click(); + const message = new Message(this.lastMessage); + await expect(message.moreButton).toBeVisible(); + await message.moreButton.click(); }Also applies to: 28-32
apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (2)
163-173: Reduce brittleness: use regex names and scope to the open menuExact strings are likely to change; also, scoping to the open menu avoids cross-UI collisions if multiple menus exist.
Apply:
@@ - private get forwardMessageButton(): Locator { - return this.page.getByRole('button', { name: 'Forward message not available on encrypted content' }); - } + private get forwardMessageButton(): Locator { + return this.page.getByRole('button', { name: /Forward message/i }); + } @@ - private get replyInDirectMessageOption(): Locator { - return this.page.getByRole('menuitem', { name: 'Reply in direct message' }); - } + private get menu(): Locator { + return this.page.getByRole('menu'); + } + private get replyInDirectMessageOption(): Locator { + return this.menu.getByRole('menuitem', { name: /Reply in direct message/i }); + } @@ - private get copyLinkOption(): Locator { - return this.page.getByRole('menuitem', { name: 'Copy link' }); - } + private get copyLinkOption(): Locator { + return this.menu.getByRole('menuitem', { name: /Copy link/i }); + }
175-185: Prefer accessibility state over CSS class for “disabled”Asserting
toHaveClass(/disabled/)is brittle. Preferaria-disabled="true"for menuitems andtoBeDisabled()for buttons.Apply:
@@ - async expectForwardMessageToBeDisabled(): Promise<void> { - await expect(this.forwardMessageButton).toBeDisabled(); - } + async expectForwardMessageToBeDisabled(): Promise<void> { + await expect(this.forwardMessageButton).toBeDisabled(); + } @@ - async expectReplyInDirectMessageToBeDisabled(): Promise<void> { - await expect(this.replyInDirectMessageOption).toHaveClass(/disabled/); - } + async expectReplyInDirectMessageToBeDisabled(): Promise<void> { + await expect(this.replyInDirectMessageOption).toHaveAttribute('aria-disabled', 'true'); + } @@ - async expectCopyLinkToBeDisabled(): Promise<void> { - await expect(this.copyLinkOption).toHaveClass(/disabled/); - } + async expectCopyLinkToBeDisabled(): Promise<void> { + await expect(this.copyLinkOption).toHaveAttribute('aria-disabled', 'true'); + }If
aria-disabledisn’t present, fall back to a semantic attribute the component actually sets (e.g.,data-disabled="true").apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts (1)
16-18: Match Pinned tab: reuse Message.moreButton + web-first assertMirror the same refactor here to reduce duplication and improve stability.
Apply:
@@ -import type { Locator, Page } from '@playwright/test'; +import type { Locator, Page } from '@playwright/test'; +import { Message } from './message'; @@ - private getMessageMoreButton(messageLocator: Locator): Locator { - return messageLocator.getByRole('button', { name: 'More' }); - } @@ async openLastMessageMenu(): Promise<void> { await this.lastMessage.hover(); - await this.getMessageMoreButton(this.lastMessage).waitFor(); - await this.getMessageMoreButton(this.lastMessage).click(); + const message = new Message(this.lastMessage); + await expect(message.moreButton).toBeVisible(); + await message.moreButton.click(); }Also applies to: 28-32
📜 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.
📒 Files selected for processing (6)
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/index.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/message.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/meteor/tests/e2e/page-objects/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Files:
apps/meteor/tests/e2e/page-objects/fragments/index.tsapps/meteor/tests/e2e/page-objects/fragments/message.tsapps/meteor/tests/e2e/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts
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/page-objects/fragments/index.tsapps/meteor/tests/e2e/page-objects/fragments/message.tsapps/meteor/tests/e2e/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.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/page-objects/fragments/index.tsapps/meteor/tests/e2e/page-objects/fragments/message.tsapps/meteor/tests/e2e/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts
🧠 Learnings (12)
📓 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
📚 Learning: 2025-09-16T22:08:51.490Z
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/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/index.tsapps/meteor/tests/e2e/page-objects/fragments/message.tsapps/meteor/tests/e2e/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/index.tsapps/meteor/tests/e2e/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/index.tsapps/meteor/tests/e2e/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/index.tsapps/meteor/tests/e2e/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/message.tsapps/meteor/tests/e2e/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Use page.waitFor() with specific conditions and avoid hardcoded timeouts
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : All Playwright test files must be located under apps/meteor/tests/e2e/ and use the .spec.ts extension (e.g., login.spec.ts)
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Implement proper wait strategies for dynamic content
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts
🔇 Additional comments (1)
apps/meteor/tests/e2e/page-objects/fragments/index.ts (1)
11-12: Re-exports LGTMSurface area looks consistent with the fragments pattern; no issues.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts (3)
9-10: Make the dialog locator resilient to casing/localizationUse a case‑insensitive name to reduce flakiness and future i18n drift.
- this.root = page.getByRole('dialog', { name: 'Pinned Messages' }); + this.root = page.getByRole('dialog', { name: /pinned messages/i });
12-14: Prefer semantic locators over attribute selectorsPer our E2E guidelines, favor getByRole where possible. Adjust role as appropriate for your DOM (article vs listitem).
Option A (article):
- return this.root.locator('[data-qa-type="message"]').last(); + return this.root.getByRole('article').last();Option B (list item):
- return this.root.locator('[data-qa-type="message"]').last(); + return this.root.getByRole('listitem').last();
20-22: Allow RegExp for more flexible assertionsEnables callers to pass case‑insensitive or partial matches without pre‑processing.
- async expectLastMessageToContainText(text: string): Promise<void> { + async expectLastMessageToContainText(text: string | RegExp): Promise<void> { await expect(this.lastMessage).toContainText(text); }apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (6)
35-37: Make cleanup resilient to partial failures.If a room was never created or was already removed, Promise.all will reject and fail the suite in afterAll. Use allSettled so cleanup never flips the suite red.
Apply this diff:
- await Promise.all(createdChannels.map((channelName) => api.post('/groups.delete', { roomName: channelName }))); + await Promise.allSettled(createdChannels.map((roomName) => api.post('/groups.delete', { roomName })));
27-33: Prefer POM navigation for consistency.Use HomeChannel.goto() to keep navigation centralized and future‑proof.
Apply this diff:
- await page.goto('/home'); + await poHomeChannel.goto();
55-63: Scope assertions to the opened menu to avoid ambiguity.Current queries use page‑wide role selectors; scoping to the visible menu reduces false positives if multiple menus exist.
Assuming you adjust E2EEMessageActions to accept a root Locator (see snippet below), update this step:
await test.step('verify disabled message menu actions', async () => { await poHomeChannel.content.lastUserMessage.hover(); - await e2eeMessageActions.expectForwardMessageToBeDisabled(); + // Optional: assert the button is visible before checking disabled + await expect(page.getByRole('button', { name: 'Forward message not available on encrypted content' })).toBeVisible(); + await e2eeMessageActions.expectForwardMessageToBeDisabled(); await poHomeChannel.content.openLastMessageMenu(); - - await e2eeMessageActions.expectReplyInDirectMessageToBeDisabled(); - await e2eeMessageActions.expectCopyLinkToBeDisabled(); + const menu = page.getByRole('menu'); + await expect(menu).toBeVisible(); + const e2eeMenu = new E2EEMessageActions(menu); // requires fragment change below + await e2eeMenu.expectReplyInDirectMessageToBeDisabled(); + await e2eeMenu.expectCopyLinkToBeDisabled(); });Fragment change (apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts):
export class E2EEMessageActions { private readonly root: Locator; constructor(root: Locator) { this.root = root; } private get forwardMessageButton(): Locator { return this.root.getByRole('button', { name: 'Forward message not available on encrypted content' }); } private get replyInDirectMessageOption(): Locator { return this.root.getByRole('menuitem', { name: 'Reply in direct message' }); } private get copyLinkOption(): Locator { return this.root.getByRole('menuitem', { name: 'Copy link' }); } // ...existing expect* methods unchanged }
86-92: Wait for confirm control before clicking to reduce flake.Ensure the confirm button is rendered before interaction.
Apply this diff:
await test.step('pin the message', async () => { await poHomeChannel.content.openLastMessageMenu(); await poHomeChannel.content.btnOptionPinMessage.click(); - await poHomeChannel.content.btnModalConfirm.click(); + await expect(poHomeChannel.content.btnModalConfirm).toBeVisible(); + await poHomeChannel.content.btnModalConfirm.click(); await expect(poHomeChannel.toastSuccess).toBeVisible(); await poHomeChannel.dismissToast(); });
39-39: Sharpen test names.Use behavior‑focused phrasing; drop the leading “expect”.
Apply this diff:
-test('expect create a private encrypted channel and check disabled message menu actions on an encrypted message', async ({ page }) => { +test('disables menu actions for encrypted messages in a newly created private encrypted channel', async ({ page }) => {-test('expect create a private encrypted channel and pin/star an encrypted message', async ({ page }) => { +test('allows pinning and starring an encrypted message in a new private encrypted channel', async ({ page }) => {Also applies to: 66-66
56-58: Optional: assert visibility before disabled state.Small stability boost: verify the forward button is visible after hover before checking disabled.
Apply this diff:
await poHomeChannel.content.lastUserMessage.hover(); - await e2eeMessageActions.expectForwardMessageToBeDisabled(); + await expect(page.getByRole('button', { name: 'Forward message not available on encrypted content' })).toBeVisible(); + await e2eeMessageActions.expectForwardMessageToBeDisabled();
📜 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.
📒 Files selected for processing (4)
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/home-content.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/tests/e2e/page-objects/fragments/home-content.ts
- apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts
🧰 Additional context used
📓 Path-based instructions (4)
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-message-actions.spec.ts
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/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.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/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts
apps/meteor/tests/e2e/page-objects/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Files:
apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts
🧠 Learnings (13)
📓 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
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Group related tests in the same file
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Maintain test isolation between test cases
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Write concise, technical TypeScript/JavaScript with accurate typing
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Use page.waitFor() with specific conditions and avoid hardcoded timeouts
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts
🧬 Code graph analysis (1)
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (5)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts (1)
PinnedMessagesTab(5-23)apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts (1)
StarredMessagesTab(5-23)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (1)
E2EEMessageActions(156-186)
⏰ 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
🔇 Additional comments (6)
apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts (4)
1-3: Imports LGTMConsistent with our Playwright test utils wrapper.
16-18: Visibility assertion LGTMWeb‑first assertion aligns with our Playwright practices.
5-23: POM structure and API surface LGTMClear root scoping and concise assertions; small nits above are optional.
5-6: Export and usage verified — no action required.
apps/meteor/tests/e2e/page-objects/fragments/index.ts exports './pinned-messages-tab' and PinnedMessagesTab is instantiated in apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts.apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (2)
10-13: Good: deterministic E2EE setting management (preserve + enable during suite).Using preserveSettings plus a targeted beforeAll toggle keeps the environment predictable and restores it after the suite.
Also applies to: 23-25
42-54: LGTM: clear, web‑first checks and good use of test.step + toasts.Creation, encryption indicators, and star/pin flows are asserted with stable, semantic locators. Nice.
Also applies to: 79-92
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (12)
apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.ts (5)
27-29: Channel cleanup strategy — confirm parallel safety.Using a shared createdChannels array with afterAll is fine when tests in a file run serially; if this file ever moves to describe.parallel, array mutation could interleave. Consider per-test cleanup (afterEach) or a Set plus idempotent delete.
31-31: Tighten test names for clarity.Prefer “should …” phrasing to state behavior explicitly.
-test('expect create an encrypted private channel and mention user', async ({ page }) => { +test('should create an encrypted private channel and render a user mention', async ({ page }) => { @@ -test('expect create an encrypted private channel, mention a channel and navigate to it', async ({ page }) => { +test('should create an encrypted private channel, render a channel mention, and navigate to it', async ({ page }) => { @@ -test('expect create an encrypted private channel, mention a channel and user', async ({ page }) => { +test('should create an encrypted private channel and render both channel and user mentions', async ({ page }) => {Also applies to: 47-47, 68-68
37-37: Avoid brittle URL assertions (query strings/trailing slashes).Use pattern-based assertions to reduce flakiness across environments.
- await expect(page).toHaveURL(`/group/${channelName}`); + await expect(page).toHaveURL(new RegExp(`/group/${channelName}(\\?.*)?$`)); @@ - await expect(page).toHaveURL(`/group/${channelName}`); + await expect(page).toHaveURL(new RegExp(`/group/${channelName}(\\?.*)?$`)); @@ - await expect(page).toHaveURL(`/channel/general`); + await expect(page).toHaveURL(/\/channel\/general(\?.*)?$/); @@ - await expect(page).toHaveURL(`/group/${channelName}`); + await expect(page).toHaveURL(new RegExp(`/group/${channelName}(\\?.*)?$`));Also applies to: 53-53, 64-64, 74-74
41-44: Verify mention targets exist and are resolvable.Hard-coding '@user1' and '#general' assumes those entities exist and are visible to the current user. If not seeded uniformly, tests will flake. Prefer pulling target usernames/channels from fixtures or seeding them in beforeAll.
Also applies to: 78-82
32-32: Room name generation — ensure compatibility with server slug rules.UUIDs are usually fine, but if room-name policies change (allowed chars/length), consider a deterministic, lowercase, short slug (e.g., e2ee-mentions-<8 alnum>).
Also applies to: 48-48, 69-69
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (1)
68-95: Minor: consider asserting “reply in DM” disabled after star/pin flows tooTo strengthen coverage, add the same disabled assertion you use in the first test for “Reply in direct message” within the Pinned/Starred dialogs.
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts (6)
36-38: Make cleanup resilient (ignore missing/failed deletions).Use allSettled so a single 404/409 doesn’t fail teardown.
- await Promise.all(createdChannels.map((channelName) => api.post('/groups.delete', { roomName: channelName }))); + await Promise.allSettled( + createdChannels.map((channelName) => api.post('/groups.delete', { roomName: channelName })), + );
6-6: DRY the E2E toggle and drop force clicks.You already expose EncryptedRoomPage.enableEncryption/disableEncryption. Use them to remove duplicated modal flows and avoid force: true.
-import { DisableRoomEncryptionModal, EnableRoomEncryptionModal } from '../page-objects/fragments/e2ee'; +// Modals are exercised via EncryptedRoomPage helpers - let enableEncryptionModal: EnableRoomEncryptionModal; - let disableEncryptionModal: DisableRoomEncryptionModal; + // no direct modal handles needed - enableEncryptionModal = new EnableRoomEncryptionModal(page); - disableEncryptionModal = new DisableRoomEncryptionModal(page); + // modals handled by EncryptedRoomPage helpers - await test.step('disable encryption', async () => { - await poHomeChannel.tabs.kebab.click({ force: true }); - await expect(poHomeChannel.tabs.btnDisableE2E).toBeVisible(); - await poHomeChannel.tabs.btnDisableE2E.click({ force: true }); - await disableEncryptionModal.disable(); - await expect(poHomeChannel.content.encryptedRoomHeaderIcon).toBeHidden(); - }); + await test.step('disable encryption', async () => { + await encryptedRoomPage.disableEncryption(); + await expect(poHomeChannel.content.encryptedRoomHeaderIcon).toBeHidden(); + }); - await test.step('re-enable encryption', async () => { - await poHomeChannel.tabs.kebab.click({ force: true }); - await expect(poHomeChannel.tabs.btnEnableE2E).toBeVisible(); - await poHomeChannel.tabs.btnEnableE2E.click({ force: true }); - await enableEncryptionModal.enable(); - await expect(poHomeChannel.content.encryptedRoomHeaderIcon).toBeVisible(); - }); + await test.step('re-enable encryption', async () => { + await encryptedRoomPage.enableEncryption(); + await expect(poHomeChannel.content.encryptedRoomHeaderIcon).toBeVisible(); + });Also applies to: 18-19, 30-31, 56-62, 70-76
41-41: Channel name: ensure RFC/validation-safe.Some deployments restrict starting chars; prefer a slug prefixed with letters.
- const channelName = faker.string.uuid(); + const channelName = `e2ee-${faker.string.uuid().replace(/-/g, '').slice(0, 8)}`;
86-86: Same here: safer channel slug.- const channelName = faker.string.uuid(); + const channelName = `e2ee-${faker.string.uuid().replace(/-/g, '').slice(0, 8)}`;
40-40: Test titles: make them declarative.Prefer “should …” phrasing for clarity in reports.
-test('expect create a private channel encrypted and send an encrypted message', async ({ page }) => { +test('should create an encrypted private channel and send an encrypted message', async ({ page }) => { @@ -test('expect create a private channel, encrypt it and send an encrypted message', async ({ page }) => { +test('should create a private channel, enable encryption, and send an encrypted message', async ({ page }) => {Also applies to: 85-85
51-54: Stronger assertions for encryption icon.Count-based checks reduce false positives from hidden elements.
- await expect(encryptedRoomPage.lastMessage.encryptedIcon).toBeVisible(); + await expect(encryptedRoomPage.lastMessage.encryptedIcon).toHaveCount(1); @@ - await expect(encryptedRoomPage.lastMessage.encryptedIcon).not.toBeVisible(); + await expect(encryptedRoomPage.lastMessage.encryptedIcon).toHaveCount(0); @@ - await expect(encryptedRoomPage.lastMessage.encryptedIcon).toBeVisible(); + await expect(encryptedRoomPage.lastMessage.encryptedIcon).toHaveCount(1); @@ - await expect(encryptedRoomPage.lastMessage.encryptedIcon).toBeVisible(); + await expect(encryptedRoomPage.lastMessage.encryptedIcon).toHaveCount(1);Also applies to: 67-67, 78-82, 110-110
📜 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.
📒 Files selected for processing (5)
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/home-content.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts
- apps/meteor/tests/e2e/page-objects/fragments/home-content.ts
🧰 Additional context used
📓 Path-based instructions (3)
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-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts
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/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.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/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.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
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/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Group related tests in the same file
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Maintain test isolation between test cases
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : All Playwright test files must be located under apps/meteor/tests/e2e/ and use the .spec.ts extension (e.g., login.spec.ts)
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Write concise, technical TypeScript/JavaScript with accurate typing
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Ensure a clean state for each test execution
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use test.step() to organize complex test scenarios
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use test.beforeAll() and test.afterAll() for setup and teardown
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
🧬 Code graph analysis (3)
apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.ts (2)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (6)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts (1)
PinnedMessagesTab(5-23)apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts (1)
StarredMessagesTab(5-23)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (1)
E2EEMessageActions(156-186)apps/meteor/tests/e2e/page-objects/encrypted-room.ts (1)
EncryptedRoomPage(5-47)
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts (4)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)apps/meteor/tests/e2e/page-objects/encrypted-room.ts (1)
EncryptedRoomPage(5-47)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (2)
EnableRoomEncryptionModal(118-135)DisableRoomEncryptionModal(137-154)
⏰ 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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (9)
apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.ts (2)
12-15: Good suite scaffolding and page-object usage.Clean describe-scoped setup, page-object construction, and state preservation align with our Playwright rules.
18-21: E2E setting is preserved and enabled — LGTM.Preserving and then forcing E2E_Enable in beforeAll is correct.
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (3)
11-14: Good: settings preservation is in placeUsing preserveSettings for E2E_Enable is correct and will restore server state post‑suite.
41-66: LGTM: core flow and assertionsSolid use of POM, web‑first assertions, and E2EE action checks (hover, menu open, disabled states).
15-15: Serialize this suite to avoid global E2E_Enable races across specsOther suites also toggle /settings/E2E_Enable; running this file in a single worker reduces cross‑suite flake. Add serial mode to this describe.
test.describe('E2EE Message Actions', () => { + test.describe.configure({ mode: 'serial' });apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts (4)
10-13: Settings preservation looks good.Good use of preserveSettings for E2E_Enable and E2E_Allow_Unencrypted_Messages.
88-97: Label/URL mismatch: verify this creates a /group (private) room.openNewByLabel('Channel') typically creates a public channel (/channel). You assert /group. If the UI label is “Private”, switch to that.
- await poHomeChannel.sidenav.openNewByLabel('Channel'); + await poHomeChannel.sidenav.openNewByLabel('Private');Also applies to: 94-95
28-34: Nice setup flow.Instantiating page objects in beforeEach and navigating to /home aligns with POM and isolation guidelines.
21-26: Global settings mutation verified — preserved and single-workerpreserveSettings(settingsList) is used in apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts (saves via test.beforeAll and restores via test.afterAll), and the Playwright configs set workers: 1 for the e2e suites, so no cross-worker interference detected.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (1)
37-39: Harden cleanup to avoid short‑circuit on first failureIf one deletion fails, the rest won’t run. Use allSettled so cleanup proceeds for all created rooms.
- await Promise.all(createdChannels.map((channelName) => api.post('/groups.delete', { roomName: channelName }))); + await Promise.allSettled(createdChannels.map((channelName) => api.post('/groups.delete', { roomName: channelName })));apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts (1)
30-32: Assert menu visibility after openingAfter clicking “More”, assert the menu is visible within this dialog to reduce flakiness.
async openLastMessageMenu(): Promise<void> { - await this.lastMessage.openMenu(); + await this.lastMessage.openMenu(); + await expect(this.root.getByRole('menu')).toBeVisible(); }apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts (1)
30-32: Assert menu visibility after openingMirror the starred tab: verify the menu is visible inside this dialog after opening.
async openLastMessageMenu(): Promise<void> { - await this.lastMessage.openMenu(); + await this.lastMessage.openMenu(); + await expect(this.root.getByRole('menu')).toBeVisible(); }
📜 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.
📒 Files selected for processing (4)
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/home-flextab.ts(0 hunks)apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/meteor/tests/e2e/page-objects/fragments/home-flextab.ts
🧰 Additional context used
📓 Path-based instructions (4)
apps/meteor/tests/e2e/page-objects/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Files:
apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts
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/page-objects/fragments/starred-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.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/page-objects/fragments/starred-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.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-message-actions.spec.ts
🧠 Learnings (15)
📓 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
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/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
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/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
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/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
📚 Learning: 2025-09-16T22:08:51.490Z
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/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Group related tests in the same file
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Maintain test isolation between test cases
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Ensure a clean state for each test execution
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Write concise, technical TypeScript/JavaScript with accurate typing
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use test.beforeAll() and test.afterAll() for setup and teardown
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use test.step() to organize complex test scenarios
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
🧬 Code graph analysis (3)
apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts (1)
apps/meteor/tests/e2e/page-objects/fragments/message.ts (1)
Message(3-27)
apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts (1)
apps/meteor/tests/e2e/page-objects/fragments/message.ts (1)
Message(3-27)
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (6)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts (1)
PinnedMessagesTab(6-33)apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts (1)
StarredMessagesTab(6-33)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (1)
E2EEMessageActions(156-186)apps/meteor/tests/e2e/page-objects/encrypted-room.ts (1)
EncryptedRoomPage(5-47)
⏰ 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). (7)
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 🔨 Test Storybook / Test Storybook
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 📦 Meteor Build - coverage
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (5)
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (3)
96-104: Good fix: scope message menu actions to the Pinned dialogUsing
pinnedMessagesTab.openLastMessageMenu()avoids leaking to the main timeline menu.
105-111: Good fix: scope message menu actions to the Starred dialogUsing
starredMessagesTab.openLastMessageMenu()correctly targets the modal’s menu.
15-16: Serialize this suite to avoid global E2E_Enable setting racesThis suite toggles a server‑wide setting. Run the describe serially to prevent flakes when other specs also touch E2E_Enable.
Apply this diff:
test.describe('E2EE Message Actions', () => { + test.describe.configure({ mode: 'serial' });Run to confirm other specs also modify this setting and whether fullyParallel is enabled:
#!/bin/bash # 1) Check for global parallelization rg -nP 'fullyParallel\s*:\s*true' -C2 --glob 'playwright*.{ts,js,cjs,mjs}' # 2) Find other places toggling E2E_Enable rg -nP '/settings/E2E_Enable' -C3 apps/meteor/tests/e2eapps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts (1)
6-33: LGTM overallEncapsulation and locator strategy look consistent with the POM and test utilities.
apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts (1)
6-33: LGTM overallClear, minimal API; consistent with Message fragment and test utils.
dougfabris
force-pushed
the
test/imp-e2e-encrypted-channels
branch
from
739b703 to
e625a18
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts (1)
30-38: Add explicit waits around the menu to reduce flakiness.Wait for the “Options” button and menu to be visible before interacting. This aligns with our wait‑strategy guideline and hardens tests against timing issues.
Apply this diff within the methods:
async openPinnedMessagesList(): Promise<void> { - await this.btnToolbarPrimaryActionsMore.click(); - await this.optionPinnedMessages.click(); + await expect(this.btnToolbarPrimaryActionsMore).toBeVisible(); + await this.btnToolbarPrimaryActionsMore.click(); + await expect(this.menuToolbarPrimaryActionsMore).toBeVisible(); + await this.optionPinnedMessages.click(); } async openStarredMessagesList(): Promise<void> { - await this.btnToolbarPrimaryActionsMore.click(); - await this.optionStarredMessages.click(); + await expect(this.btnToolbarPrimaryActionsMore).toBeVisible(); + await this.btnToolbarPrimaryActionsMore.click(); + await expect(this.menuToolbarPrimaryActionsMore).toBeVisible(); + await this.optionStarredMessages.click(); }Since this introduces
expect, add the import at the top of the file:import type { Locator, Page } from '@playwright/test'; +import { expect } from '../../utils/test';apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts (1)
26-28: Assert against the message body for precision.Scope the text assertion to the message body to avoid false positives from metadata.
- await expect(this.lastMessage.root).toContainText(text); + await expect(this.lastMessage.body).toContainText(text);
📜 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.
📒 Files selected for processing (5)
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/home-flextab.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
- apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts
- apps/meteor/tests/e2e/page-objects/fragments/home-flextab.ts
🧰 Additional context used
📓 Path-based instructions (3)
apps/meteor/tests/e2e/page-objects/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Files:
apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts
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/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/room-toolbar.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/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts
🧠 Learnings (7)
📓 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
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/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
📚 Learning: 2025-09-16T22:08:51.490Z
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/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts
🧬 Code graph analysis (1)
apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts (2)
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts (1)
RoomToolbar(3-39)apps/meteor/tests/e2e/page-objects/fragments/message.ts (1)
Message(3-27)
⏰ 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). (5)
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 📦 Meteor Build - coverage
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (1)
apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts (1)
21-24: LGTM: clear open-and-wait pattern.Opening via the toolbar and asserting the dialog visibility matches our POM and wait guidelines.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts (1)
30-38: Add an explicit wait for the Options menu before clicking items (reduce flakes).Wait for the menu to appear after clicking "Options" to avoid rare timing issues.
async openPinnedMessagesList(): Promise<void> { await this.btnToolbarPrimaryActionsMore.click(); + await this.menuToolbarPrimaryActionsMore.waitFor(); await this.optionPinnedMessages.click(); } async openStarredMessagesList(): Promise<void> { await this.btnToolbarPrimaryActionsMore.click(); + await this.menuToolbarPrimaryActionsMore.waitFor(); await this.optionStarredMessages.click(); }apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts (1)
30-32: Assert the dialog-scoped action menu is visible after opening.Small stabilization to ensure assertions target the correct (modal) menu.
async openLastMessageMenu(): Promise<void> { await this.lastMessage.openMenu(); + await expect(this.root.getByRole('menu')).toBeVisible(); }apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (1)
96-103: Remove redundant kebab clicks before opening Pinned/Starred tabs.RoomToolbar.openTab handles opening the Options menu; the extra click is unnecessary and can interfere.
@@ - await poHomeChannel.tabs.kebab.click(); await pinnedMessagesTab.openTab(); @@ - await poHomeChannel.tabs.kebab.click(); await starredMessagesTab.openTab();Also applies to: 105-112
📜 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.
📒 Files selected for processing (5)
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/home-flextab.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts
- apps/meteor/tests/e2e/page-objects/fragments/home-flextab.ts
🧰 Additional context used
📓 Path-based instructions (4)
apps/meteor/tests/e2e/page-objects/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts
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/page-objects/fragments/room-toolbar.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.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/page-objects/fragments/room-toolbar.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.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-message-actions.spec.ts
🧠 Learnings (16)
📓 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
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/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
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/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
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/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
📚 Learning: 2025-09-16T22:08:51.490Z
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/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.tsapps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Group related tests in the same file
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Maintain test isolation between test cases
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Write concise, technical TypeScript/JavaScript with accurate typing
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Ensure a clean state for each test execution
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : All Playwright test files must be located under apps/meteor/tests/e2e/ and use the .spec.ts extension (e.g., login.spec.ts)
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use test.beforeAll() and test.afterAll() for setup and teardown
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use test.step() to organize complex test scenarios
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
🧬 Code graph analysis (2)
apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts (2)
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts (1)
RoomToolbar(3-39)apps/meteor/tests/e2e/page-objects/fragments/message.ts (1)
Message(3-27)
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (6)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts (1)
PinnedMessagesTab(7-33)apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts (1)
StarredMessagesTab(7-33)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (1)
E2EEMessageActions(156-186)apps/meteor/tests/e2e/page-objects/encrypted-room.ts (1)
EncryptedRoomPage(5-47)
⏰ 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: 🔎 Code Check / Code Lint
- GitHub Check: 📦 Meteor Build - coverage
🔇 Additional comments (1)
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (1)
15-15: Serialize this suite to avoid global E2E_Enable setting races.Other specs toggle the same setting; run this describe serially.
test.describe('E2EE Message Actions', () => { + test.describe.configure({ mode: 'serial' });
dougfabris
force-pushed
the
test/imp-e2e-encrypted-channels
branch
from
e625a18 to
a03af2e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts (1)
30-38: Add an explicit wait for the menu popover before clicking items.Improves stability on slower UIs.
async openPinnedMessagesList(): Promise<void> { await this.btnToolbarPrimaryActionsMore.click(); + await this.page.getByRole('menu').first().waitFor(); await this.optionPinnedMessages.click(); } async openStarredMessagesList(): Promise<void> { await this.btnToolbarPrimaryActionsMore.click(); + await this.page.getByRole('menu').first().waitFor(); await this.optionStarredMessages.click(); }apps/meteor/tests/e2e/page-objects/fragments/home-flextab.ts (1)
59-62: Replace localized “Options” locator with stable data-qa-id.Removes i18n brittleness and aligns with RoomToolbar usage.
- // TODO: replace this with the room-toolbar get kebab(): Locator { - return this.toolbarPrimaryActions.locator('role=button[name="Options"]'); + return this.page.locator('[data-qa-id="ToolBox-Menu"]'); }
📜 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.
📒 Files selected for processing (5)
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/home-flextab.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts
- apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts
🧰 Additional context used
📓 Path-based instructions (4)
apps/meteor/tests/e2e/page-objects/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab.ts
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/page-objects/fragments/room-toolbar.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.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/page-objects/fragments/room-toolbar.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.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-message-actions.spec.ts
🧠 Learnings (20)
📓 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
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/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
📚 Learning: 2025-09-16T22:08:51.490Z
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/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-23T19:22:59.217Z
Learnt from: dougfabris
PR: RocketChat/Rocket.Chat#36987
File: apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts:10-20
Timestamp: 2025-09-23T19:22:59.217Z
Learning: In Playwright e2e tests, prefer stable selectors like data-qa-id attributes over localized text in getByRole() or getByText() calls to prevent test failures when UI language changes. Test translations separately by validating actual text content after ensuring UI interactions work with stable selectors.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Implement proper wait strategies for dynamic content
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Write concise, technical TypeScript/JavaScript with accurate typing
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Avoid code comments in the implementation
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Group related tests in the same file
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Maintain test isolation between test cases
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Ensure a clean state for each test execution
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : All Playwright test files must be located under apps/meteor/tests/e2e/ and use the .spec.ts extension (e.g., login.spec.ts)
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use test.beforeAll() and test.afterAll() for setup and teardown
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use test.step() to organize complex test scenarios
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
⏰ 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). (6)
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 📦 Meteor Build - coverage
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (3)
apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts (1)
10-28: Use stable data-qa-id selectors instead of localized names.Replace role+name locators with data-qa-id to avoid i18n flakes.
- private get toolbarPrimaryActions(): Locator { - return this.page.getByRole('toolbar', { name: 'Primary Room actions' }); - } - - private get btnToolbarPrimaryActionsMore(): Locator { - return this.toolbarPrimaryActions.getByRole('button', { name: 'Options' }); - } - - private get menuToolbarPrimaryActionsMore(): Locator { - return this.page.getByRole('menu', { name: 'Options' }); - } + private get btnToolbarPrimaryActionsMore(): Locator { + return this.page.locator('[data-qa-id="ToolBox-Menu"]'); + } private get optionPinnedMessages(): Locator { - return this.menuToolbarPrimaryActionsMore.getByRole('menuitem', { name: 'Pinned Messages' }); + return this.page.locator('[data-qa-id="ToolBoxAction-pin"]'); } private get optionStarredMessages(): Locator { - return this.menuToolbarPrimaryActionsMore.getByRole('menuitem', { name: 'Starred Messages' }); + return this.page.locator('[data-qa-id="ToolBoxAction-star"]'); }apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (2)
96-109: Good fix: scope message menu actions to the Pinned/Starred dialogs.Switched to
pinnedMessagesTab.openLastMessageMenu()andstarredMessagesTab.openLastMessageMenu(); resolves scoping flakiness.
15-26: Serialize this suite to avoid global E2E_Enable races.Other specs toggle the same setting; run serial to prevent flakes.
-test.describe('E2EE Message Actions', () => { +test.describe('E2EE Message Actions', () => { + test.describe.configure({ mode: 'serial' });
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts (1)
51-67: Run this suite serially to prevent global E2E setting races.This describe flips the
/settings/E2E_*flags inbeforeAlland restores them inafterAll. When Playwright runs multiple specs in parallel (default), another encryption spec can still be executing while this one resets the settings, producing hard-to-repro flakes. Please serialize the block (or otherwise prevent concurrent mutation) so the settings stay stable for the duration of each suite.test.describe('E2EE Legacy Format', () => { + test.describe.configure({ mode: 'serial' });apps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.ts (1)
22-36: Serialize the suite to avoid clobbering shared E2E settings.This file also toggles the global
/settings/E2E_*values in its hooks. Running concurrently with the other encryption specs means the first suite to finish will revert the settings while the others are still mid-test, leading to intermittent failures. Please mark the describe as serial (or otherwise guard the shared settings) so the values remain stable while the suite runs.test.describe('E2EE PDF Export', () => { + test.describe.configure({ mode: 'serial' });apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (2)
206-213: Remove 'undefined' fallback and assert saved passphrase presenceFallbacking to the string 'undefined' will make the flow enter an invalid password and cause flakiness. Assert the value exists before using it.
- e2eePassword = (await page.evaluate(() => localStorage.getItem('e2e.randomPassword'))) || 'undefined'; + const savedPwd = await page.evaluate(() => localStorage.getItem('e2e.randomPassword')); + await expect(savedPwd, 'Missing e2e.randomPassword in localStorage').toBeTruthy(); + e2eePassword = savedPwd!;
259-259: Replace fixed sleep with a readiness waitAvoid hardcoded timeout; wait for the encryption readiness signal instead.
- // For E2EE to complete init setup - await page.waitForTimeout(300); + // For E2EE to complete init setup + await expect(encryptedRoomPage.encryptionNotReadyIndicator).not.toBeVisible();
🧹 Nitpick comments (7)
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (3)
304-306: Prefer semantic getByRole over raw locator chains for search resultsUse role-based locators (and/or a page-object helper) for stability.
- await poHomeChannel.sidenav.openSearch(); - await poHomeChannel.sidenav.inputSearch.fill(channelName); - await page.locator(`role=search >> role=listbox >> role=link >> text="${channelName}"`).click(); + await poHomeChannel.sidenav.openSearch(); + await poHomeChannel.sidenav.inputSearch.fill(channelName); + await page.getByRole('search').getByRole('listbox').getByRole('link', { name: channelName, exact: true }).click();
183-183: Prefer web-first/POM waits instead of CSS selector waitsSwitch from waitForSelector to a semantic/POM wait for channel readiness.
- await page.waitForSelector('#main-content'); + await poHomeChannel.sidenav.waitForChannel();
38-38: Optional: deduplicate test.use(storageState) placementYou set storageState both inside the first describe and again at file scope. Hoist a single test.use({ storageState: Users.admin.state }) to the top of the file to cover all suites.
Also applies to: 135-135
apps/meteor/tests/e2e/utils/resolve-room-id.ts (4)
10-12: Wait for Meteor to be ready to avoid flakinessCallers may hit this before the app boots Meteor on window, causing intermittent nulls. Add a readiness check.
Apply this diff:
export async function resolveRoomId(page: Page, type: RoomType, name: string): Promise<string | null> { - return page.evaluate( + await page.waitForFunction(() => typeof (window as any).Meteor?.call === 'function'); + return page.evaluate( async ({ roomType, roomName }: { roomType: RoomType; roomName: string }) => {
21-22: Bind Meteor.call to preserve contextExtracting a method can lose its original this binding. Bind to Meteor (or invoke directly) to avoid “illegal invocation” edge cases.
Apply this diff:
- const meteorCall = meteor.call; + const meteorCall = meteor.call.bind(meteor);
24-33: Return early with direct call (optional)You can simplify by inlining the call without the intermediate variable.
Apply this diff:
- return new Promise<string | null>((resolve) => { - meteorCall('getRoomByTypeAndName', roomType, roomName, (error: unknown, room: { _id?: string } | null) => { + return new Promise<string | null>((resolve) => { + meteor.call('getRoomByTypeAndName', roomType, roomName, (error: unknown, room: { _id?: string } | null) => { if (error || !room?._id) { resolve(null); return; } resolve(room._id); }); });
45-50: Consider adding a direct-message helper for completenessTests sometimes need 'd' (DM) room IDs. Add a small wrapper for parity with channels and private groups.
Outside the shown range, add:
export function resolveDirectRoomId(page: Page, name: string): Promise<string | null> { return resolveRoomId(page, 'd', name); }
📜 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.
📒 Files selected for processing (15)
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts(3 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts(13 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts(4 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts(5 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts(4 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts(13 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.ts(5 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.ts(7 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-sidebar.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts(1 hunks)apps/meteor/tests/e2e/utils/index.ts(1 hunks)apps/meteor/tests/e2e/utils/resolve-room-id.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.ts
- apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts
- apps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts
- apps/meteor/tests/e2e/e2e-encryption/e2ee-sidebar.spec.ts
🧰 Additional context used
📓 Path-based instructions (3)
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-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts
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/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.tsapps/meteor/tests/e2e/utils/index.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/utils/resolve-room-id.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/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.tsapps/meteor/tests/e2e/utils/index.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/utils/resolve-room-id.ts
🧠 Learnings (18)
📓 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
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/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
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/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/utils/resolve-room-id.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.tsapps/meteor/tests/e2e/utils/index.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/utils/resolve-room-id.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Maintain test isolation between test cases
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.tsapps/meteor/tests/e2e/utils/index.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/utils/resolve-room-id.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Ensure a clean state for each test execution
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use test.beforeAll() and test.afterAll() for setup and teardown
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/utils/resolve-room-id.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Group related tests in the same file
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/utils/index.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/utils/resolve-room-id.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.tsapps/meteor/tests/e2e/utils/index.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/utils/resolve-room-id.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use expect matchers (toEqual, toContain, toBeTruthy, toHaveLength, etc.) instead of assert statements
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/utils/resolve-room-id.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.tsapps/meteor/tests/e2e/utils/resolve-room-id.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : All Playwright test files must be located under apps/meteor/tests/e2e/ and use the .spec.ts extension (e.g., login.spec.ts)
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/utils/resolve-room-id.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Write concise, technical TypeScript/JavaScript with accurate typing
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/utils/resolve-room-id.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use test.step() to organize complex test scenarios
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
🧬 Code graph analysis (10)
apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.ts (4)
apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)apps/meteor/tests/e2e/page-objects/encrypted-room.ts (1)
EncryptedRoomPage(5-47)apps/meteor/tests/e2e/utils/create-target-channel.ts (1)
deleteRoom(48-50)apps/meteor/tests/e2e/utils/resolve-room-id.ts (1)
resolvePrivateRoomId(41-43)
apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts (3)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/utils/create-target-channel.ts (1)
deleteRoom(48-50)apps/meteor/tests/e2e/utils/resolve-room-id.ts (1)
resolvePrivateRoomId(41-43)
apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts (1)
apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (2)
DisableRoomEncryptionModal(137-154)EnableRoomEncryptionModal(118-135)
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (9)
apps/meteor/tests/e2e/page-objects/login.ts (1)
LoginPage(9-53)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (4)
EnterE2EEPasswordBanner(26-31)EnterE2EEPasswordModal(70-97)E2EEKeyDecodeFailureBanner(33-41)ResetE2EEPasswordModal(99-116)apps/meteor/tests/e2e/page-objects/fragments/home-sidenav.ts (1)
HomeSidenav(7-254)apps/meteor/tests/e2e/page-objects/account-security.ts (1)
AccountSecurityPage(5-59)apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/encrypted-room.ts (1)
EncryptedRoomPage(5-47)apps/meteor/tests/e2e/utils/create-target-channel.ts (1)
deleteRoom(48-50)apps/meteor/tests/e2e/utils/resolve-room-id.ts (1)
resolvePrivateRoomId(41-43)apps/meteor/tests/e2e/fixtures/userStates.ts (1)
restoreState(142-155)
apps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.ts (2)
apps/meteor/tests/e2e/utils/create-target-channel.ts (1)
deleteRoom(48-50)apps/meteor/tests/e2e/utils/resolve-room-id.ts (1)
resolvePrivateRoomId(41-43)
apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts (1)
apps/meteor/tests/e2e/page-objects/login.ts (1)
LoginPage(9-53)
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (8)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts (1)
PinnedMessagesTab(7-33)apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts (1)
StarredMessagesTab(7-33)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (1)
E2EEMessageActions(156-186)apps/meteor/tests/e2e/page-objects/encrypted-room.ts (1)
EncryptedRoomPage(5-47)apps/meteor/tests/e2e/utils/create-target-channel.ts (1)
deleteRoom(48-50)apps/meteor/tests/e2e/utils/resolve-room-id.ts (1)
resolvePrivateRoomId(41-43)
apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts (3)
apps/meteor/tests/e2e/page-objects/encrypted-room.ts (1)
EncryptedRoomPage(5-47)apps/meteor/tests/e2e/utils/create-target-channel.ts (1)
deleteRoom(48-50)apps/meteor/tests/e2e/utils/resolve-room-id.ts (1)
resolvePrivateRoomId(41-43)
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts (6)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)apps/meteor/tests/e2e/page-objects/encrypted-room.ts (1)
EncryptedRoomPage(5-47)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (2)
EnableRoomEncryptionModal(118-135)DisableRoomEncryptionModal(137-154)apps/meteor/tests/e2e/utils/create-target-channel.ts (1)
deleteRoom(48-50)apps/meteor/tests/e2e/utils/resolve-room-id.ts (1)
resolvePrivateRoomId(41-43)
apps/meteor/tests/e2e/utils/resolve-room-id.ts (1)
apps/meteor/server/publications/room/index.ts (1)
type(53-93)
⏰ 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). (7)
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 🔨 Test Storybook / Test Storybook
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 📦 Meteor Build - coverage
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (5)
apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts (1)
35-53: Page object usage looks great.Switching the login button assertions to the
LoginPagepage object keeps the spec aligned with our POM guidelines and removes duplicated locator strings. Thanks for tightening that up.apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts (1)
49-57: Modal page objects keep the flow resilient.Nice swap to the dedicated enable/disable modal helpers—this encapsulates the toast handling and should make the toggle sequence less flaky.
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (1)
17-41: Serialize this suite to avoid cross-spec E2E setting races.Same issue as before: the suite mutates
/settings/E2E_EnableinbeforeAlland restores it later. While other E2E specs are now running in parallel, whichever one finishes first resets the server state, breaking the rest. Please configure the describe to run serially (or otherwise coordinate the shared setting) so tests remain reliable.test.describe('E2EE Message Actions', () => { + test.describe.configure({ mode: 'serial' });apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts (1)
18-30: Make this describe serial to keep/settings/E2E_*stable.
beforeAllforces the E2E flags on andafterAllreverts them. With the new spec split, Playwright can run these suites in parallel—meaning one suite’s cleanup flips the settings while another still depends on them, causing non-deterministic failures. Please configure the describe to run serially (or otherwise coordinate the shared settings).test.describe('E2EE Encryption and Decryption - Basic Features', () => { + test.describe.configure({ mode: 'serial' });⛔ Skipped due to learnings
Learnt from: cardoso PR: RocketChat/Rocket.Chat#36890 File: apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts:29-29 Timestamp: 2025-09-17T14:11:53.796Z Learning: Using test.describe.serial is not recommended by Playwright documentation. It is usually better to make tests isolated so they can run independently rather than relying on serial execution to avoid race conditions.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 : Use test.beforeAll() and test.afterAll() for setup and teardownLearnt 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 : Ensure tests run reliably in parallel without shared state conflictsLearnt 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 : Maintain test isolation between test casesLearnt 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) consistentlyLearnt 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 : Ensure a clean state for each test executionLearnt 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 : Use test.step() to organize complex test scenariosLearnt 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 : Use descriptive test names that clearly communicate expected behaviorapps/meteor/tests/e2e/utils/index.ts (1)
5-5: Barrel re-export LGTMExposes the new room-resolution utilities via the shared utils index, keeping imports consistent in specs.
| test.describe('E2EE Channel Basic Functionality', () => { | ||
| const createdChannels: { name: string; id?: string | null }[] = []; | ||
| let poHomeChannel: HomeChannel; | ||
| let encryptedRoomPage: EncryptedRoomPage; | ||
| let enableEncryptionModal: EnableRoomEncryptionModal; | ||
| let disableEncryptionModal: DisableRoomEncryptionModal; | ||
|
|
||
| test.use({ storageState: Users.userE2EE.state }); | ||
|
|
||
| test.beforeAll(async ({ api }) => { | ||
| await api.post('/settings/E2E_Enable', { value: true }); | ||
| await api.post('/settings/E2E_Allow_Unencrypted_Messages', { value: true }); | ||
| }); | ||
|
|
||
| test.beforeEach(async ({ page }) => { | ||
| poHomeChannel = new HomeChannel(page); | ||
| enableEncryptionModal = new EnableRoomEncryptionModal(page); | ||
| disableEncryptionModal = new DisableRoomEncryptionModal(page); | ||
| encryptedRoomPage = new EncryptedRoomPage(page); | ||
| await poHomeChannel.goto(); | ||
| }); | ||
|
|
||
| test.afterAll(async ({ api }) => { | ||
| await Promise.all(createdChannels.map(({ id }) => (id ? deleteRoom(api, id) : Promise.resolve()))); | ||
| }); |
There was a problem hiding this comment.
Run this describe serially to protect shared E2E settings.
beforeAll sets /settings/E2E_Enable and friends, afterAll (via preserveSettings) restores them. When multiple specs now execute in parallel, one suite can finish and revert the settings while another is still using them—classic flake territory. Please mark the describe as serial (or otherwise coordinate) so the toggled settings stay consistent for the duration of the suite.
test.describe('E2EE Channel Basic Functionality', () => {
+ test.describe.configure({ mode: 'serial' });🤖 Prompt for AI Agents
In apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts around lines 16 to
40, the test.describe block must be run serially to avoid races that flip shared
E2E settings between suites; change the describe invocation to a serial describe
(e.g., test.describe.serial(...)) so the beforeAll/afterAll setting toggles
remain consistent for the suite duration and prevent parallel suites from
reverting settings mid-run.
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
Outdated
Show resolved
Hide resolved
| await poHomeChannel.sidenav.createEncryptedChannel(channelName); | ||
| const roomId = await resolvePrivateRoomId(page, channelName); | ||
| createdChannels.push({ name: channelName, id: roomId }); | ||
|
|
There was a problem hiding this comment.
Stop silently skipping cleanup when resolvePrivateRoomId fails
If resolvePrivateRoomId happens to return null (API delay, UI flake, etc.), we still push that entry—and our afterAll cleanup simply skips it. That leaks encrypted rooms into subsequent runs and breaks isolation. Guard the lookup result and fail fast so we never carry an undeletable room forward.
- const roomId = await resolvePrivateRoomId(page, channelName);
- createdChannels.push({ name: channelName, id: roomId });
+ const roomId = await resolvePrivateRoomId(page, channelName);
+ if (!roomId) {
+ throw new Error(`Failed to resolve room id for encrypted room ${channelName}`);
+ }
+ createdChannels.push({ name: channelName, id: roomId });Also applies to: 93-96
🤖 Prompt for AI Agents
In apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.ts around
lines 49-52 (and similarly at lines 93-96), the test pushes createdChannels even
when resolvePrivateRoomId returns null which causes afterAll to skip cleanup and
leak rooms; change the flow to guard the lookup result: after calling
resolvePrivateRoomId, if the returned roomId is null/undefined throw a
descriptive error (or explicitly fail the test) instead of pushing to
createdChannels, and only push when roomId is a valid non-null string —
replicate the same guard at the other referenced lines.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (2)
258-260: Replace fixed timeout with a web-first assertion for E2EE readinessHardcoded waits are flaky. Wait for a meaningful UI signal instead.
- // For E2EE to complete init setup - await page.waitForTimeout(300); + // Wait until encryption is ready + await expect(encryptedRoomPage.encryptionNotReadyIndicator).not.toBeVisible();
304-306: Prefer semantic locators or a page‑object method for selecting search resultsAvoid page.locator and chained text engine selectors. Wrap this in a HomeSidenav method using getByRole to align with our POM and locator guidelines.
- await poHomeChannel.sidenav.openSearch(); - await poHomeChannel.sidenav.inputSearch.fill(channelName); - await page.locator(`role=search >> role=listbox >> role=link >> text="${channelName}"`).click(); + await poHomeChannel.sidenav.selectSearchResultByName(channelName);Suggested page-object method (in HomeSidenav):
async selectSearchResultByName(name: string) { await this.openSearch(); await this.inputSearch.fill(name); await this.page.getByRole('listbox').getByRole('link', { name, exact: true }).click(); }
📜 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.
📒 Files selected for processing (1)
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts(13 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
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-passphrase-management.spec.ts
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/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/e2ee-passphrase-management.spec.ts
🧠 Learnings (11)
📓 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
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/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
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/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
📚 Learning: 2025-09-16T22:08:51.490Z
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/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Maintain test isolation between test cases
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Ensure a clean state for each test execution
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use test.beforeAll() and test.afterAll() for setup and teardown
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Group related tests in the same file
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
🧬 Code graph analysis (1)
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (8)
apps/meteor/tests/e2e/page-objects/login.ts (1)
LoginPage(9-53)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (4)
EnterE2EEPasswordBanner(26-31)EnterE2EEPasswordModal(70-97)E2EEKeyDecodeFailureBanner(33-41)ResetE2EEPasswordModal(99-116)apps/meteor/tests/e2e/page-objects/account-security.ts (1)
AccountSecurityPage(5-59)apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)apps/meteor/tests/e2e/page-objects/encrypted-room.ts (1)
EncryptedRoomPage(5-47)apps/meteor/tests/e2e/utils/create-target-channel.ts (1)
deleteRoom(48-50)apps/meteor/tests/e2e/utils/resolve-room-id.ts (1)
resolvePrivateRoomId(41-43)
⏰ 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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (2)
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (2)
140-152: Assert roomId before tracking; prevent leaked channels; DRY channel creationresolvePrivateRoomId returns string | null. Pushing a null id skips cleanup and can leak rooms. Assert the id is truthy and store a non-null type; optionally wrap in a small helper to DRY.
Apply these diffs:
- const createdChannels: { name: string; id?: string | null }[] = []; + const createdChannels: { name: string; id: string }[] = [];- await Promise.all(createdChannels.map(({ id }) => (id ? deleteRoom(api, id) : Promise.resolve()))); + await Promise.all(createdChannels.map(({ id }) => deleteRoom(api, id)));- await poHomeChannel.sidenav.createEncryptedChannel(channelName); - const roomId = await resolvePrivateRoomId(page, channelName); - createdChannels.push({ name: channelName, id: roomId }); + await poHomeChannel.sidenav.createEncryptedChannel(channelName); + const roomId = await resolvePrivateRoomId(page, channelName); + await expect(roomId, `Failed to resolve roomId for ${channelName}`).toBeTruthy(); + createdChannels.push({ name: channelName, id: roomId! });- await poHomeChannel.sidenav.createEncryptedChannel(channelName); - const roomId = await resolvePrivateRoomId(page, channelName); - createdChannels.push({ name: channelName, id: roomId }); + await poHomeChannel.sidenav.createEncryptedChannel(channelName); + const roomId = await resolvePrivateRoomId(page, channelName); + await expect(roomId, `Failed to resolve roomId for ${channelName}`).toBeTruthy(); + createdChannels.push({ name: channelName, id: roomId! });- await poHomeChannel.sidenav.createEncryptedChannel(channelName); - const roomId = await resolvePrivateRoomId(page, channelName); - createdChannels.push({ name: channelName, id: roomId }); + await poHomeChannel.sidenav.createEncryptedChannel(channelName); + const roomId = await resolvePrivateRoomId(page, channelName); + await expect(roomId, `Failed to resolve roomId for ${channelName}`).toBeTruthy(); + createdChannels.push({ name: channelName, id: roomId! });Optional helper to reduce duplication (place inside this describe):
async function createEncryptedChannelAndTrack(page: Page, name: string) { await poHomeChannel.sidenav.createEncryptedChannel(name); const id = await resolvePrivateRoomId(page, name); await expect(id, `Failed to resolve roomId for ${name}`).toBeTruthy(); createdChannels.push({ name, id: id! }); }Then replace the three blocks with: await createEncryptedChannelAndTrack(page, channelName);
Also applies to: 188-191, 232-235, 275-277
253-254: Break cross-test dependency on e2eePassword; read from localStorage within this testAvoid relying on a value set by a previous test, even in serial mode. Read the saved passphrase locally.
- await enterE2EEPasswordModal.enterPassword(e2eePassword); + const savedPwd = await page.evaluate(() => localStorage.getItem('e2e.randomPassword')); + await expect(savedPwd, 'Missing e2e.randomPassword in localStorage').toBeTruthy(); + await enterE2EEPasswordModal.enterPassword(savedPwd!);
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (1)
178-181: DB reset hazards: isolate this suiteinjectInitialData() re-seeds the DB and can interfere with parallel suites. Keep this describe in a dedicated, single-worker project or mark it to run in isolation.
Based on learnings
🧹 Nitpick comments (20)
apps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.ts (4)
34-36: Harden cleanup to avoid suite failures and dangling roomsIf a deletion fails (network flake, permissions), the entire suite could fail, and if id is missing, a room may be left behind. Wrap deletes in try/catch to avoid masking test results, and attempt a best-effort delete only when id exists.
Apply this diff:
-test.afterAll(async ({ api }) => { - await Promise.all(createdChannels.map(({ id }) => (id ? deleteRoom(api, id) : Promise.resolve()))); - }); +test.afterAll(async ({ api }) => { + await Promise.all( + createdChannels.map(async ({ id }) => { + if (!id) return; + try { + await deleteRoom(api, id); + } catch { + // swallow cleanup errors to not fail the suite + } + }), + ); +});If you want extra robustness, we can add an API-based fallback to resolve roomId by name before delete (requires an API endpoint like /rooms.info). As per coding guidelines
57-58: Use test.step for better traceabilityWrap creation in a step to improve trace logs and debugging.
- await createE2EEChannel.createAndStore(channelName, createdChannels); + await test.step('create encrypted channel', async () => { + await createE2EEChannel.createAndStore(channelName, createdChannels); + });As per coding guidelines
62-64: Prefer web-first disabled assertions over class checksClass-based checks are brittle. If possible, assert disabled state via toBeDisabled() or aria attributes. Also, avoid inline FIXME comments in specs; track via issue/annotation.
Example:
- Replace toContainClass('disabled') with toBeDisabled(), or:
- await expect(exportMessagesTab.method).toHaveAttribute('aria-disabled', /^(true|disabled)$/);
As per coding guidelines
72-73: Add a tracing step around channel creationSame as above, add a test.step for the channel creation to aid trace readability.
- await createE2EEChannel.createAndStore(channelName, createdChannels); + await test.step('create encrypted channel', async () => { + await createE2EEChannel.createAndStore(channelName, createdChannels); + });As per coding guidelines
apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (1)
165-167: Use semantic locator for forward message button.The forward message button uses
getByRolewhich is good, but the text is very long. Consider using a more stable selector if available.As per coding guidelines
apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts (1)
36-38: Avoid duplicate afterAll hooks.There are two
test.afterAllhooks (Lines 36-38 and 40-43). While Playwright allows multiple hooks, it's cleaner to combine related cleanup logic.Combine the afterAll hooks:
- test.afterAll(async ({ api }) => { - await Promise.all(createdChannels.map(({ id }) => (id ? deleteRoom(api, id) : Promise.resolve()))); - }); - - test.afterAll(async ({ api }) => { + test.afterAll(async ({ api }) => { + await Promise.all(createdChannels.map(({ id }) => (id ? deleteRoom(api, id) : Promise.resolve()))); await api.post('/settings/FileUpload_MediaTypeWhiteList', { value: '' }); await api.post('/settings/FileUpload_MediaTypeBlackList', { value: 'image/svg+xml' }); });As per coding guidelines
apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts (1)
83-85: Remove unnecessary wait and blank line.The
waitForHome()call on Line 83 appears redundant sincepage.goto('/home')is already called inbeforeEach(Line 74). The blank line on Line 84 also adds unnecessary spacing.- await poHomeChannel.sidenav.waitForHome(); - await createE2EEChannel.createAndStore(channelName, createdChannels);As per coding guidelines
apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts (2)
79-81: Make placeholder text assertions resilient to copy changesAvoid asserting the whole paragraph; check a stable substring to reduce flakes from minor wording changes.
Apply this diff:
- await expect(encryptedRoomPage.lastMessage.body).toHaveText( - 'This message is end-to-end encrypted. To view it, you must enter your encryption key in your account settings.', - ); + await expect(encryptedRoomPage.lastMessage.body).toContainText('This message is end-to-end encrypted.'); @@ - await expect(encryptedRoomPage.lastNthMessage(1).body).toHaveText( - 'This message is end-to-end encrypted. To view it, you must enter your encryption key in your account settings.', - ); + await expect(encryptedRoomPage.lastNthMessage(1).body).toContainText('This message is end-to-end encrypted.');As per coding guidelines
Also applies to: 148-155
16-27: Guard against inter-suite setting races
E2EE tests toggle global/settings/E2E_Enableand/settings/E2E_Allow_Unencrypted_Messagesacross multiple suites, risking flakes under parallel execution. Isolate them by:
- using
test.describe.serialfor E2EE suites- assigning E2EE tests to a dedicated Playwright project/worker
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (1)
60-67: Reduce brittleness in “disabled” checksThese depend on specific menu item labels. Consider asserting by role with partial name match or data-qa (if available) to make them less copy-sensitive, keeping the disabled assertion.
As per coding guidelines
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (2)
256-258: Avoid arbitrary timeouts; wait for a concrete conditionReplace waitForTimeout(300) with a web-first assertion that the encryption init completed.
Apply this diff:
- // For E2EE to complete init setup - await page.waitForTimeout(300); + await expect(encryptedRoomPage.encryptionNotReadyIndicator).not.toBeVisible();As per coding guidelines
287-287: Prefer POM over raw navigation locatorsReplace the direct “Security” link click with a page-object method (e.g., AccountSecurityPage.goto or AccountProfile.openSecurityTab) to keep tests DRY and robust to UI changes.
As per coding guidelines
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts (1)
61-66: Avoid force clicks; handle overlays deterministicallyForce clicks hide timing issues (e.g., toasts). Prefer dismissing toasts or waiting for the kebab to be actionable, then click without force.
Apply this diff:
- await poHomeChannel.tabs.kebab.click({ force: true }); + await expect(poHomeChannel.tabs.kebab).toBeVisible(); + await poHomeChannel.tabs.kebab.click(); @@ - await poHomeChannel.tabs.kebab.click({ force: true }); + await expect(poHomeChannel.tabs.kebab).toBeVisible(); + await poHomeChannel.tabs.kebab.click();As per coding guidelines
Also applies to: 75-80
apps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts (7)
37-37: Rename test title for clarityMake the title descriptive and action-oriented.
-test('expect create a private encrypted channel and send an encrypted thread message', async ({ page }) => { +test('creates a private encrypted channel and sends encrypted thread messages (also to channel)', async ({ page }) => {As per coding guidelines
41-44: Make URL assertion resilient to query strings/trailing changesAvoid brittle exact-match.
- await expect(page).toHaveURL(`/group/${channelName}`); + await expect(page).toHaveURL(new RegExp(`/group/${channelName}(?:\\?.*)?$`));As per coding guidelines
46-50: Assert encryption readiness before sending the first messagePrevents false positives if encryption isn’t ready yet.
await test.step('send main thread message', async () => { + await expect(encryptedRoomPage.encryptionNotReadyIndicator).not.toBeVisible(); await poHomeChannel.content.sendMessage('This is the thread main message.'); await expect(poHomeChannel.content.lastUserMessageBody).toHaveText('This is the thread main message.'); await expect(encryptedRoomPage.lastMessage.encryptedIcon).toBeVisible(); });As per coding guidelines
67-73: Stronger verification: assert encrypted icon on both thread messages explicitlyCheck both the reply and the main thread message icons to reduce ambiguity.
await test.step('verify all thread messages are encrypted', async () => { await expect(poHomeChannel.content.lastThreadMessageText).toContainText('This is an encrypted thread message also sent in channel'); await expect(encryptedRoomPage.lastMessage.encryptedIcon).toBeVisible(); + await expect(encryptedRoomPage.lastNthMessage(1).encryptedIcon).toBeVisible(); // main thread message await expect(poHomeChannel.content.lastUserMessage).toContainText('This is an encrypted thread message also sent in channel'); await expect(poHomeChannel.content.mainThreadMessageText).toContainText('This is the thread main message.'); await expect(encryptedRoomPage.lastMessage.encryptedIcon).toBeVisible(); });As per coding guidelines
33-35: Harden cleanup against already-deleted roomsAvoid failing the suite on benign cleanup errors.
- await Promise.all(createdChannels.map(({ id }) => (id ? deleteRoom(api, id) : Promise.resolve()))); + await Promise.all(createdChannels.map(({ id }) => (id ? deleteRoom(api, id).catch(() => {}) : Promise.resolve())));As per coding guidelines
38-38: Add a meaningful prefix to the room name for easier debuggingHelps identify which spec created the room when inspecting logs.
- const channelName = faker.string.uuid(); + const channelName = `e2ee-threads-${faker.string.uuid()}`;As per coding guidelines
11-25: Centralize E2E_Enable toggling to avoid races in parallel testsMultiple specs independently POST
/settings/E2E_Enable, flapping the global flag. Move setup into a shared fixture (and leveragepreserveSettings) so it’s applied once per suite and restored reliably.
📜 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.
📒 Files selected for processing (11)
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts(4 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts(12 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts(5 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts(12 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.ts(5 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.ts(6 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/tests/e2e/e2e-encryption/e2ee-mentions.spec.ts
🧰 Additional context used
📓 Path-based instructions (4)
apps/meteor/tests/e2e/page-objects/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Files:
apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts
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/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.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/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.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-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts
🧠 Learnings (20)
📓 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
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/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
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/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
📚 Learning: 2025-09-16T22:08:51.490Z
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/page-objects/**/*.ts : Utilize and place Page Object implementations under apps/meteor/tests/e2e/page-objects/
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Follow the Page Object Model pattern consistently
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Write concise, technical TypeScript/JavaScript with accurate typing
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/e2ee.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx} : Use page.waitFor() with specific conditions and avoid hardcoded timeouts
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Maintain test isolation between test cases
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : All Playwright test files must be located under apps/meteor/tests/e2e/ and use the .spec.ts extension (e.g., login.spec.ts)
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Group related tests in the same file
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Ensure a clean state for each test execution
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts
📚 Learning: 2025-09-17T14:11:53.796Z
Learnt from: cardoso
PR: RocketChat/Rocket.Chat#36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts:29-29
Timestamp: 2025-09-17T14:11:53.796Z
Learning: Using test.describe.serial is not recommended by Playwright documentation. It is usually better to make tests isolated so they can run independently rather than relying on serial execution to avoid race conditions.
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use test.beforeAll() and test.afterAll() for setup and teardown
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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 : Use test.step() to organize complex test scenarios
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
📚 Learning: 2025-09-16T13:33:49.237Z
Learnt from: cardoso
PR: RocketChat/Rocket.Chat#36890
File: apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts:21-26
Timestamp: 2025-09-16T13:33:49.237Z
Learning: In Rocket.Chat test files, the im.delete API endpoint accepts either a `roomId` parameter (requiring the actual DM room _id) or a `username` parameter (for the DM partner's username). It does not accept slug-like constructions such as concatenating usernames together.
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
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/**/*.{ts,tsx,js,jsx} : Avoid code comments in the implementation
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts
🧬 Code graph analysis (10)
apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (2)
apps/meteor/tests/e2e/page-objects/fragments/home-sidenav.ts (1)
HomeSidenav(7-254)apps/meteor/tests/e2e/utils/resolve-room-id.ts (1)
resolvePrivateRoomId(41-43)
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts (6)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)apps/meteor/tests/e2e/page-objects/encrypted-room.ts (1)
EncryptedRoomPage(5-47)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (3)
EnableRoomEncryptionModal(120-137)DisableRoomEncryptionModal(139-156)CreateE2EEChannel(190-216)apps/meteor/tests/e2e/utils/create-target-channel.ts (1)
deleteRoom(48-50)apps/meteor/tests/e2e/utils/resolve-room-id.ts (1)
resolvePrivateRoomId(41-43)
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (8)
apps/meteor/tests/e2e/page-objects/login.ts (1)
LoginPage(9-53)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (5)
EnterE2EEPasswordBanner(28-33)EnterE2EEPasswordModal(72-99)E2EEKeyDecodeFailureBanner(35-43)ResetE2EEPasswordModal(101-118)CreateE2EEChannel(190-216)apps/meteor/tests/e2e/page-objects/fragments/home-sidenav.ts (1)
HomeSidenav(7-254)apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/encrypted-room.ts (1)
EncryptedRoomPage(5-47)apps/meteor/tests/e2e/utils/create-target-channel.ts (1)
deleteRoom(48-50)apps/meteor/tests/e2e/fixtures/inject-initial-data.ts (1)
injectInitialData(8-84)apps/meteor/tests/e2e/fixtures/userStates.ts (1)
restoreState(142-155)
apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts (4)
apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)apps/meteor/tests/e2e/page-objects/encrypted-room.ts (1)
EncryptedRoomPage(5-47)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (1)
CreateE2EEChannel(190-216)apps/meteor/tests/e2e/utils/create-target-channel.ts (1)
deleteRoom(48-50)
apps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.ts (2)
apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (1)
CreateE2EEChannel(190-216)apps/meteor/tests/e2e/utils/create-target-channel.ts (1)
deleteRoom(48-50)
apps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts (5)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)apps/meteor/tests/e2e/page-objects/encrypted-room.ts (1)
EncryptedRoomPage(5-47)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (1)
CreateE2EEChannel(190-216)apps/meteor/tests/e2e/utils/create-target-channel.ts (1)
deleteRoom(48-50)
apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts (3)
apps/meteor/tests/e2e/page-objects/encrypted-room.ts (1)
EncryptedRoomPage(5-47)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (1)
CreateE2EEChannel(190-216)apps/meteor/tests/e2e/utils/create-target-channel.ts (1)
deleteRoom(48-50)
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (7)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)apps/meteor/tests/e2e/page-objects/fragments/pinned-messages-tab.ts (1)
PinnedMessagesTab(7-33)apps/meteor/tests/e2e/page-objects/fragments/starred-messages-tab.ts (1)
StarredMessagesTab(7-33)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (2)
E2EEMessageActions(158-188)CreateE2EEChannel(190-216)apps/meteor/tests/e2e/page-objects/encrypted-room.ts (1)
EncryptedRoomPage(5-47)apps/meteor/tests/e2e/utils/create-target-channel.ts (1)
deleteRoom(48-50)
apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.ts (4)
apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)apps/meteor/tests/e2e/page-objects/encrypted-room.ts (1)
EncryptedRoomPage(5-47)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (1)
CreateE2EEChannel(190-216)apps/meteor/tests/e2e/utils/create-target-channel.ts (1)
deleteRoom(48-50)
apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts (2)
apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (1)
CreateE2EEChannel(190-216)apps/meteor/tests/e2e/utils/create-target-channel.ts (1)
deleteRoom(48-50)
⏰ 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). (3)
- GitHub Check: Builds matrix rust bindings against alpine
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (16)
apps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.ts (4)
5-5: Good POM usage for channel creationSwitching to CreateE2EEChannel aligns with the Page Object Model and keeps specs clean.
8-8: Centralized cleanup helper looks goodUsing deleteRoom via the shared util is consistent and keeps teardown simple.
22-24: Suite-scoped tracking of created rooms is appropriateLocalizing createdChannels to the describe scope avoids cross-file interference and supports reliable cleanup.
40-40: Instantiate page object per testCreating CreateE2EEChannel in beforeEach prevents cross-test state. LGTM.
apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.ts (2)
51-51: Consistent with past review: validate room ID before storing.As flagged in previous reviews, if
resolvePrivateRoomIdreturns null, we still push tocreatedChannelscausing cleanup to silently skip. This breaks test isolation.The
createAndStoremethod should throw an error if room resolution fails, as suggested in the past review comments.
93-93: Consistent with past review: validate room ID before storing.Same issue as Line 51 - room resolution failures will cause cleanup to be skipped.
apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts (1)
29-31: Good cleanup strategy with API-backed room deletionsThe afterAll cleanup using ids recorded via CreateE2EEChannel is solid and minimizes test data leakage.
apps/meteor/tests/e2e/e2e-encryption/e2ee-message-actions.spec.ts (2)
16-29: Serialize or isolate to protect shared E2EE settings (already flagged earlier)Global setting toggles can race with other suites; either run this describe serially or isolate via a dedicated project.
Based on learnings
97-104: Nice: menu scoping within Pinned/Starred dialogsUsing the tab-scoped openLastMessageMenu avoids targeting the main timeline accidentally.
Also applies to: 105-111
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (3)
150-152: Good: centralized cleanup of created roomsEnsures no room leakage even on failures.
251-252: Break cross-test dependency on e2eePasswordThis test relies on a value set by a previous test; make it self-contained by reading from localStorage here.
Apply this diff:
- await enterE2EEPasswordModal.enterPassword(e2eePassword); + const savedPwd = await page.evaluate(() => localStorage.getItem('e2e.randomPassword')); + await expect(savedPwd, 'Missing e2e.randomPassword in localStorage').toBeTruthy(); + await enterE2EEPasswordModal.enterPassword(savedPwd!);As per coding guidelines
171-218: Good adoption of CreateE2EEChannel and icon checksUsing CreateE2EEChannel and EncryptedRoomPage.lastMessage.encryptedIcon improves clarity and reuse.
apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts (3)
47-52: Nice encapsulation with CreateE2EEChannelcreateAndStore + encrypted header assertions are clean and readable.
40-42: Cleanup approach looks goodDeleting rooms via API using stored ids is correct.
16-29: Shared E2EE settings can race across suites (previously flagged)Consider serializing this describe or isolating in a single-worker project to avoid interleaving with other specs toggling E2E_Enable.
Based on learnings
apps/meteor/tests/e2e/e2e-encryption/e2ee-threads.spec.ts (1)
15-75: Well-structured E2EE thread spec; solid POM usage and isolationUses fixtures, steps, web-first assertions, and cleans up created rooms. Overall looks good.
As per coding guidelines
| const roomId = await resolvePrivateRoomId(page, channelName); | ||
|
|
||
| createdChannels.push({ name: channelName, id: roomId ?? undefined }); | ||
| await expect(page).toHaveURL(`/group/${channelName}`); | ||
| await expect(poHomeChannel.toastSuccess).toBeVisible(); | ||
| await poHomeChannel.dismissToast(); | ||
| }); |
There was a problem hiding this comment.
Assert roomId resolution to ensure cleanup and fail fast
If resolvePrivateRoomId returns null, you silently skip cleanup. Assert and store a non-null id.
Apply this diff:
- const roomId = await resolvePrivateRoomId(page, channelName);
-
- createdChannels.push({ name: channelName, id: roomId ?? undefined });
+ const roomId = await resolvePrivateRoomId(page, channelName);
+ await expect(roomId, 'Failed to resolve roomId for cleanup').toBeTruthy();
+ createdChannels.push({ name: channelName, id: roomId! });Based on learnings
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const roomId = await resolvePrivateRoomId(page, channelName); | |
| createdChannels.push({ name: channelName, id: roomId ?? undefined }); | |
| await expect(page).toHaveURL(`/group/${channelName}`); | |
| await expect(poHomeChannel.toastSuccess).toBeVisible(); | |
| await poHomeChannel.dismissToast(); | |
| }); | |
| const roomId = await resolvePrivateRoomId(page, channelName); | |
| - | |
| await expect(roomId, 'Failed to resolve roomId for cleanup').toBeTruthy(); | |
| createdChannels.push({ name: channelName, id: roomId! }); | |
| await expect(page).toHaveURL(`/group/${channelName}`); | |
| await expect(poHomeChannel.toastSuccess).toBeVisible(); | |
| await poHomeChannel.dismissToast(); |
🤖 Prompt for AI Agents
In apps/meteor/tests/e2e/e2e-encryption/e2ee-channel.spec.ts around lines
98-104, the test stores roomId which may be null from resolvePrivateRoomId and
silently skips proper cleanup; update the test to assert that roomId is non-null
(e.g., expect(roomId).not.toBeNull() or throw if null) immediately after
resolution, then push the non-null id into createdChannels (use the asserted
value or a non-null assertion) so cleanup runs and the test fails fast on
resolution failure.
| async create(name: string): Promise<string> { | ||
| await this.sidenav.createEncryptedChannel(name); | ||
| const id = await resolvePrivateRoomId(this.page, name); | ||
| await expect(id, `Failed to resolve roomId for ${name}`).toBeTruthy(); | ||
| return id || ''; | ||
| } |
There was a problem hiding this comment.
Add error handling for room creation failures.
The create method continues with an empty string if resolvePrivateRoomId returns null, which will cause cleanup failures and potential test flakes.
Apply this fix to fail fast on room resolution errors:
async create(name: string): Promise<string> {
await this.sidenav.createEncryptedChannel(name);
const id = await resolvePrivateRoomId(this.page, name);
- await expect(id, `Failed to resolve roomId for ${name}`).toBeTruthy();
- return id || '';
+ if (!id) {
+ throw new Error(`Failed to resolve roomId for ${name}`);
+ }
+ return id;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async create(name: string): Promise<string> { | |
| await this.sidenav.createEncryptedChannel(name); | |
| const id = await resolvePrivateRoomId(this.page, name); | |
| await expect(id, `Failed to resolve roomId for ${name}`).toBeTruthy(); | |
| return id || ''; | |
| } | |
| async create(name: string): Promise<string> { | |
| await this.sidenav.createEncryptedChannel(name); | |
| const id = await resolvePrivateRoomId(this.page, name); | |
| if (!id) { | |
| throw new Error(`Failed to resolve roomId for ${name}`); | |
| } | |
| return id; | |
| } |
🤖 Prompt for AI Agents
In apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts around lines 200-205,
the create method currently continues and returns an empty string when
resolvePrivateRoomId returns null; change it to fail fast: after awaiting
resolvePrivateRoomId, if the id is falsy throw a new Error(`Failed to resolve
roomId for ${name}`) (or include any diagnostic info available) instead of
returning '' so callers/teardown never get a silent empty id; ensure the
function returns the resolved id when present.
Proposed changes (including videos or screenshots)
This PR refactors the
e2ee-encrypted-channels.spec.tstest file by breaking it down into multiple smaller, focused test files. This improves test maintainability, reduces complexity, and makes the test suite more organized.Test File Restructuring
e2ee-encrypted-channels.spec.ts(322 lines deleted)e2ee-channel.spec.ts(+110 lines) - Basic channel encryption functionalitye2ee-mentions.spec.ts(+84 lines) - E2E encryption with mentionse2ee-message-actions.spec.ts(+109 lines) - Message actions in encrypted channelse2ee-sidebar.spec.ts(+86 lines) - Sidebar functionality for encrypted roomse2ee-threads.spec.ts(+71 lines) - Thread messaging in encrypted channelsPage Object Enhancements
home-content.ts(+44 lines):Issue(s)
ESH-33
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Tests
Chores