test: add guards to composer focus to prevent flakyness#41106
test: add guards to composer focus to prevent flakyness#41106sampaiodiego wants to merge 3 commits into
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (8)
📜 Recent review details⏰ Context from checks skipped due to timeout. (4)
WalkthroughThe E2E suite now uses a shared group-navigation helper in several flows, adds explicit focus assertions before keyboard interactions, and switches composer text checks to Playwright ChangesE2E test stability and navigation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
|
/jira ARCH-1538 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
sampaiodiego
added
the
stat: QA assured
dionisio-bot
Bot
added
the
stat: ready to merge
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #41106 +/- ##
===========================================
- Coverage 69.16% 69.12% -0.04%
===========================================
Files 3433 3433
Lines 132323 132323
Branches 23087 23076 -11
===========================================
- Hits 91518 91467 -51
- Misses 37455 37499 +44
- Partials 3350 3357 +7
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
sampaiodiego
force-pushed
the
test-flaky-compose-focus
branch
2 times, most recently
from
5ae7178 to
6f5ded9
Compare
sampaiodiego
force-pushed
the
test-flaky-compose-focus
branch
from
6f5ded9 to
9164ccf
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/feature-preview.spec.ts (1)
154-187: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
pagefixture removed but still referenced — will throwReferenceError.The test callback was changed to
async ({ browser }) => {, dropping thepageparameter, but the body still references the barepageidentifier at Line 167 (expect(page.url())...) and Line 173 (expect(page.url())...). Sincepageis no longer destructured/in scope, this will throw aReferenceErrorat runtime and fail the test.Based on the line-range change details: "removed the `page` fixture parameter from the test callback" while preserving the remaining assertions that still use `page.url()`.🐛 Proposed fix
- test('should keep the main room on the top even if child has unread messages', async ({ browser }) => { + test('should keep the main room on the top even if child has unread messages', async ({ page, browser }) => { const user1Page = await browser.newPage({ storageState: Users.user1.state }); const user1Channel = new HomeChannel(user1Page);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/tests/e2e/feature-preview.spec.ts` around lines 154 - 187, The test callback in feature-preview.spec.ts no longer receives the `page` fixture, but the body still uses the bare `page` identifier in the `should keep the main room on the top even if child has unread messages` test. Update the test signature to include the `page` fixture again, or replace those `page.url()` checks with an in-scope page reference from the existing `HomeChannel`/browser setup, so the assertions in the `poHomeTeam` flow can run without a `ReferenceError`.
🧹 Nitpick comments (4)
apps/meteor/tests/e2e/team-management.spec.ts (1)
254-262: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStore the repeated locator in a variable.
The same
page.getByRole('button', { name: targetChannel }).first()locator is constructed twice (focus + assertion). Storing it once improves readability and reuse.♻️ Proposed refactor
test('should access team channel through targetTeam header', async ({ page }) => { await poHomeTeam.navbar.openChat(targetChannel); - await page.getByRole('button', { name: targetChannel }).first().focus(); - await expect(page.getByRole('button', { name: targetChannel }).first()).toBeFocused(); + const targetChannelButton = page.getByRole('button', { name: targetChannel }).first(); + await targetChannelButton.focus(); + await expect(targetChannelButton).toBeFocused(); await page.keyboard.press('Shift+Tab'); await page.keyboard.press('Space');As per coding guidelines, "Store commonly used locators in variables/constants for reuse."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/tests/e2e/team-management.spec.ts` around lines 254 - 262, The test in team-management.spec.ts constructs the same button locator twice in the “should access team channel through targetTeam header” case, which should be refactored for reuse. Update the test to store the repeated page.getByRole('button', { name: targetChannel }).first() locator in a single variable and use that variable for both the focus action and the focused assertion, keeping the rest of the test flow unchanged.Source: Coding guidelines
apps/meteor/tests/e2e/channel-management.spec.ts (1)
141-142: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract repeated locator into a variable.
The same
getByRolelocator is constructed twice in each pair (once to.focus(), once fortoBeFocused()). Storing it in a localconstavoids duplication and matches the existing pattern used elsewhere in the file (e.g.collapservariables insidebar.spec.ts).♻️ Proposed refactor
- await page.getByRole('button', { name: targetChannel }).first().focus(); - await expect(page.getByRole('button', { name: targetChannel }).first()).toBeFocused(); + const roomNameBtn = page.getByRole('button', { name: targetChannel }).first(); + await roomNameBtn.focus(); + await expect(roomNameBtn).toBeFocused();- await page.getByRole('button', { name: `Back to ${targetChannel} channel`, exact: true }).focus(); - await expect(page.getByRole('button', { name: `Back to ${targetChannel} channel`, exact: true })).toBeFocused(); + const backToChannelBtn = page.getByRole('button', { name: `Back to ${targetChannel} channel`, exact: true }); + await backToChannelBtn.focus(); + await expect(backToChannelBtn).toBeFocused();As per path instructions: "Store commonly used locators in variables/constants for reuse."
Also applies to: 164-165
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/tests/e2e/channel-management.spec.ts` around lines 141 - 142, The channel-management test repeats the same Playwright locator twice for the focus assertion. In the affected test cases, extract the `page.getByRole('button', { name: targetChannel }).first()` locator into a local constant and reuse it for both `.focus()` and `toBeFocused()`, following the same reusable-locator pattern used elsewhere in the spec file.Source: Path instructions
apps/meteor/tests/e2e/messaging.spec.ts (1)
145-145: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame repeated-locator pattern as in
channel-management.spec.ts.
page.getByRole('button', { name: targetChannel }).first()is constructed twice for the focus+assert pair at both locations. Consider extracting to a local variable for reuse, consistent with the path-instruction guidance on storing commonly used locators.Also applies to: 156-156
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/tests/e2e/messaging.spec.ts` at line 145, The repeated locator pattern in the messaging focus assertions should be simplified by reusing a single locator instead of rebuilding page.getByRole('button', { name: targetChannel }).first() multiple times. In messaging.spec.ts, extract that locator to a local variable near the affected test blocks and use it for both the focus and assertion steps, following the same reuse approach already used in channel-management.spec.ts; apply the same cleanup at both occurrences mentioned in the comment.Source: Path instructions
apps/meteor/tests/e2e/sidebar.spec.ts (1)
66-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove implementation comment.
The added comment explaining the guard's rationale conflicts with the repo guideline for Playwright spec files.
✏️ Proposed fix
await poHomeChannel.navbar.btnHome.focus(); - // Guard that focus actually landed on Home before counting tab stops; otherwise - // focus stays on <body> and the tab sequence ends one stop short of the search input. await expect(poHomeChannel.navbar.btnHome).toBeFocused();As per coding guidelines: "Avoid code comments in the implementation" (applies to
**/*.{ts,tsx,js}Playwright tests).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/tests/e2e/sidebar.spec.ts` around lines 66 - 69, Remove the implementation comment from the Playwright spec and keep only the test logic around poHomeChannel.navbar.btnHome.focus() and toBeFocused(); the issue is the inline rationale comment in sidebar.spec.ts, so delete that comment while preserving the focus guard behavior in the same test block.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/meteor/tests/e2e/feature-preview.spec.ts`:
- Around line 154-187: The test callback in feature-preview.spec.ts no longer
receives the `page` fixture, but the body still uses the bare `page` identifier
in the `should keep the main room on the top even if child has unread messages`
test. Update the test signature to include the `page` fixture again, or replace
those `page.url()` checks with an in-scope page reference from the existing
`HomeChannel`/browser setup, so the assertions in the `poHomeTeam` flow can run
without a `ReferenceError`.
---
Nitpick comments:
In `@apps/meteor/tests/e2e/channel-management.spec.ts`:
- Around line 141-142: The channel-management test repeats the same Playwright
locator twice for the focus assertion. In the affected test cases, extract the
`page.getByRole('button', { name: targetChannel }).first()` locator into a local
constant and reuse it for both `.focus()` and `toBeFocused()`, following the
same reusable-locator pattern used elsewhere in the spec file.
In `@apps/meteor/tests/e2e/messaging.spec.ts`:
- Line 145: The repeated locator pattern in the messaging focus assertions
should be simplified by reusing a single locator instead of rebuilding
page.getByRole('button', { name: targetChannel }).first() multiple times. In
messaging.spec.ts, extract that locator to a local variable near the affected
test blocks and use it for both the focus and assertion steps, following the
same reuse approach already used in channel-management.spec.ts; apply the same
cleanup at both occurrences mentioned in the comment.
In `@apps/meteor/tests/e2e/sidebar.spec.ts`:
- Around line 66-69: Remove the implementation comment from the Playwright spec
and keep only the test logic around poHomeChannel.navbar.btnHome.focus() and
toBeFocused(); the issue is the inline rationale comment in sidebar.spec.ts, so
delete that comment while preserving the focus guard behavior in the same test
block.
In `@apps/meteor/tests/e2e/team-management.spec.ts`:
- Around line 254-262: The test in team-management.spec.ts constructs the same
button locator twice in the “should access team channel through targetTeam
header” case, which should be refactored for reuse. Update the test to store the
repeated page.getByRole('button', { name: targetChannel }).first() locator in a
single variable and use that variable for both the focus action and the focused
assertion, keeping the rest of the test flow unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3a07ad5e-441c-4341-a8d7-3db67ab80630
📒 Files selected for processing (9)
apps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/feature-preview.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/rooms-join.spec.tsapps/meteor/tests/e2e/sidebar.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/video-conference.spec.ts
✅ Files skipped from review due to trivial changes (1)
- apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: Hacktron Security Check
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/tests/e2e/video-conference.spec.tsapps/meteor/tests/e2e/rooms-join.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/sidebar.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/feature-preview.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
apps/meteor/tests/e2e/video-conference.spec.tsapps/meteor/tests/e2e/rooms-join.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/sidebar.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/feature-preview.spec.ts
apps/meteor/tests/e2e/**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.spec.ts: All test files must be created inapps/meteor/tests/e2e/directory
Avoid usingpage.locator()in Playwright tests - always prefer semantic locators such aspage.getByRole(),page.getByLabel(),page.getByText(), orpage.getByTitle()
Usetest.beforeAll()andtest.afterAll()for setup/teardown in Playwright tests
Usetest.step()for complex test scenarios to improve organization in Playwright tests
Group related tests in the same file
Utilize Playwright fixtures (test,page,expect) for consistency in test files
Prefer web-first assertions (toBeVisible,toHaveText, etc.) in Playwright tests
Useexpectmatchers for assertions (toEqual,toContain,toBeTruthy,toHaveLength, etc.) instead ofassertstatements in Playwright tests
Usepage.waitFor()with specific conditions instead of hardcoded timeouts in Playwright tests
Implement proper wait strategies for dynamic content in Playwright tests
Maintain test isolation between test cases in Playwright tests
Ensure clean state for each test execution in Playwright tests
Ensure tests run reliably in parallel without shared state conflicts
Files:
apps/meteor/tests/e2e/video-conference.spec.tsapps/meteor/tests/e2e/rooms-join.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/sidebar.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/feature-preview.spec.ts
apps/meteor/tests/e2e/**/*.{ts,spec.ts}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,spec.ts}: Store commonly used locators in variables/constants for reuse
Follow Page Object Model pattern consistently in Playwright tests
Files:
apps/meteor/tests/e2e/video-conference.spec.tsapps/meteor/tests/e2e/rooms-join.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/sidebar.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/feature-preview.spec.ts
apps/meteor/tests/e2e/page-objects/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Utilize existing page objects pattern from
apps/meteor/tests/e2e/page-objects/
Files:
apps/meteor/tests/e2e/page-objects/home-channel.ts
🧠 Learnings (7)
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
apps/meteor/tests/e2e/video-conference.spec.tsapps/meteor/tests/e2e/rooms-join.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/sidebar.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/feature-preview.spec.ts
📚 Learning: 2026-02-24T19:39:42.247Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/message.ts:7-7
Timestamp: 2026-02-24T19:39:42.247Z
Learning: In RocketChat e2e tests, avoid using data-qa attributes to locate elements. Prefer semantic locators such as getByRole, getByLabel, getByText, getByTitle and ARIA-based selectors. Apply this rule to all TypeScript files under apps/meteor/tests/e2e to improve test reliability, accessibility, and maintainability.
Applied to files:
apps/meteor/tests/e2e/video-conference.spec.tsapps/meteor/tests/e2e/rooms-join.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/sidebar.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/feature-preview.spec.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/tests/e2e/video-conference.spec.tsapps/meteor/tests/e2e/rooms-join.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/sidebar.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/feature-preview.spec.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/tests/e2e/video-conference.spec.tsapps/meteor/tests/e2e/rooms-join.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/sidebar.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/feature-preview.spec.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
apps/meteor/tests/e2e/video-conference.spec.tsapps/meteor/tests/e2e/rooms-join.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/sidebar.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/feature-preview.spec.ts
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
apps/meteor/tests/e2e/video-conference.spec.tsapps/meteor/tests/e2e/rooms-join.spec.tsapps/meteor/tests/e2e/channel-management.spec.tsapps/meteor/tests/e2e/page-objects/home-channel.tsapps/meteor/tests/e2e/sidebar.spec.tsapps/meteor/tests/e2e/team-management.spec.tsapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/feature-preview.spec.ts
📚 Learning: 2025-12-16T17:29:40.430Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37834
File: apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-emoji.ts:12-22
Timestamp: 2025-12-16T17:29:40.430Z
Learning: In all page-object files under apps/meteor/tests/e2e/page-objects/, import expect from ../../utils/test (Playwright's async expect) instead of from Jest. Jest's expect is synchronous and incompatible with web-first assertions like toBeVisible, which can cause TypeScript errors.
Applied to files:
apps/meteor/tests/e2e/page-objects/home-channel.ts
🔇 Additional comments (8)
apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
104-107: LGTM!apps/meteor/tests/e2e/rooms-join.spec.ts (1)
146-148: LGTM!apps/meteor/tests/e2e/team-management.spec.ts (1)
438-442: LGTM!apps/meteor/tests/e2e/feature-preview.spec.ts (1)
109-109: LGTM!Also applies to: 145-152, 227-236, 240-240
apps/meteor/tests/e2e/channel-management.spec.ts (1)
30-31: LGTM!Also applies to: 43-44
apps/meteor/tests/e2e/messaging.spec.ts (1)
170-173: LGTM!Also applies to: 193-193
apps/meteor/tests/e2e/sidebar.spec.ts (1)
113-127: LGTM!apps/meteor/tests/e2e/video-conference.spec.ts (1)
51-52: LGTM!
sampaiodiego
force-pushed
the
test-flaky-compose-focus
branch
from
9164ccf to
ada7b97
Compare
1 participant
Proposed changes (including videos or screenshots)
Hardens three e2e assertions that race against composer focus/population, causing intermittent failures (e.g.
ArrowUptriggering edit before the composer is focused → empty input).Changes
messaging.spec.ts– assert the composer is focused beforeArrowUp(edit shortcut) and use auto-retryingtoHaveValue('msg2')instead of a one-shotinputValue()read.messaging.spec.ts– same focus guard before eachArrowUpin the message-edition stress loop.e2ee-file-encryption.spec.ts– replace one-shotinputValue()read with auto-retryingtoHaveValue('any_description')after triggering edit.Why
ArrowUp-to-edit only works when the composer already holds focus, andinputValue()samples a single instant. Both relied on timing that fails under CI load. Mirrors the existing guarded pattern inthreads.spec.ts. Test-only; no product code touched.Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Task: ARCH-2194