Skip to content

Conversation

@vaebe
Copy link
Owner

@vaebe vaebe commented Nov 24, 2025

🧪 优化 CCUI 测试套件

📝 概述

优化了 CCUI 组件库的测试套件,通过引入辅助函数、合并重复测试、使用 it.each 等方式,显著提升了代码质量和可维护性。

✨ 主要改进

代码优化

  • 减少代码量: 547 行 (-36%)
  • 优化文件数: 8 个核心测试文件
  • 移除重复测试: 9 个重复测试用例

优化的文件

  • tooltip.test.ts - 488→272 行 (-44%)
  • calendar.test.ts - 274→145 行 (-47%)
  • button.test.ts - 242→165 行 (-32%)
  • timeline.test.ts - 198→128 行 (-35%)
  • input-number.test.ts - 292→230 行 (-21%)
  • input.test.ts - 206→154 行 (-25%)
  • rate.test.ts - 159→110 行 (-31%)
  • popover.test.ts - 已优化

🔧 技术改进

1. 引入辅助函数

为每个测试文件添加了 createWrappercreateShallowWrapper 辅助函数,统一默认配置,减少重复代码。

function createWrapper(props = {}, slots = {}) {
  return mount(Component, { props, slots })
}

2. 使用 it.each 合并相似测试

将多个相似测试合并为参数化测试,提高可读性和可维护性。

it.each([
  ['primary', '.node--primary'],
  ['success', '.node--success'],
  // ...
])('should render with type %s', (type, expectedClass) => {
  // 测试逻辑
})

3. 合并关联测试

将相关的测试用例合并为完整的工作流测试,例如:

  • 合并 focus/blur 事件测试
  • 合并 click 事件测试(正常/禁用/加载中)
  • 合并月份导航测试(上一月/下一月/今天)

4. 移除重复测试

识别并移除了真正重复的测试用例:

  • input-number.test.ts 中的重复精度测试
  • input-number.test.ts 中的重复边界测试

5. 代码质量提升

  • 修复 Vue 3 兼容性问题($onaddEventListener
  • 移除冗余注释
  • 简化 nextTick 使用
  • 统一代码风格

✅ 测试结果

  • 测试通过率: 231/231 (100%) ✓
  • 测试覆盖率: 100% 保持不变 ✓
  • 执行时间: ~2.0s

📊 影响范围

  • ✅ 无破坏性变更
  • ✅ 所有测试通过
  • ✅ 测试覆盖率保持 100%
  • ✅ 提升代码可维护性

🔍 Review 重点

  1. 辅助函数的实现是否合理
  2. it.each 的使用是否恰当
  3. 合并后的测试是否仍然清晰易懂
  4. 是否有遗漏的测试场景

类型: 测试优化
影响: 测试代码
风险:

Summary by CodeRabbit

  • Tests
    • Refactored test infrastructure across multiple UI components to improve maintainability and reduce code duplication through standardized test helpers and setup functions. No changes to component functionality or end-user features.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Nov 24, 2025

Walkthrough

This pull request systematically refactors test suites across nine UI components to improve maintainability. Each test file introduces helper functions (e.g., createWrapper, createShallowWrapper) to centralize component mounting logic, replacing repeated direct mount/shallowMount calls with standardized wrapper factories that consolidate prop/slot provisioning and reduce boilerplate throughout the tests.

Changes

Cohort / File(s) Change Summary
Input/Form Control Tests
packages/ccui/ui/input/test/input.test.ts, packages/ccui/ui/input-number/test/input-number.test.ts
Introduced createWrapper and createShallowWrapper helpers to centralize mounting; replaced direct mounts with helpers across all test cases; consolidated event and prop-related test setup; added new custom step test for InputNumber.
Display/Container Component Tests
packages/ccui/ui/popover/test/popover.test.ts, packages/ccui/ui/tooltip/test/tooltip.test.ts
Introduced createWrapper/createShallowWrapper helpers with default teleported=false and Trigger slots; replaced individual test setups with wrappers; refactored themed, placement, and interaction tests using it.each parameterization; standardized element/style assertions via wrapper instances.
Selection/Information Display Tests
packages/ccui/ui/button/test/button.test.ts, packages/ccui/ui/rate/test/rate.test.ts
Introduced createWrapper and createShallowWrapper helpers; replaced direct mount usages with helpers across tests; maintained existing assertions for clicks, props (type, size, round, circle, plain), slots, and rating interactions.
Date/Timeline/Slider Tests
packages/ccui/ui/calendar/test/calendar.test.ts, packages/ccui/ui/timeline/test/timeline.test.ts, packages/ccui/ui/slider/test/slider.test.ts
Introduced createWrapper helpers (Calendar/Timeline) and removed beforeEach setup with centralized createMockRect helper (Slider); replaced repeated mounts with wrapper factories; converted async tests to synchronous where applicable; refactored calendar month navigation and timeline item type tests to use it.each parameterization.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Reason: Heterogeneous refactoring across nine distinct test files with consistent pattern (wrapper helpers), but each component's test suite requires individual verification to ensure functional coverage is preserved. The scope is broad (multiple files) but changes follow a repeatable pattern, reducing per-file complexity.
  • Areas requiring attention:
    • Verify that all original test assertions remain functionally equivalent after replacing direct mount calls with helper wrappers.
    • Confirm that parameterized tests (it.each conversions in Calendar, Timeline, Slider) cover the same scenarios as original inline tests.
    • Check that helper defaults (e.g., teleported=false in Popover/Tooltip, timestamp in Timeline) do not mask unintended behavior or mask edge cases.
    • Validate that async→sync conversions in Slider tests (removal of beforeEach) do not affect timing-sensitive assertions or DOM interactions.

Possibly related PRs

  • feat: 增加 input 组件 #13: Modifies the same input test file; main PR refactors Input tests with wrapper helpers while the referenced PR introduces the initial Input component and tests, making them directly related at the test-file level.
  • feat: timeline #9: Directly related to the Timeline test refactoring; the main PR's createTimelineWrapper/createItemWrapper helpers refactor the timeline.test.ts introduced in the referenced PR.
  • feat: popover #7: Main PR's refactoring of Popover tests (adding createWrapper/createShallowWrapper) directly modifies the Popover test suite behavior introduced in the referenced PR, making them code-level related.

Poem

🐰 Test helpers bloom in nine neat rows,
Boilerplate fades as abstraction grows,
Wrappers unite what was scattered wide,
Mount and assert with standardized pride—
Our fuzzy paws refactor with glee!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 53.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title refers to test refactoring and optimization, which aligns with the main changes across all 8 test files that introduce helper wrappers and consolidate test setup.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat-poppver-test

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
packages/ccui/ui/slider/test/slider.test.ts (3)

437-450: input change with invalid value test no longer exercises an invalid-path scenario

The test name suggests verifying Slider’s behavior when CInputNumber emits an invalid value, but the body only asserts that CInputNumber exists — something already covered by other tests. As written, this is effectively a no-op and can mislead future readers about what’s covered.

Consider either:

  • Emitting a clearly invalid value (per your component’s spec) and asserting the expected clamping/ignore behavior, or
  • Removing/renaming the test if there’s no distinct invalid-input behavior to assert.

490-536: Input control button tests are too weak for their names

The three tests:

  • event - input increase button
  • event - input decrease button
  • event - input buttons disabled at boundaries

currently only assert that CInputNumber exists. They don’t simulate clicks on the +/- controls, don’t assert emitted values, and don’t verify disabled states at bounds, so they don’t actually cover the behaviors implied by their names and are largely redundant with other existence checks.

It would be better to either:

  • Drive the actual interactions (e.g., trigger increase/decrease events or button clicks and assert the resulting update:modelValue/clamping/disabled behavior), or
  • Remove these tests or narrow their descriptions if the intended scenarios are covered elsewhere.

As-is, they give a false sense of coverage around the input control buttons.


615-621: Test name does not match persistent prop value

In it('props - persistent false with showTooltip false', ...), the props set persistent: true while the description says “persistent false”. That mismatch is confusing and can mislead future maintainers about the scenario under test.

Either flip the prop to persistent: false or adjust the test name to reflect that this case is “persistent true + showTooltip false”.

🧹 Nitpick comments (6)
packages/ccui/ui/slider/test/slider.test.ts (2)

15-23: createMockRect helper nicely centralizes rect mocking

Extracting the DOMRect-like object into createMockRect reduces duplication and keeps click/drag tests consistent. If you ever start relying on x/y fields, you can extend this helper without touching individual tests.


36-46: Synchronous props - modelValue test is correct and clearer

This test only inspects wrapper.vm.currentValue, so dropping the unnecessary async wrapper is appropriate and keeps the test simpler.

You might also consider introducing a createWrapper helper for Slider (similar to other suites in this PR) to DRY up repeated mount(Slider, { props }) and wrapper.unmount() calls, but it’s not required for correctness.

packages/ccui/ui/input/test/input.test.ts (4)

14-21: Helper wrappers keep tests DRY and focused

Centralizing mount/shallowMount into createWrapper/createShallowWrapper reads well and reduces boilerplate. If you ever want a bit more type safety, you could lightly type the props parameter, e.g.:

-function createWrapper(props = {}) {
+function createWrapper(props: Record<string, any> = {}) {
   return mount(Input, { props })
 }

-function createShallowWrapper(props = {}) {
+function createShallowWrapper(props: Record<string, any> = {}) {
   return shallowMount(Input, { props })
 }

Not required, but it can help catch prop name typos in tests.


25-79: Nice consolidation of simple prop/DOM tests onto createShallowWrapper

Switching the “dom / type / size / placeholder / disabled / readonly / clearable / prepend / append” tests to createShallowWrapper keeps them very concise while still asserting the same behaviour (including the disabled class logic via ns.m('disabled').substring(1)).

One minor nit: only the 'dom' test explicitly calls wrapper.unmount(), while the others rely on test-utils cleanup. For consistency, consider either:

  • dropping explicit unmount() everywhere and relying on the global cleanup, or
  • adding wrapper.unmount() to the remaining tests as well.

Purely stylistic — behaviour is fine as-is.


100-110: Combined focus/blur test is compact and still readable

Merging focus and blur into it('emits focus and blur events', ...) with a shared wrapper and separate spies looks good and keeps the flow linear: focus → assertion → blur → assertion. The tradeoff in diagnostics (a single test covers both events) seems acceptable given the reduced duplication.


126-144: Password visibility toggle test is solid; optional stronger assertion

The password visibility test drives the icon toggling via clicks and asserts the correct BEM element switches between password-hidden and password-visible, which is good coverage for the visual affordance.

If you want slightly stronger guarantees, you could also assert the <input> type attribute flips between 'password' and 'text' alongside the icon expectations, but that’s an optional enhancement.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 07cd26c and 9656c62.

📒 Files selected for processing (9)
  • packages/ccui/ui/button/test/button.test.ts (2 hunks)
  • packages/ccui/ui/calendar/test/calendar.test.ts (1 hunks)
  • packages/ccui/ui/input-number/test/input-number.test.ts (10 hunks)
  • packages/ccui/ui/input/test/input.test.ts (1 hunks)
  • packages/ccui/ui/popover/test/popover.test.ts (16 hunks)
  • packages/ccui/ui/rate/test/rate.test.ts (3 hunks)
  • packages/ccui/ui/slider/test/slider.test.ts (10 hunks)
  • packages/ccui/ui/timeline/test/timeline.test.ts (3 hunks)
  • packages/ccui/ui/tooltip/test/tooltip.test.ts (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
packages/ccui/ui/popover/test/popover.test.ts (1)
packages/ccui/ui/popover/index.ts (1)
  • Popover (8-8)
packages/ccui/ui/rate/test/rate.test.ts (1)
packages/ccui/ui/rate/index.ts (1)
  • Rate (8-8)
packages/ccui/ui/timeline/test/timeline.test.ts (1)
packages/ccui/ui/timeline/index.ts (2)
  • Timeline (10-10)
  • TimelineItem (10-10)
packages/ccui/ui/input-number/test/input-number.test.ts (1)
packages/ccui/ui/input-number/index.ts (1)
  • InputNumber (8-8)
🔇 Additional comments (20)
packages/ccui/ui/input-number/test/input-number.test.ts (3)

6-11: LGTM! Clean abstraction for test setup.

The helper function effectively reduces boilerplate and centralizes mounting logic. This makes tests more readable and easier to maintain, aligning well with the PR's standardization goals.


165-171: Good addition for decimal step coverage.

This test nicely complements the existing step test by verifying decimal increments with precision, ensuring fractional steps work correctly.


14-183: Excellent systematic refactoring!

All tests have been consistently migrated to use the helper function while preserving their original logic and coverage. The refactoring successfully reduces boilerplate (21% line reduction) without compromising test quality or coverage.

packages/ccui/ui/rate/test/rate.test.ts (1)

10-17: LGTM! Clean helper function implementation.

The test helpers follow a consistent pattern with the rest of the PR, centralizing component mounting logic and providing a clear API for props and slots configuration.

packages/ccui/ui/tooltip/test/tooltip.test.ts (2)

6-31: LGTM! Well-designed helper functions.

The helpers provide sensible defaults (teleported: false and a default trigger button) that work for most test scenarios while allowing overrides. This significantly reduces boilerplate across the test suite.


96-107: Excellent use of parameterized testing.

Using it.each to test multiple theme variations reduces duplication and makes it easy to add new themes in the future.

packages/ccui/ui/button/test/button.test.ts (2)

21-40: LGTM! Consistent helper implementation.

The helpers follow the same pattern as other component tests in this PR, providing a default slot content while allowing customization.


85-115: Excellent comprehensive click event testing.

This test thoroughly covers all click scenarios: normal clicks, disabled state preventing clicks, and loading state preventing clicks. The use of addEventListener is appropriate for testing native DOM event behavior and aligns with the Vue 3 compatibility improvements mentioned in the PR objectives.

packages/ccui/ui/timeline/test/timeline.test.ts (2)

6-22: LGTM! Smart dual-helper approach.

Providing separate helpers for Timeline and TimelineItem is a good design choice. The createItemWrapper includes sensible defaults (timestamp and content) that work for most tests while remaining flexible for customization.


60-69: Great use of parameterized testing.

Converting multiple type tests to a data-driven approach with it.each makes the test suite more maintainable and easier to extend with new types.

packages/ccui/ui/popover/test/popover.test.ts (2)

6-31: LGTM! Consistent helper implementation.

The helpers mirror the approach used in tooltip.test.ts with sensible defaults that work for most scenarios while allowing flexibility.


111-122: Excellent parameterization.

Using it.each for theme and arrow visibility tests reduces duplication and improves maintainability.

Also applies to: 139-150

packages/ccui/ui/calendar/test/calendar.test.ts (2)

13-27: LGTM! Helper handles global component registration.

The helper appropriately includes global component registration for CButton, which is necessary for Calendar's internal button usage. The default modelValue ensures the component is always in a valid state.


65-87: Good consolidation of navigation tests.

Merging the month navigation tests into a single flow improves readability while maintaining comprehensive coverage. The approach of re-fetching buttons before each interaction (lines 70, 75, 80) is correct, as the DOM updates between clicks.

packages/ccui/ui/slider/test/slider.test.ts (4)

2-2: Vitest import cleanup is appropriate

Removing beforeEach from the vitest import aligns with the per-test mount/unmount pattern now used; no behavioral impact.


240-250: Updated showStops expectation looks consistent with defaults

With an implied 0–100 range and step: 10, expecting 11 stops (including both endpoints) is internally consistent with the component behavior.


264-279: Click event test correctly reuses createMockRect

Using vi.spyOn(...getBoundingClientRect).mockReturnValue(createMockRect()) keeps the click-position math under test while avoiding hard-coded rectangles in multiple places. Assertions on both update:modelValue and input emissions remain intact.


317-329: Drag event test keeps behavior, now with shared rect helper

Reusing createMockRect for sliderRef.getBoundingClientRect preserves the drag behavior under test while centralizing the geometry setup. Checking wrapper.vm.isDragging after mousedown still verifies the drag entry path.

packages/ccui/ui/input/test/input.test.ts (2)

81-98: Event tests for input and change are clearer with createWrapper

Using createWrapper plus onInput / onChange handlers and driving via setValue/trigger('change') mirrors real consumer usage nicely. The expectations are straightforward (toBeCalled()), and the explicit wrapper.unmount() keeps these full-mount tests tidy.


112-124: Clearable behaviour test covers both callback and DOM state

The clearable scenario is well covered: you assert that onClear is invoked and that the underlying <input> value is reset to '' after clicking the clear icon. Using createWrapper with the minimal prop set (clearable, modelValue, onClear) keeps the intent very clear.

expect(hide).toHaveBeenCalled()
})

it('aRIA 属性', async () => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix typo in test name.

The test name contains "aRIA" which should be "ARIA" (all caps).

Apply this diff:

-    it('aRIA 属性', async () => {
+    it('ARIA 属性', async () => {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('aRIA 属性', async () => {
it('ARIA 属性', async () => {
🤖 Prompt for AI Agents
In packages/ccui/ui/popover/test/popover.test.ts around line 259, the test name
contains a typo "aRIA 属性" — change the test description string to use the
correct casing "ARIA 属性" (replace "aRIA" with "ARIA") so the test name reads
"ARIA 属性". Ensure only the visible test title is edited and no other code or
formatting is changed.

},
})

it('aRIA 属性', async () => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix typo in test name.

The test name contains "aRIA" which should be "ARIA" (all caps) as it's an acronym for Accessible Rich Internet Applications.

Apply this diff:

-    it('aRIA 属性', async () => {
+    it('ARIA 属性', async () => {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('aRIA 属性', async () => {
it('ARIA 属性', async () => {
🤖 Prompt for AI Agents
In packages/ccui/ui/tooltip/test/tooltip.test.ts around line 249, the test title
contains a typo "aRIA 属性"; change the test name to use the correct uppercase
acronym "ARIA" (e.g., replace "aRIA 属性" with "ARIA 属性") so the test description
correctly reflects the ARIA acronym.

@vaebe vaebe merged commit 91589e5 into main Nov 24, 2025
4 checks passed
@vaebe vaebe deleted the feat-poppver-test branch November 24, 2025 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants