test: add layout component e2e test cases#372
Conversation
WalkthroughThis PR adds a full Playwright-based E2E test suite for a ChangesLayout E2E Test Suite
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
SonyLeo
changed the title
|
✅ Preview build completed successfully!
|
📦 Package Previewpnpm add https://pkg.pr.new/@opentiny/tiny-robot@43dd097 pnpm add https://pkg.pr.new/@opentiny/tiny-robot-kit@43dd097 pnpm add https://pkg.pr.new/@opentiny/tiny-robot-svgs@43dd097 commit: 43dd097 |
SonyLeo
force-pushed
the
feat/layout-e2e-test
branch
from
a8d643e to
9aea032
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
packages/test/src/layout/specs/aside.spec.ts (1)
61-74: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winLoose post-resize width assertions could miss a no-op resize.
After an explicit
resizeAside('left', 160, ...)/resizeAside('right', -160, ...), the width checks usetoBeGreaterThanOrEqual(before...)(lines 71, 86), which would still pass even if the resize produced zero net change. The start/end phase and count assertions (lines 69-70, 72-74, 84-85, 87-89) partially cover this, but a strict bound (or asserting the delta magnitude) would better catch a regression where drag input is registered but width never actually updates.Also applies to: 76-89
🤖 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 `@packages/test/src/layout/specs/aside.spec.ts` around lines 61 - 74, The post-resize width checks in aside.spec.ts are too permissive and can pass even when resize is a no-op. In the resize tests around resizeAside('left', 160) and resizeAside('right', -160), tighten the assertions on harness.widths.left and harness.widths.right so they verify an actual size change relative to the pre-resize values, rather than just using a greater-than-or-equal check. Keep the existing phase/count checks, but make the width assertion in the resize test block explicitly validate the expected delta using the resize helpers and harness.readHarness() results.packages/test/src/layout/specs/main-scrollbar.spec.ts (1)
30-54: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated drag mechanics.
This test reimplements the down/move/up hover+boundingBox flow that
LayoutTestPage.dragByalready encapsulates (seeLayoutTestPage.ts:335-348), duplicating logic just to insert a mid-drag assertion. Consider adding a small helper (e.g.,pressAndHold/dragBywith an optional callback) to avoid re-deriving drag coordinates here.🤖 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 `@packages/test/src/layout/specs/main-scrollbar.spec.ts` around lines 30 - 54, The scrollbar drag test is duplicating the same hover/bounding-box/mouse down-up flow already handled by LayoutTestPage.dragBy. Refactor the test to reuse LayoutTestPage.dragBy, or extend it with a small optional hook/callback so the mid-drag assertion can run without re-deriving coordinates in main-scrollbar.spec.ts. Keep the drag mechanics centralized in LayoutTestPage and only express the assertion-specific behavior in the spec.packages/test/src/layout/helpers/LayoutTestPage.ts (1)
91-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider scoping
main,scrollbar,scrollbarThumb,backdroplike their sibling getters.
getAside,getAsideContent,getResizeTrigger, andgetBackdropall accept an optionalscopeparameter defaulting tothis.page, butmain/scrollbar/scrollbarThumb/backdropare hardcoded tothis.page. Once multipleTrLayoutsurfaces are visible simultaneously (aside/floating/css-var fixtures render as siblings, not replacements), these unscoped locators risk matching more than one element and failing Playwright's strict-mode single-match requirement.🤖 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 `@packages/test/src/layout/helpers/LayoutTestPage.ts` around lines 91 - 109, Update the LayoutTestPage locator getters so they can be scoped like the sibling helpers. The hardcoded page-wide locators in main, scrollbar, scrollbarThumb, and backdrop should accept an optional scope parameter defaulting to this.page, matching getAside, getAsideContent, getResizeTrigger, and getBackdrop. Use that scope when calling locator(...) so LayoutTestPage can target a specific TrLayout surface and avoid strict-mode multi-match failures.packages/test/src/layout/specs/floating.spec.ts (1)
206-227: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueManual mouse simulation duplicates drag/resize logic already encapsulated in the helper.
This test bypasses
resizeSurface/dragSurfaceand hand-rollsmouse.move/down/moveto inspect mid-gesture state, duplicating positioning/step logic used elsewhere in the file. If mid-gesture inspection is a recurring need, consider exposing a helper (e.g., astartResize/stepResize/endResizesequence onLayoutTestPage) so other specs can assert intermediate states without re-implementing raw mouse mechanics.🤖 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 `@packages/test/src/layout/specs/floating.spec.ts` around lines 206 - 227, The test in floating.spec.ts is re-implementing raw mouse drag/resize steps instead of using the existing LayoutTestPage helpers. Refactor the interaction setup around getFloatingResizeTrigger and the drag bar into reusable helper methods on LayoutTestPage (for example a start/step/end resize flow, or equivalent dragSurface/resizeSurface support for mid-gesture assertions), then update this spec to use that helper so intermediate state checks can be expressed without duplicating mouse mechanics.
🤖 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.
Inline comments:
In `@packages/test/src/layout/fixtures/LayoutCssVarFixtures.vue`:
- Around line 165-179: The scroll target in LayoutCssVarFixtures.vue reuses the
same data-testid as the main layout harness, which causes locator collisions
when both fixtures are mounted together. Update the TrLayout main slot markup
for the scrollbar fixture to use a fixture-specific testid instead of
layout-scroll-target, and keep the ref and TrLayout.ProxyScrollbar wiring intact
so this fixture remains independently addressable.
In `@packages/test/src/layout/helpers/LayoutTestPage.ts`:
- Around line 350-366: The startX calculation in dragResizeHandle is redundant
because both branches of the side ternary are identical. Update the logic in
dragResizeHandle to either remove the conditional if the same edge is always
intended, or make the left and right branches return different starting
positions to match the behavior of dragFloatingResizeHandle and the intended
resize direction.
In `@packages/test/src/layout/selectors.ts`:
- Around line 5-8: The aside content selectors are targeting the outer
`.tr-layout__aside` instead of the nested content wrapper used by the fixture.
Update `leftAsideContent` and `rightAsideContent` in `selectors.ts` to point at
the inner `.layout-demo__aside-content` elements tied to the `left-aside-slot`
and `right-aside-slot` test IDs, or remove the redundant helpers if
`getAsideContent` is not needed.
In `@packages/test/src/layout/specs/aside.spec.ts`:
- Line 267: The drawer toggle interaction in aside.spec.ts is using a synthetic
DOM click via evaluate(...click()), which bypasses Playwright actionability
checks and can hide real occlusion/interactivity issues. Update the drawer
toggle in the relevant test to use the locator’s native click path, matching the
other interactions in this spec (for example the same pattern used around
fixture.getByTestId and related drawer toggles), so the test exercises real
user-visible behavior and fails if the element is not actually clickable.
- Line 67: The test file uses Array.prototype.at(-1), which is not available in
the package’s ES2020 type libs and will break typechecking. Replace the
last-item access in aside.spec.ts with an ES2020-compatible approach such as
slice(-1)[0] at leftEndLog and every other .at(-1) usage in this file, keeping
the existing test logic unchanged.
In `@packages/test/src/layout/specs/floating.spec.ts`:
- Line 146: The floating spec uses Array.prototype.at(-1), but packages/test
only targets ES2020 libs so these calls won’t type-check. Update
floating.spec.ts by either adding the ES2022 array lib in the test tsconfig or
replacing the .at(-1) usages in the affected assertions with ES2020-compatible
last-item access, keeping the references in harness.logs.floatingDrag and the
related log arrays consistent.
---
Nitpick comments:
In `@packages/test/src/layout/helpers/LayoutTestPage.ts`:
- Around line 91-109: Update the LayoutTestPage locator getters so they can be
scoped like the sibling helpers. The hardcoded page-wide locators in main,
scrollbar, scrollbarThumb, and backdrop should accept an optional scope
parameter defaulting to this.page, matching getAside, getAsideContent,
getResizeTrigger, and getBackdrop. Use that scope when calling locator(...) so
LayoutTestPage can target a specific TrLayout surface and avoid strict-mode
multi-match failures.
In `@packages/test/src/layout/specs/aside.spec.ts`:
- Around line 61-74: The post-resize width checks in aside.spec.ts are too
permissive and can pass even when resize is a no-op. In the resize tests around
resizeAside('left', 160) and resizeAside('right', -160), tighten the assertions
on harness.widths.left and harness.widths.right so they verify an actual size
change relative to the pre-resize values, rather than just using a
greater-than-or-equal check. Keep the existing phase/count checks, but make the
width assertion in the resize test block explicitly validate the expected delta
using the resize helpers and harness.readHarness() results.
In `@packages/test/src/layout/specs/floating.spec.ts`:
- Around line 206-227: The test in floating.spec.ts is re-implementing raw mouse
drag/resize steps instead of using the existing LayoutTestPage helpers. Refactor
the interaction setup around getFloatingResizeTrigger and the drag bar into
reusable helper methods on LayoutTestPage (for example a start/step/end resize
flow, or equivalent dragSurface/resizeSurface support for mid-gesture
assertions), then update this spec to use that helper so intermediate state
checks can be expressed without duplicating mouse mechanics.
In `@packages/test/src/layout/specs/main-scrollbar.spec.ts`:
- Around line 30-54: The scrollbar drag test is duplicating the same
hover/bounding-box/mouse down-up flow already handled by LayoutTestPage.dragBy.
Refactor the test to reuse LayoutTestPage.dragBy, or extend it with a small
optional hook/callback so the mid-drag assertion can run without re-deriving
coordinates in main-scrollbar.spec.ts. Keep the drag mechanics centralized in
LayoutTestPage and only express the assertion-specific behavior in the spec.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0ce1a6e7-6891-4c4f-adb0-51541ff5a46b
📒 Files selected for processing (13)
packages/test/src/App.vuepackages/test/src/layout/fixtures/AsideStateFixtures.vuepackages/test/src/layout/fixtures/FloatingStateFixtures.vuepackages/test/src/layout/fixtures/LayoutCssVarFixtures.vuepackages/test/src/layout/helpers/LayoutTestPage.tspackages/test/src/layout/helpers/index.tspackages/test/src/layout/index.vuepackages/test/src/layout/selectors.tspackages/test/src/layout/specs/aside.spec.tspackages/test/src/layout/specs/css-vars.spec.tspackages/test/src/layout/specs/floating.spec.tspackages/test/src/layout/specs/main-scrollbar.spec.tspackages/test/src/layout/specs/structure.spec.ts
…r layout components
1 participant
背景
本次变更补充
Layout组件的 E2E 测试覆盖。变更范围
本次仅涉及
packages/test下的 Layout E2E 测试相关代码，主要包括：packages/test/src/App.vuepackages/test/src/layout/index.vuepackages/test/src/layout/helpers/*packages/test/src/layout/fixtures/*packages/test/src/layout/specs/*packages/test/package.json当前覆盖项
当前已覆盖：
用例矩阵
header / footer / left-aside / right-aside插槽渲染structure.spec.tsstructure.spec.tsLayout.AsideToggle插槽参数isOpenstructure.spec.tsclass / id / data-*透传到 surfacestructure.spec.tsstructure.spec.tsdock / drawer模式切换aside.spec.tsopen受控更新驱动 UIaside.spec.tsaside.spec.tscollapseEffect=slide行为契约aside.spec.tsaside-resize*三阶段事件对齐aside.spec.tsaside.spec.tscollapsedWidth=0后完全隐藏aside.spec.tsresizable=false时隐藏 resize triggeraside.spec.tsopen但父级不回写时仅发事件、不自改 UIaside.spec.tsexpandedWidth但父级不回写时仅发事件、不自改宽度aside.spec.tsdefaultOpen / defaultExpandedWidth初始化aside.spec.tsaside.spec.tsdefault*不重新同步aside.spec.tsmin/max边界aside.spec.tsopen / expandedWidth = undefined时回退为非受控语义aside.spec.tsaside.spec.tsnormal / floating模式切换floating.spec.tsplacement初始定位floating.spec.tsfloating-drag*三阶段事件对齐floating.spec.tsfloating.spec.tsfloating.spec.tsdraggable=false后禁止拖动floating.spec.tsfloating.spec.tsresizable=false后隐藏 7 个 resize handlefloating.spec.tsfloating-resize*三阶段事件对齐floating.spec.tsfloating.spec.tsfloatingState但父级不回写时仅发事件、不自改位置尺寸floating.spec.tsdefaultFloatingState初始化与交互更新floating.spec.tsdefaultFloatingState不重新同步floating.spec.tsfloatingState = undefined时回退为非受控语义floating.spec.tsminWidth / maxWidth缩放边界约束floating.spec.tsscrollTarget解析真实滚动宿主main-scrollbar.spec.tsmain-scrollbar.spec.tsmain-scrollbar.spec.tsmain-scrollbar.spec.tsmain-scrollbar.spec.tsmain-scrollbar.spec.tsmain-scrollbar.spec.ts--tr-layout-height / --tr-layout-bg生效css-vars.spec.tscss-vars.spec.ts--tr-layout-main-min-width生效css-vars.spec.tscss-vars.spec.ts--tr-layout-drawer-width生效css-vars.spec.tscss-vars.spec.tscss-vars.spec.tscss-vars.spec.tsFixture 对应
AsideStateFixtures.vueundefined回退非受控场景FloatingStateFixtures.vueundefined回退非受控场景LayoutCssVarFixtures.vue自检项
是否包含破坏性变更
Summary by CodeRabbit
New Features
Bug Fixes