test: modularize e2e encryption tests and extract common utilities #36890
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
jessicaschelly
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #36890 +/- ##
===========================================
+ Coverage 67.10% 67.13% +0.03%
===========================================
Files 3404 3406 +2
Lines 117782 117742 -40
Branches 21465 21577 +112
===========================================
+ Hits 79035 79044 +9
+ Misses 36051 36015 -36
+ Partials 2696 2683 -13
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
There was a problem hiding this comment.
Pull Request Overview
This PR modularizes the e2e encryption tests by breaking down a large monolithic test file (1172 lines) into smaller, focused test modules and extracts common utilities for better maintainability and organization.
- Splits single large e2e-encryption.spec.ts into 10 specialized test files organized by feature area
- Extracts shared
preserveSettingsutility for consistent settings management across test modules - Creates dedicated test files for specific E2EE functionality like passphrase management, file encryption, server settings, etc.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/meteor/tests/e2e/utils/preserveSettings.ts | New utility for preserving and restoring server settings across tests |
| apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.ts | Tests for E2EE server settings and slash commands functionality |
| apps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.ts | Tests for PDF export functionality in encrypted rooms |
| apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts | Tests for E2EE passphrase setup, reset, and room state management |
| apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts | Tests for OTR (Off-The-Record) functionality in encrypted rooms |
| apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts | Tests for legacy E2EE message format support |
| apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts | Tests for E2EE key reset functionality |
| apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts | Tests for file encryption features and settings |
| apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts | Tests for basic encryption/decryption functionality |
| apps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.ts | Tests for encrypted channel creation and message handling |
| apps/meteor/tests/e2e/e2e-encryption.spec.ts | Original monolithic test file removed |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughDeletes a monolithic E2EE end-to-end spec and replaces it with multiple focused Playwright specs under Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor TestRunner as Test Runner
participant API as Test API Client
participant Server as Server
participant Browser as Playwright Browser
participant App as Web App
Note over TestRunner,API: test setup
TestRunner->>API: GET current settings (preserve)
API->>Server: read settings
Server-->>API: setting values
TestRunner->>API: POST /settings (enable E2E, flags)
API->>Server: apply settings
Server-->>API: 200 OK
Note over TestRunner,Browser: test execution
TestRunner->>Browser: launch context (storage state)
Browser->>App: navigate /home or /room
App-->>Browser: render UI (encrypted icons/banners)
Browser->>App: user actions (send message / upload file / toggle E2E / open kebab)
App-->>Browser: show modals, toasts, placeholders, download
Note over TestRunner,API: adjust mid-test server state
TestRunner->>API: modify whitelist/blacklist or reset keys
API->>Server: update config
Server-->>TestRunner: confirmation
Note over TestRunner,API: teardown
TestRunner->>API: POST original settings (restore)
API->>Server: restore settings
Server-->>API: 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts (2)
31-34: Remove manual restoration; rely on preserveSettings.This duplicates the utility and may restore to incorrect defaults instead of originals.
-test.afterAll(async ({ api }) => { - await api.post('/settings/FileUpload_MediaTypeWhiteList', { value: '' }); - await api.post('/settings/FileUpload_MediaTypeBlackList', { value: 'image/svg+xml' }); -});
123-126: Nested manual restoration not needed.Let preserveSettings restore E2E_Enable_Encrypt_Files and FileUpload_* values at suite end.
- test.afterAll(async ({ api }) => { - await api.post('/settings/E2E_Enable_Encrypt_Files', { value: originalSettings.E2E_Enable_Encrypt_Files }); - await api.post('/settings/FileUpload_MediaTypeBlackList', { value: 'image/svg+xml' }); - });apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (1)
27-27: Drop manual setting restores and avoid unused originalSettings.preserveSettings already restores originals; the extra afterAll is redundant and introduces a risk of restoring wrong values. Also prevents an unused variable.
-const originalSettings = preserveSettings(settingsList); +preserveSettings(settingsList); @@ -test.afterAll(async ({ api }) => { - await api.post('/settings/E2E_Enable', { value: originalSettings.E2E_Enable }); - await api.post('/settings/E2E_Allow_Unencrypted_Messages', { value: originalSettings.E2E_Allow_Unencrypted_Messages }); - await api.post('/settings/E2E_Enabled_Default_DirectRooms', { value: originalSettings.E2E_Enabled_Default_DirectRooms }); - await api.post('/settings/E2E_Enabled_Default_PrivateRooms', { value: originalSettings.E2E_Enabled_Default_PrivateRooms }); -});Also applies to: 39-44
apps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.ts (1)
171-176: DRY up repeated mention locators.Extract helpers (e.g., getUserMention(name), getChannelMention(name)) in a page-object.
Also applies to: 189-197, 211-221
🧹 Nitpick comments (20)
apps/meteor/tests/e2e/utils/preserveSettings.ts (2)
14-16: Make restore resilient (allSettled) and skip undefined values.If any restore fails, Promise.all will reject early and may leave settings partially modified. Also, attempting to POST an undefined value can be problematic.
- test.afterAll(async ({ api }) => { - await Promise.all(settingsList.map((setting) => api.post(`/settings/${setting}`, { value: originalSettings[setting] }))); - }); + test.afterAll(async ({ api }) => { + const tasks = settingsList + .filter((s) => Object.prototype.hasOwnProperty.call(originalSettings, s)) + .map(async (s) => { + const value = originalSettings[s]; + if (value === undefined) return; // skip if not retrievable + await api.post(`/settings/${s}`, { value }); + }); + await Promise.allSettled(tasks); + });
4-4: Add explicit return type for clarity.-export function preserveSettings(settingsList: string[]) { +export function preserveSettings(settingsList: string[]): Record<string, unknown> {apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts (3)
37-39: Avoid fixed sleeps; wait for UI readiness.Replace hard waits with locator-driven waits to reduce flakiness.
- await page.waitForTimeout(1000); + await poHomeChannel.sidenav.searchList.waitFor();
55-56: Remove arbitrary delay; assert on the expected state instead.This 1s sleep is unnecessary with proper assertions following it.
- await page.waitForTimeout(1000);
41-41: DM URL can vary by username order; assert with a regex.Slug order may differ; this makes the check robust.
- await expect(page).toHaveURL(`/direct/user2${Users.userE2EE.data.username}`); + await expect(page).toHaveURL(new RegExp(`/direct/(user2${Users.userE2EE.data.username}|${Users.userE2EE.data.username}user2)$`));apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts (1)
36-38: Guard closing auxiliary page on failures.Minor: if beforeEach fails, afterEach may throw on close().
-test.afterEach(async () => { - await anotherClientPage.close(); -}); +test.afterEach(async () => { + if (anotherClientPage && !anotherClientPage.isClosed()) { + await anotherClientPage.close(); + } +});apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts (3)
24-31: Move data injection and state restore to suite setup to reduce flakiness.Running DB writes in-test increases runtime and cross-test interference.
test.beforeAll(async ({ api }) => { await api.post('/settings/E2E_Enable', { value: true }); await api.post('/settings/E2E_Allow_Unencrypted_Messages', { value: true }); await api.post('/settings/E2E_Enabled_Default_DirectRooms', { value: false }); await api.post('/settings/E2E_Enabled_Default_PrivateRooms', { value: false }); await api.post('/im.delete', { roomId: `user2${Users.userE2EE.data.username}` }); + await injectInitialData(); }); @@ - await injectInitialData(); - await restoreState(page, Users.userE2EE, { except: ['private_key', 'public_key', 'e2e.randomPassword'] }); + await restoreState(page, Users.userE2EE, { except: ['private_key', 'public_key', 'e2e.randomPassword'] });Also applies to: 40-41
51-52: Assert rid presence before using it.Avoid passing null to APIs if attribute is missing.
- const rid = await page.locator('[data-qa-rc-room]').getAttribute('data-qa-rc-room'); + const ridAttr = await page.locator('[data-qa-rc-room]').getAttribute('data-qa-rc-room'); + expect(ridAttr, 'room id (data-qa-rc-room) must be present').toBeTruthy(); + const rid = ridAttr!;
54-59: Prefer import() over require() in browser context.If the selector engine/bundler changes, require path may break. Dynamic import is safer.
- const { e2e } = require('/client/lib/e2ee/rocketchat.e2e.ts'); + const { e2e } = await import('/client/lib/e2ee/rocketchat.e2e.ts');apps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.ts (2)
55-57: Prefer semantic disabled assertion.Once the a11y issue is fixed, use toBeDisabled() for clarity; keep current as a FIXME fallback.
- await expect(exportMessagesTab.method).toContainClass('disabled'); // FIXME: looks like the component have an a11y issue + // FIXME: component a11y issue—switch to the assertion below when fixed: + // await expect(exportMessagesTab.method).toBeDisabled();
31-41: Key reset + login flow: ensure state isolation.Good isolation. Consider moving resetOwnE2EKey to test.step for easier debugging in traces.
apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts (2)
8-14: Preserve FileUpload whitelist/blacklist too to avoid cross-spec pollution.Extend the preserved settings so manual resets aren’t needed and the runner restores true originals.
Apply this diff:
const settingsList = [ 'E2E_Enable', 'E2E_Allow_Unencrypted_Messages', 'E2E_Enable_Encrypt_Files', 'E2E_Enabled_Default_DirectRooms', 'E2E_Enabled_Default_PrivateRooms', + 'FileUpload_MediaTypeWhiteList', + 'FileUpload_MediaTypeBlackList', ];
105-114: Assert the failure explicitly to reduce false positives.Right now we only infer failure from “last message unchanged.” Also verify no new message was added.
await test.step('send text file again with blacklisted setting set, file upload should fail', async () => { - await poHomeChannel.content.dragAndDropTxtFile(); + const messages = page.locator('[data-qa-type="message"]'); + const countBefore = await messages.count(); + await poHomeChannel.content.dragAndDropTxtFile(); await poHomeChannel.content.descriptionInput.fill('message 3'); await poHomeChannel.content.fileNameInput.fill('any_file3.txt'); await poHomeChannel.content.btnModalConfirm.click(); + await expect(messages).toHaveCount(countBefore); await expect(poHomeChannel.content.lastUserMessage.locator('.rcx-icon--name-key')).toBeVisible(); await expect(poHomeChannel.content.getFileDescription).toHaveText('message 2'); await expect(poHomeChannel.content.lastMessageFileName).toContainText('any_file2.txt'); });Optionally also assert an error toast if available (e.g.,
.rcx-toastbar--error).apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (2)
263-263: Prefer role/name over generic CSS for modal primary action.More robust against class changes and improves a11y alignment.
- await page.locator('#modal-root .rcx-button--primary').click(); + await page.getByRole('button', { name: 'Enable encryption' }).click();
269-269: Replace fixed sleep with a deterministic readiness check.Avoid
waitForTimeout(300). Wait for a stable UI signal (e.g., composer enabled or banner gone).Example:
await expect(poHomeChannel.content.inputMessage).toBeEnabled();apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts (1)
71-73: Relax exact placeholder string match to reduce i18n/wording flakiness.Match a stable substring instead of the full sentence.
- 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.');Also applies to: 145-147
apps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.ts (4)
56-57: Replace fixed sleeps with state-based waits.Wait for the header encryption icon to reflect the toggle instead of arbitrary delays.
- await page.waitForTimeout(1000); + await expect(poHomeChannel.content.encryptedRoomHeaderIcon).not.toBeVisible(); @@ - await page.waitForTimeout(1000); + await expect(poHomeChannel.content.encryptedRoomHeaderIcon).toBeVisible(); @@ - await page.waitForTimeout(1000); + await expect(poHomeChannel.content.encryptedRoomHeaderIcon).toBeVisible(); @@ - await page.waitForTimeout(1000); + await expect(poHomeChannel.content.encryptedRoomHeaderIcon).toBeVisible();Also applies to: 68-69, 149-151, 269-270
49-53: Avoid force: true clicks unless strictly necessary.Prefer visible/attached checks; force can mask legitimate UI issues and cause flakiness.
Example:
await expect(poHomeChannel.tabs.kebab).toBeVisible(); await poHomeChannel.tabs.kebab.click();Also applies to: 63-66, 264-268
299-301: Fix “stared” → “starred” in test strings and assertions.Minor text quality nit to avoid propagating typos across assertions.
- await poHomeChannel.content.sendMessage('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 stared.'); + await expect(poHomeChannel.content.lastUserMessageBody).toHaveText('This message should be pinned and starred.'); @@ - await expect(lastPinnedMessage).toContainText('This message should be pinned and stared.'); + await expect(lastPinnedMessage).toContainText('This message should be pinned and starred.'); @@ - await expect(lastStarredMessage).toContainText('This message should be pinned and stared.'); + await expect(lastStarredMessage).toContainText('This message should be pinned and starred.');Also applies to: 323-324, 336-337
273-275: Rename “encriptedMessage” to “encryptedMessage”.**Spelling nit for readability.
- const encriptedMessage1 = 'first ENCRYPTED message'; - const encriptedMessage2 = 'second ENCRYPTED message'; - await poHomeChannel.content.sendMessage(encriptedMessage1); - await poHomeChannel.content.sendMessage(encriptedMessage2); + const encryptedMessage1 = 'first ENCRYPTED message'; + const encryptedMessage2 = 'second ENCRYPTED message'; + await poHomeChannel.content.sendMessage(encryptedMessage1); + await poHomeChannel.content.sendMessage(encryptedMessage2); @@ - await expect(poHomeChannel.content.lastUserMessageBody).toHaveText(encriptedMessage2); + await expect(poHomeChannel.content.lastUserMessageBody).toHaveText(encryptedMessage2); @@ - await expect(sidebarChannel.locator('span')).toContainText(encriptedMessage1); + await expect(sidebarChannel.locator('span')).toContainText(encryptedMessage1);Also applies to: 279-279, 287-287
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/meteor/tests/e2e/e2e-encryption.spec.ts(0 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.ts(1 hunks)apps/meteor/tests/e2e/utils/preserveSettings.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/meteor/tests/e2e/e2e-encryption.spec.ts
🧰 Additional context used
🧬 Code graph analysis (9)
apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts (6)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/login.ts (1)
LoginPage(9-50)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (2)
SaveE2EEPasswordBanner(19-23)SaveE2EEPasswordModal(42-67)apps/meteor/tests/e2e/page-objects/encrypted-room.ts (1)
EncryptedRoomPage(5-47)apps/meteor/tests/e2e/page-objects/fragments/home-sidenav.ts (1)
HomeSidenav(6-245)apps/meteor/tests/e2e/page-objects/fragments/file-upload-modal.ts (1)
FileUploadModal(5-34)
apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.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-key-reset.spec.ts (3)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/fixtures/createAuxContext.ts (1)
createAuxContext(5-19)apps/meteor/tests/e2e/page-objects/account-profile.ts (1)
AccountProfile(5-187)
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (9)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/login.ts (1)
LoginPage(9-50)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (7)
SaveE2EEPasswordBanner(19-23)SaveE2EEPasswordModal(42-67)EnterE2EEPasswordBanner(25-30)EnterE2EEPasswordModal(69-96)E2EEKeyDecodeFailureBanner(32-40)password(50-52)ResetE2EEPasswordModal(98-111)apps/meteor/tests/e2e/page-objects/fragments/home-sidenav.ts (1)
HomeSidenav(6-245)apps/meteor/tests/e2e/page-objects/account-security.ts (1)
AccountSecurityPage(5-59)apps/meteor/tests/e2e/page-objects/account-profile.ts (1)
AccountProfile(5-187)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)apps/meteor/tests/e2e/fixtures/inject-initial-data.ts (1)
injectInitialData(8-84)apps/meteor/tests/e2e/fixtures/userStates.ts (2)
restoreState(142-155)storeState(138-140)
apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.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-encrypted-channels.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-file-encryption.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-legacy-format.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/fixtures/inject-initial-data.ts (1)
injectInitialData(8-84)apps/meteor/tests/e2e/fixtures/userStates.ts (1)
restoreState(142-155)apps/meteor/client/lib/e2ee/rocketchat.e2e.ts (1)
e2e(897-897)
apps/meteor/tests/e2e/e2e-encryption/e2ee-pdf-export.spec.ts (5)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/login.ts (1)
LoginPage(9-50)apps/meteor/tests/e2e/page-objects/fragments/home-sidenav.ts (1)
HomeSidenav(6-245)apps/meteor/tests/e2e/page-objects/encrypted-room.ts (1)
EncryptedRoomPage(5-47)apps/meteor/tests/e2e/page-objects/fragments/export-messages-tab.ts (1)
ExportMessagesTab(3-87)
🔇 Additional comments (4)
apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts (1)
23-29: Verify /im.delete payload as above.Same concern as other spec; ensure correct rid usage.
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
4-19: Playwright configured with a single worker — cross-worker race risk mitigatedapps/meteor/playwright.config.ts and apps/meteor/playwright-federation.config.ts set workers: 1, so preserveSettings' global beforeAll/afterAll won’t run across multiple Playwright workers. If CI/CLI overrides workers (e.g., -w/--workers or PLAYWRIGHT_WORKERS), re-evaluate.
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (1)
20-26: No change required — already single-worker/serialPlaywright config sets workers: 1 (apps/meteor/playwright.config.ts:66) and this spec uses test.describe.serial (apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts:160).
apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.ts (1)
51-54: Playwright Locator.clear() is not a thing; use fill('').This will crash at runtime.
- await page.locator('[name="msg"]').clear(); + await page.locator('[name="msg"]').fill('');Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (1)
27-27: Deduplicate settings restoration; rely on preserveSettings’s built-in beforeAll/afterAll.Manual restoration duplicates the utility and risks drift. Also avoids an unused variable if you don’t need
originalSettingselsewhere.Apply this diff:
-const originalSettings = preserveSettings(settingsList); +preserveSettings(settingsList);-test.afterAll(async ({ api }) => { - await api.post('/settings/E2E_Enable', { value: originalSettings.E2E_Enable }); - await api.post('/settings/E2E_Allow_Unencrypted_Messages', { value: originalSettings.E2E_Allow_Unencrypted_Messages }); - await api.post('/settings/E2E_Enabled_Default_DirectRooms', { value: originalSettings.E2E_Enabled_Default_DirectRooms }); - await api.post('/settings/E2E_Enabled_Default_PrivateRooms', { value: originalSettings.E2E_Enabled_Default_PrivateRooms }); -});Optional (preferred): scope preservation to this suite (mirrors the second suite) and drop the module-level call entirely.
-const originalSettings = preserveSettings(settingsList); +// move inside the describe just after test.use(...)Also applies to: 39-44
🧹 Nitpick comments (8)
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (8)
258-264: Use the EnterE2EEPasswordModal page object instead of brittle CSS selectors.Improves resiliency and aligns with semantic-locator guidance.
await poHomeChannel.btnRoomEnterE2EEPassword.click(); - - await page.locator('#modal-root input').fill(e2eePassword); - - await page.locator('#modal-root .rcx-button--primary').click(); + const enterModal = new EnterE2EEPasswordModal(page); + await enterModal.enterPassword(e2eePassword);
268-269: Avoid hardcoded timeout; use a web‑first readiness check.Sleeps are flaky; assert the composer is enabled/ready.
- // For E2EE to complete init setup - await page.waitForTimeout(300); + await expect(poHomeChannel.content.inputMessage).toBeEnabled();
187-189: Prefer semantic locators (getByRole) over page.locator with role= queries.- await page.locator('role=button[name="Login"]').waitFor(); + await expect(page.getByRole('button', { name: 'Login', exact: true })).toBeVisible();
302-307: Prefer semantic locators (getByRole) over page.locator with role= queries.- await page.reload(); - await page.locator('role=button[name="Login"]').waitFor(); + await page.reload(); + await expect(page.getByRole('button', { name: 'Login', exact: true })).toBeVisible();
297-299: Use role-based locator for the Security link.- await page.locator('role=navigation >> a:has-text("Security")').click(); + await page.getByRole('navigation').getByRole('link', { name: 'Security' }).click();
311-314: Reuse page objects for search instead of raw chained selectors.- await page.locator('role=navigation >> role=button[name=Search]').click(); - await page.locator('role=search >> role=searchbox').fill(channelName); - await page.locator(`role=search >> role=listbox >> role=link >> text="${channelName}"`).click(); + await poHomeChannel.sidenav.searchRoom(channelName); + await poHomeChannel.sidenav.getSearchItemByName(channelName).click();
192-194: Avoid id-based waits; assert the main region is visible.- await page.goto('/home'); - await page.waitForSelector('#main-content'); + await page.goto('/home'); + await expect(page.getByRole('main')).toBeVisible();
182-229: Structure the long scenario with test.step for readability and diagnostics.Makes failures easier to pinpoint, per guideline.
test('expect save password state on encrypted room', async ({ page }) => { - await page.goto('/account/security'); - ... - await expect(poHomeChannel.content.lastUserMessage.locator('.rcx-icon--name-key')).toBeVisible(); + await test.step('reset keys and relogin', async () => { + await page.goto('/account/security'); + await poAccountProfile.securityE2EEncryptionSection.click(); + await poAccountProfile.securityE2EEncryptionResetKeyButton.click(); + await expect(page.getByRole('button', { name: 'Login', exact: true })).toBeVisible(); + await injectInitialData(); + await restoreState(page, Users.admin); + }); + + await test.step('create encrypted room and verify save-password state', async () => { + await page.goto('/home'); + await expect(poHomeChannel.bannerSaveEncryptionPassword).toBeVisible(); + const channelName = faker.string.uuid(); + await poHomeChannel.sidenav.createEncryptedChannel(channelName); + await expect(page).toHaveURL(`/group/${channelName}`); + await expect(poHomeChannel.content.encryptedRoomHeaderIcon.first()).toBeVisible(); + await expect(poHomeChannel.btnRoomSaveE2EEPassword).toBeVisible(); + }); + + await test.step('save password and send encrypted message', async () => { + await poHomeChannel.btnRoomSaveE2EEPassword.click(); + e2eePassword = (await page.evaluate(() => localStorage.getItem('e2e.randomPassword'))) || 'undefined'; + await expect(poHomeChannel.dialogSaveE2EEPassword).toContainText(e2eePassword); + await poHomeChannel.btnSavedMyPassword.click(); + await poHomeChannel.content.inputMessage.waitFor(); + await poHomeChannel.content.sendMessage('hello world'); + await expect(poHomeChannel.content.lastUserMessageBody).toHaveText('hello world'); + await expect(poHomeChannel.content.lastUserMessage.locator('.rcx-icon--name-key')).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 (3)
apps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/tests/e2e/e2e-encryption/e2ee-encryption-decryption.spec.ts
- apps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.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-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 (9)
📓 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/**/*.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)
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
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
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
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
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
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
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
📚 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/**/*.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 : 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 : Group related tests in the same file
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/**/*.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.step() to organize complex test scenarios
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
🧬 Code graph analysis (1)
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts (9)
apps/meteor/tests/e2e/utils/preserveSettings.ts (1)
preserveSettings(4-19)apps/meteor/tests/e2e/page-objects/login.ts (1)
LoginPage(9-53)apps/meteor/tests/e2e/page-objects/fragments/e2ee.ts (7)
SaveE2EEPasswordBanner(20-24)SaveE2EEPasswordModal(43-68)EnterE2EEPasswordBanner(26-31)EnterE2EEPasswordModal(70-97)E2EEKeyDecodeFailureBanner(33-41)password(51-53)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/page-objects/account-profile.ts (1)
AccountProfile(5-179)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(5-123)apps/meteor/tests/e2e/fixtures/inject-initial-data.ts (1)
injectInitialData(8-84)apps/meteor/tests/e2e/fixtures/userStates.ts (2)
restoreState(142-155)storeState(138-140)
⏰ 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
apps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.ts
Outdated
Show resolved
Hide resolved
apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts
Outdated
Show resolved
Hide resolved
apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
Outdated
Show resolved
Hide resolved
apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts
Outdated
Show resolved
Hide resolved
apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts (7)
50-64: Assert the REST response to catch silent failuresValidate HTTP status and
successpayload to reduce flaky false positives.const sendEncryptedMessage = async (request: APIRequestContext, rid: string, encryptedMsg: string) => { return request.post(`${BASE_API_URL}/chat.sendMessage`, { headers: { 'X-Auth-Token': Users.userE2EE.data.loginToken, 'X-User-Id': Users.userE2EE.data._id, + 'Content-Type': 'application/json', }, data: { message: { rid, msg: encryptedMsg, t: 'e2e', }, }, }); }; @@ - await sendEncryptedMessage(request, rid as string, encryptedMessage); + const res = await sendEncryptedMessage(request, rid as string, encryptedMessage); + expect(res.ok()).toBeTruthy(); + const body = await res.json(); + expect((body as any).success).toBeTruthy();Also applies to: 85-86
76-79: Replace flaky “Send” button check with editor readinessThe Send button visibility is timing/UI-variant. Assert the composer instead via the page object.
await expect(page.getByTitle('Encrypted')).toBeVisible(); - // TODO: Fix this flakiness - await expect(page.getByRole('button', { name: 'Send' })).toBeVisible(); + await expect(poHomeChannel.composer).toBeVisible();
66-68: Tighten test name and make channel id more diagnosable/deterministicClearer name and unique prefix aid debugging across parallel runs.
-test('legacy expect create a private channel encrypted and send an encrypted message', async ({ page, request }) => { - const channelName = faker.string.uuid(); +test('should create an encrypted private channel and deliver a legacy-format encrypted message', async ({ page, request }) => { + const channelName = `e2ee-${test.info().workerIndex}-${faker.string.uuid()}`;
23-23: Redundant auth/session setupYou use both
test.use({ storageState })andrestoreState(...). Double initialization can mask issues and adds flakiness. Pick one approach (prefer the fixture-leveltest.use), and only callrestoreStateif you intentionally need to override specific keys for this scenario.Also applies to: 69-71
80-89: Prefer POM methods over raw locators for rid and encrypted iconEncapsulate DOM details to reduce churn and improve readability.
Spec changes:
- const rid = await page.locator('[data-qa-rc-room]').getAttribute('data-qa-rc-room'); + const rid = await poHomeChannel.getRid(); @@ - await expect(poHomeChannel.content.lastUserMessage.locator('.rcx-icon--name-key')).toBeVisible(); + await expect(poHomeChannel.lastMessageIsEncryptedIcon).toBeVisible();Page object additions (apps/meteor/tests/e2e/page-objects/home-channel.ts):
get currentRoom(): Locator { return this.page.getByRole('main').locator('[data-qa-rc-room]'); } async getRid(): Promise<string | null> { return this.currentRoom.getAttribute('data-qa-rc-room'); } get lastMessageIsEncryptedIcon(): Locator { return this.content.lastUserMessage.locator('.rcx-icon--name-key'); }
11-19: Settings preservation scope may be too narrow
injectInitialData()touches non‑E2EE settings (e.g.,Show_Setup_Wizard,API_Enable_Rate_Limiter_Dev). Either run this suite serial with bootstrap isolated (recommended above) or extendsettingsListto cover everything this spec changes, to avoid leaking state to other specs.
38-47: Brittle absolute require inside page contextConfirmed apps/meteor/client/lib/e2ee/rocketchat.e2e.ts exists and exports
e2e(export const e2e = new E2E()). The current require('/client/lib/e2ee/rocketchat.e2e.ts') still works but is brittle (absolute path + .ts extension relies on bundler internals and can break in other build modes).
- Prefer exposing a stable test-only hook (e.g., window.__rc.e2e = e2e) and access that from page.evaluate.
- Or drop the extension and confirm resolution: require('/client/lib/e2ee/rocketchat.e2e').
- Alternatively use Puppeteer/Playwright bridging (page.exposeFunction / evaluateOnNewDocument) to call the e2e API.
Location: apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts (lines ~38–47).
📜 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-encrypted-channels.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-otr.spec.ts(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-server-settings.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/meteor/tests/e2e/e2e-encryption/e2ee-encrypted-channels.spec.ts
- apps/meteor/tests/e2e/e2e-encryption/e2ee-key-reset.spec.ts
- 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-otr.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-legacy-format.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-legacy-format.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-legacy-format.spec.ts
🧠 Learnings (6)
📓 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/**/*.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)
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
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
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
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
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
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
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
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
📚 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-legacy-format.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-legacy-format.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-legacy-format.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-legacy-format.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-legacy-format.spec.ts
🧬 Code graph analysis (1)
apps/meteor/tests/e2e/e2e-encryption/e2ee-legacy-format.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/client/lib/e2ee/rocketchat.e2e.ts (1)
e2e(897-897)apps/meteor/tests/e2e/fixtures/inject-initial-data.ts (1)
injectInitialData(8-84)apps/meteor/tests/e2e/fixtures/userStates.ts (1)
restoreState(142-155)
⏰ 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
dougfabris
force-pushed
the
test/break-down-e2e-encrypt
branch
from
48677a9 to
4b6d2a1
Compare
dougfabris
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Proposed changes (including videos or screenshots)
This PR modularizes the e2e encryption tests by breaking down a large monolithic test file (1172 lines) into smaller, focused test modules and extracts common utilities for better maintainability and organization.
Issue(s)
https://rocketchat.atlassian.net/browse/ESH-32
Steps to test or reproduce
Further comments
Summary by CodeRabbit