test: Merge modal page objects
#37253
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 |
|
dougfabris
added
the
stat: QA assured
dionisio-bot
bot
removed
the
stat: QA assured
dougfabris
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
WalkthroughRefactors e2e modal handling: removes the old Changes
Sequence Diagram(s)sequenceDiagram
participant Test as E2E Test
participant AppsModal as AppsModal (page-object)
participant ModalBase as Modal (fragment)
participant UI as Browser Modal UI
Test->>AppsModal: instantiate(page)
Test->>AppsModal: waitForDisplay()
AppsModal->>ModalBase: delegate waitForDisplay()
ModalBase->>UI: assert dialog visible
Test->>AppsModal: submit("text")
AppsModal->>AppsModal: fill text input
AppsModal->>ModalBase: click submit/save
ModalBase->>UI: click button -> UI dismisses
ModalBase->>AppsModal: wait for dismissal
AppsModal->>Test: resolved
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![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
🧹 Nitpick comments (5)
apps/meteor/tests/e2e/page-objects/fragments/apps-modal.ts (3)
1-1: Use consistent import path.Other test files import from
'@playwright/test'instead of'playwright-core'. Ensure consistency across the codebase.Apply this diff:
-import type { Locator, Page } from 'playwright-core'; +import type { Locator, Page } from '@playwright/test';
6-6: Remove unused property.The
pageproperty is declared but never used in this class. Consider removing it to reduce clutter.Apply this diff:
export class AppsModal extends Modal { - protected readonly page: Page; - constructor(page: Page) {
16-18: Consider using role-based locator for error message.If the error message has a semantic role like
alertorstatus, prefergetByRole()overgetByText()for more robust and accessible selection.apps/meteor/tests/e2e/page-objects/fragments/edit-status-modal.ts (2)
1-1: Use consistent import path.Similar to
AppsModal, this file imports from'playwright-core'instead of'@playwright/test'. Maintain consistency with other test files.Apply this diff:
-import type { Page } from 'playwright-core'; +import type { Page } from '@playwright/test';
6-6: Remove unused property.The
pageproperty is declared but never referenced. Consider removing it.Apply this diff:
export class EditStatusModal extends Modal { - protected readonly page: Page; - constructor(page: Page) {
📜 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 (7)
apps/meteor/client/views/modal/uikit/ModalBlock.tsx(1 hunks)apps/meteor/tests/e2e/apps/apps-modal.spec.ts(2 hunks)apps/meteor/tests/e2e/page-objects/fragments/apps-modal.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/edit-status-modal.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/modal.ts(1 hunks)apps/meteor/tests/e2e/page-objects/modal.ts(0 hunks)apps/meteor/tests/e2e/presence.spec.ts(2 hunks)
💤 Files with no reviewable changes (1)
- apps/meteor/tests/e2e/page-objects/modal.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/presence.spec.tsapps/meteor/tests/e2e/apps/apps-modal.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/presence.spec.tsapps/meteor/tests/e2e/page-objects/fragments/edit-status-modal.tsapps/meteor/tests/e2e/page-objects/fragments/modal.tsapps/meteor/tests/e2e/apps/apps-modal.spec.tsapps/meteor/tests/e2e/page-objects/fragments/apps-modal.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/presence.spec.tsapps/meteor/tests/e2e/page-objects/fragments/edit-status-modal.tsapps/meteor/tests/e2e/page-objects/fragments/modal.tsapps/meteor/tests/e2e/apps/apps-modal.spec.tsapps/meteor/tests/e2e/page-objects/fragments/apps-modal.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/edit-status-modal.tsapps/meteor/tests/e2e/page-objects/fragments/modal.tsapps/meteor/tests/e2e/page-objects/fragments/apps-modal.ts
🧠 Learnings (8)
📓 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/**/*.{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/presence.spec.tsapps/meteor/tests/e2e/apps/apps-modal.spec.tsapps/meteor/tests/e2e/page-objects/fragments/apps-modal.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/presence.spec.tsapps/meteor/tests/e2e/apps/apps-modal.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/apps/apps-modal.spec.tsapps/meteor/tests/e2e/page-objects/fragments/apps-modal.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/apps/apps-modal.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/apps/apps-modal.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/apps/apps-modal.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/apps/apps-modal.spec.ts
🧬 Code graph analysis (3)
apps/meteor/tests/e2e/presence.spec.ts (1)
apps/meteor/tests/e2e/page-objects/fragments/edit-status-modal.ts (1)
EditStatusModal(5-20)
apps/meteor/client/views/modal/uikit/ModalBlock.tsx (1)
packages/fuselage-ui-kit/src/surfaces/index.ts (1)
modalParser(15-15)
apps/meteor/tests/e2e/apps/apps-modal.spec.ts (2)
apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(6-127)apps/meteor/tests/e2e/page-objects/fragments/apps-modal.ts (1)
AppsModal(5-29)
⏰ 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 (12)
apps/meteor/tests/e2e/page-objects/fragments/modal.ts (3)
8-10: LGTM: Well-structured visibility assertion.The method correctly uses web-first assertions and complements the existing
waitForDismissal()method.
16-18: Consider i18n for button text.The hard-coded 'Save' string may fail in non-English locales if the button text is localized. If the application supports multiple languages, consider whether this locator needs to be more flexible.
20-23: LGTM: Clean save flow.The method correctly sequences the click action with dismissal verification.
apps/meteor/client/views/modal/uikit/ModalBlock.tsx (1)
178-178: LGTM: Proper ARIA labeling for accessibility.The modal is correctly linked to its title using
aria-labelledby, which improves screen reader support and follows WAI-ARIA authoring practices.Also applies to: 181-181
apps/meteor/tests/e2e/presence.spec.ts (2)
4-4: LGTM: Clean migration to specialized modal.The switch to
EditStatusModalfollows the Page Object Model pattern consistently and improves code organization. Based on learnings.Also applies to: 51-51
55-55: LGTM: Cleaner API encapsulates modal interaction.The
changeStatusMessage()method provides a cleaner abstraction compared to the previous multi-step interaction, improving maintainability. Based on coding guidelines.apps/meteor/tests/e2e/page-objects/fragments/apps-modal.ts (1)
20-28: LGTM: Well-structured submit flow.The
btnSubmitgetter correctly uses semantic locators, and thesubmit()method properly sequences the fill, click, and wait operations.apps/meteor/tests/e2e/page-objects/fragments/edit-status-modal.ts (2)
12-14: LGTM: Proper semantic locator usage.The getter correctly uses
getByRole()with an accessible name, following coding guidelines and accessibility best practices. As per coding guidelines.
16-19: LGTM: Clean encapsulation of status change flow.The method properly leverages the base
Modal.save()method and provides a clear API for changing status messages.apps/meteor/tests/e2e/apps/apps-modal.spec.ts (3)
6-6: LGTM: Clean migration to AppsModal.The test correctly adopts the new
AppsModalpage object, following the Page Object Model pattern consistently. Based on learnings.Also applies to: 14-14, 22-22
32-40: LGTM: Tests correctly use new modal APIs.The tests properly utilize
waitForDisplay()and direct access tobtnSubmit, maintaining descriptive test names and web-first assertions. Based on learnings.
42-44: LGTM: Clean use of submit helper.The test correctly uses the
submit()helper method, which encapsulates the entire submission flow in a single, maintainable call.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37253 +/- ##
===========================================
- Coverage 67.61% 67.61% -0.01%
===========================================
Files 3341 3341
Lines 114018 114018
Branches 20672 20669 -3
===========================================
- Hits 77096 77094 -2
- Misses 34246 34248 +2
Partials 2676 2676
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
dougfabris
force-pushed
the
test/merge-modal-po
branch
from
f2b18c8 to
5409963
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/meteor/tests/e2e/page-objects/fragments/apps-modal.ts (1)
12-14: Prefer semantic locators over attribute selectors.The textInput getter uses
locator('[name="modal_input"]'), which violates the guideline to avoidpage.locator()and prefer semantic locators likegetByRole('textbox')with an accessible name orgetByLabel(). As per coding guidelines.
🧹 Nitpick comments (4)
apps/meteor/tests/e2e/page-objects/fragments/apps-modal.ts (1)
6-6: Remove unused property.The
pageproperty is declared but never used within the class. Consider removing it to reduce clutter.- protected readonly page: Page; - constructor(page: Page) {apps/meteor/tests/e2e/apps/apps-modal.spec.ts (2)
32-35: Add explicit assertion for modal visibility.The test waits for the modal to display but doesn't assert its visibility. While
waitForDisplay()may throw on timeout, an explicit assertion improves test clarity and intent.test('should allow user open app modal', async () => { await poHomeChannel.content.dispatchSlashCommand('/modal'); await poModal.waitForDisplay(); + await expect(poModal.root).toBeVisible(); });
42-44: Add assertion to verify successful submission.The test calls
submit()but doesn't assert the outcome. Consider verifying that the modal dismissed successfully or that the error message is not visible.test('should not display validation error message in app modal', async () => { await poModal.submit('something'); + await expect(poModal.textInputErrorMessage).not.toBeVisible(); });apps/meteor/tests/e2e/page-objects/fragments/edit-status-modal.ts (1)
6-6: Remove unusedpageproperty.The
pageproperty is declared but never referenced in this class. The baseModalhandles page access through the locator passed tosuper().Apply this diff to remove the unused property:
export class EditStatusModal extends Modal { - protected readonly page: Page; - constructor(page: Page) { super(page.getByRole('dialog', { name: 'Edit Status' })); }
📜 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 (8)
apps/meteor/client/views/modal/uikit/ModalBlock.tsx(1 hunks)apps/meteor/tests/e2e/apps/apps-modal.spec.ts(2 hunks)apps/meteor/tests/e2e/page-objects/fragments/apps-modal.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/edit-status-modal.ts(1 hunks)apps/meteor/tests/e2e/page-objects/fragments/modal.ts(1 hunks)apps/meteor/tests/e2e/page-objects/index.ts(0 hunks)apps/meteor/tests/e2e/page-objects/modal.ts(0 hunks)apps/meteor/tests/e2e/presence.spec.ts(2 hunks)
💤 Files with no reviewable changes (2)
- apps/meteor/tests/e2e/page-objects/modal.ts
- apps/meteor/tests/e2e/page-objects/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/tests/e2e/page-objects/fragments/modal.ts
- apps/meteor/client/views/modal/uikit/ModalBlock.tsx
🧰 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/apps-modal.tsapps/meteor/tests/e2e/page-objects/fragments/edit-status-modal.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/apps-modal.tsapps/meteor/tests/e2e/apps/apps-modal.spec.tsapps/meteor/tests/e2e/presence.spec.tsapps/meteor/tests/e2e/page-objects/fragments/edit-status-modal.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/apps-modal.tsapps/meteor/tests/e2e/apps/apps-modal.spec.tsapps/meteor/tests/e2e/presence.spec.tsapps/meteor/tests/e2e/page-objects/fragments/edit-status-modal.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/apps/apps-modal.spec.tsapps/meteor/tests/e2e/presence.spec.ts
🧠 Learnings (11)
📚 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/apps-modal.tsapps/meteor/tests/e2e/apps/apps-modal.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/apps-modal.tsapps/meteor/tests/e2e/apps/apps-modal.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/apps-modal.tsapps/meteor/tests/e2e/apps/apps-modal.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/apps-modal.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/apps-modal.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/apps/apps-modal.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/apps/apps-modal.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/apps/apps-modal.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/apps/apps-modal.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/apps/apps-modal.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/apps/apps-modal.spec.ts
🧬 Code graph analysis (2)
apps/meteor/tests/e2e/apps/apps-modal.spec.ts (2)
apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(6-127)apps/meteor/tests/e2e/page-objects/fragments/apps-modal.ts (1)
AppsModal(5-29)
apps/meteor/tests/e2e/presence.spec.ts (1)
apps/meteor/tests/e2e/page-objects/fragments/edit-status-modal.ts (1)
EditStatusModal(5-20)
⏰ 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 (7)
apps/meteor/tests/e2e/page-objects/fragments/apps-modal.ts (1)
24-28: Well-structured helper method.The
submitmethod effectively encapsulates the submission flow and follows the DRY principle by providing a reusable interaction pattern.apps/meteor/tests/e2e/apps/apps-modal.spec.ts (1)
37-40: Clear validation test with proper assertion.The test correctly verifies the error message appears when submitting without input, using web-first assertions as recommended.
apps/meteor/tests/e2e/page-objects/fragments/edit-status-modal.ts (3)
8-10: LGTM!The constructor properly targets the "Edit Status" dialog using a semantic locator and passes it to the base
Modalclass.
12-14: LGTM!The getter correctly uses a semantic locator to target the status message textbox via its accessible name.
16-19: LGTM!The method provides a clean abstraction for changing the status message. It properly fills the input and saves the modal, encapsulating the interaction logic per the Page Object Model pattern.
apps/meteor/tests/e2e/presence.spec.ts (2)
4-4: LGTM!The import correctly references the new
EditStatusModalfragment from the appropriate location.
51-55: LGTM!The refactor properly uses the new
EditStatusModalpage object, replacing the previous multi-step interaction with a single, encapsulated method call. This improves maintainability and follows the DRY principle.
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Bug Fixes
Tests