Skip to content

fix(sender-e2e): stabilize template delete e2e case#317

Merged
hexqi merged 3 commits intoopentiny:developfrom
SonyLeo:fix/sender-template-e2e-cases
Mar 30, 2026
Merged

fix(sender-e2e): stabilize template delete e2e case#317
hexqi merged 3 commits intoopentiny:developfrom
SonyLeo:fix/sender-template-e2e-cases

Conversation

@SonyLeo
Copy link
Copy Markdown
Collaborator

@SonyLeo SonyLeo commented Mar 30, 2026

一、问题背景

Sender 模板块删除场景中的 TC-DL-06 在 CI 环境下存在偶发失败。根因是该用例原先依赖 Home + ArrowRight 这类键盘步进方式来定位光标,再通过外层键盘事件触发 Delete,对浏览器选区行为和执行时序较为敏感,在不同环境下容易出现光标未准确落位的问题,从而导致未命中预期的模板块删除逻辑。

image

二、 优化方向

本次优化将“光标定位 + Delete 触发”收敛为基于 Sender 内部 editor 的浏览器侧原子操作,不再依赖外层键盘步进来猜测光标位置。这样可以更稳定地命中模板块左侧删除场景,降低 ProseMirror/Tiptap 选区差异带来的 flaky 风险,同时保留该测试场景的覆盖。

变更内容

  • 优化模板块左侧 Delete 场景的测试实现,不再依赖 Home + ArrowRight 这类对浏览器选区行为敏感的键盘步进。
  • 在 Sender 测试页中补充基于 editor 的稳定删除桥接能力,将“光标定位 + Delete 触发”收敛为浏览器侧原子操作。
  • template-helper 中补充对应的稳定辅助方法和文本断言能力,用于提升模板块边界场景下的断言可靠性。
  • 调整 TC-DL-05、恢复并稳定 TC-DL-06 的实现,使其更贴近模板块删除逻辑的真实触发路径。

三、 当前现状

验证情况

  • pnpm -F tiny-robot-test test -- --workers=1 src/sender/specs/template/delete.spec.ts

  • CI=1、单 worker 模式下连续运行 5 轮 template/delete.spec.ts,均通过

  • CI=1、单 worker 模式下运行 src/sender/specs70 passed

  • pnpm -F tiny-robot-test test , 79 passed

运行截图

【git-action】

image

【本地】

整体测试
image

相关测试
image

Summary by CodeRabbit

  • Tests
    • Updated Playwright specs to use a new helper for removing empty template blocks, simplifying steps and improving reliability.
    • Improved editor-text normalization in assertions to reduce flakiness with invisible/whitespace characters.
    • Adjusted test assertions accordingly to reflect normalized text comparisons.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 30, 2026

Walkthrough

Replaced manual cursor/keypress sequences in template-deletion tests with a new helper-driven operation (pressDeleteBeforeTemplate) and added editor-text normalization helpers; implemented a page-global test API (window.__senderTestApi) in the sender component that moves the cursor before a template and synthesizes Delete key events.

Changes

Cohort / File(s) Summary
Template specs
packages/test/src/sender/specs/template/delete.spec.ts
Rewrote deletion steps to use pressDeleteBeforeTemplate(0) helper; removed manual loops & fallback presses; replaced a containment assertion with expectNormalizedEditorText('我是,来自').
Test helpers
packages/test/src/sender/helpers/template-helper.ts
Added normalizeEditorText() (internal), pressDeleteBeforeTemplate(index) and expectNormalizedEditorText(text) to test helper; normalization strips ZWSP, converts NBSP, collapses whitespace; expectNormalizedEditorText polls until exact match.
Sender runtime (page) API
packages/test/src/sender/index.vue
Added window.__senderTestApi exposing moveCursorBeforeTemplate(index) and pressDeleteBeforeTemplate(index) on mount and removed on unmount; implemented cursor positioning via doc.descendants and synthesized bubbling/cancelable Delete keydown dispatch to editor DOM.

Sequence Diagram(s)

sequenceDiagram
    participant Test as Playwright Test
    participant PageHelper as template-helper
    participant WinAPI as window.__senderTestApi
    participant Sender as sender component (ProseMirror)
    participant EditorDOM as Editor DOM

    Test->>PageHelper: call pressDeleteBeforeTemplate(0)
    PageHelper->>WinAPI: page.evaluate -> pressDeleteBeforeTemplate(0)
    WinAPI->>Sender: moveCursorBeforeTemplate(0) (walk doc.descendants)
    Sender->>EditorDOM: set selection / focus at position before template
    WinAPI->>EditorDOM: synthesize keydown Delete (bubbling, cancelable)
    EditorDOM-->>WinAPI: dispatchEvent result (canceled or not)
    WinAPI-->>PageHelper: return boolean success
    PageHelper-->>Test: assert returned true, optionally poll normalized text
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I hopped to the editor, quick and spry,

pressed a single key and watched loops fly by.
ZWSPs cleaned, templates tamed, tests now neat,
A carrot of code—succinct and sweet.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title 'fix(sender-e2e): stabilize template delete e2e case' directly relates to the main change: replacing manual cursor-navigation operations with a single helper-driven operation to stabilize flaky E2E tests for template deletion.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 30, 2026

✅ Preview build completed successfully!

Click the image above to preview.
Preview will be automatically removed when this PR is closed.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 30, 2026

@SonyLeo SonyLeo force-pushed the fix/sender-template-e2e-cases branch from 452ef54 to 3c19a1b Compare March 30, 2026 08:39
@SonyLeo SonyLeo changed the title fix(sender-e2e): stabilize template delete e2e coverage fix(sender-e2e): remove flaky template delete case Mar 30, 2026
@SonyLeo SonyLeo marked this pull request as ready for review March 30, 2026 08:59
Copy link
Copy Markdown

@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.

🧹 Nitpick comments (1)
packages/test/src/sender/index.vue (1)

7-14: Return structured results from __senderTestApi.

This hook is a good stabilization strategy, but false currently conflates “editor unavailable”, “template index not found”, “cursor placement failed”, and “Delete not handled”. That leaves packages/test/src/sender/helpers/template-helper.ts:128-133 with only a generic expected false to be true when something regresses. Returning { ok, reason } here would keep the deterministic path and make the next failure much faster to triage.

💡 Possible adjustment
 declare global {
   interface Window {
     __senderTestApi?: {
-      moveCursorBeforeTemplate: (index: number) => boolean
-      pressDeleteBeforeTemplate: (index: number) => boolean
+      moveCursorBeforeTemplate: (index: number) => { ok: boolean; reason?: string }
+      pressDeleteBeforeTemplate: (index: number) => { ok: boolean; reason?: string }
     }
   }
 }
 
 const moveCursorBeforeTemplate = (index: number) => {
   const exposedEditor = senderRef.value?.editor
   const editor = exposedEditor?.value ?? exposedEditor
-  if (!editor) return false
+  if (!editor) return { ok: false, reason: 'editor is unavailable' }
 
   let currentIndex = 0
   let templatePosition: number | null = null
@@
-  if (templatePosition === null) return false
+  if (templatePosition === null) {
+    return { ok: false, reason: `template index ${index} was not found` }
+  }
 
   editor.commands.setTextSelection(templatePosition)
   editor.commands.focus(templatePosition)
 
-  return editor.state.selection.$from.nodeAfter?.type?.name === 'templateBlock'
+  const ok = editor.state.selection.$from.nodeAfter?.type?.name === 'templateBlock'
+  return ok ? { ok } : { ok: false, reason: 'cursor was not placed before the template' }
 }
 
 const pressDeleteBeforeTemplate = (index: number) => {
   const exposedEditor = senderRef.value?.editor
   const editor = exposedEditor?.value ?? exposedEditor
-  if (!editor || !moveCursorBeforeTemplate(index)) return false
+  if (!editor) return { ok: false, reason: 'editor is unavailable' }
+
+  const moveResult = moveCursorBeforeTemplate(index)
+  if (!moveResult.ok) return moveResult
 
   const deleteEvent = new KeyboardEvent('keydown', {
     key: 'Delete',
     bubbles: true,
     cancelable: true,
   })
 
-  return editor.view.dom.dispatchEvent(deleteEvent) === false
+  const ok = editor.view.dom.dispatchEvent(deleteEvent) === false
+  return ok ? { ok } : { ok: false, reason: `Delete before template ${index} was not handled` }
 }

Also applies to: 170-209

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/test/src/sender/index.vue` around lines 7 - 14, The test hook
__senderTestApi currently returns boolean from moveCursorBeforeTemplate and
pressDeleteBeforeTemplate which conflates multiple failure modes; change the
Window.__senderTestApi type to return a structured result { ok: boolean,
reason?: string } for both functions, update all call sites (including the
helpers that currently assert true around moveCursorBeforeTemplate and
pressDeleteBeforeTemplate) to handle the object by checking result.ok and
surface or assert result.reason on failure, and adjust any tests or assertions
in the template-helper functions to expect and propagate the structured {ok,
reason} instead of a bare boolean so failures report the precise cause.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/test/src/sender/index.vue`:
- Around line 7-14: The test hook __senderTestApi currently returns boolean from
moveCursorBeforeTemplate and pressDeleteBeforeTemplate which conflates multiple
failure modes; change the Window.__senderTestApi type to return a structured
result { ok: boolean, reason?: string } for both functions, update all call
sites (including the helpers that currently assert true around
moveCursorBeforeTemplate and pressDeleteBeforeTemplate) to handle the object by
checking result.ok and surface or assert result.reason on failure, and adjust
any tests or assertions in the template-helper functions to expect and propagate
the structured {ok, reason} instead of a bare boolean so failures report the
precise cause.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 69929036-a0e7-4c4c-9b00-41613dc27ff4

📥 Commits

Reviewing files that changed from the base of the PR and between b554414 and 961233c.

📒 Files selected for processing (3)
  • packages/test/src/sender/helpers/template-helper.ts
  • packages/test/src/sender/index.vue
  • packages/test/src/sender/specs/template/delete.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/test/src/sender/specs/template/delete.spec.ts

@SonyLeo SonyLeo changed the title fix(sender-e2e): remove flaky template delete case fix(sender-e2e): stabilize template delete e2e case Mar 30, 2026
@hexqi hexqi merged commit a240c7d into opentiny:develop Mar 30, 2026
7 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

🧹 Preview Cleaned Up

The preview deployment has been removed.

@SonyLeo SonyLeo deleted the fix/sender-template-e2e-cases branch April 2, 2026 03:29
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