test: 新增协同编辑冲突编辑用例、多浏览器测试 #340
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate 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. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/docs/fluent-editor/demos/collaborative-editing.spec.ts (1)
12-28: Add error handling for browser launch failures.The function should handle potential browser launch failures more gracefully to provide better debugging information when tests fail.
async function openTwoPages(browserType = chromium): Promise<[Page, Page, Browser, Browser]> { - const browser1 = await browserType.launch() - const browser2 = await browserType.launch() + let browser1: Browser, browser2: Browser + try { + browser1 = await browserType.launch() + browser2 = await browserType.launch() + } catch (error) { + throw new Error(`Failed to launch browsers: ${error}`) + } const page1 = await browser1.newPage() const page2 = await browser2.newPage()
🧹 Nitpick comments (5)
packages/docs/fluent-editor/demos/collaborative-editing.spec.ts (5)
1-1: Consider adding Firefox for broader browser coverage.While Chrome and WebKit provide good coverage, adding Firefox would ensure testing across all major browser engines (Blink, WebKit, and Gecko).
-import { type Browser, chromium, expect, type Page, test, webkit } from '@playwright/test' +import { type Browser, chromium, firefox, expect, type Page, test, webkit } from '@playwright/test'And update the browsers array:
const browsers = [ { name: 'chromium', launcher: chromium }, { name: 'webkit', launcher: webkit }, + { name: 'firefox', launcher: firefox }, ]
122-149: Complex font size test logic could be simplified.The nested conditions and string manipulation make this test hard to follow. Consider extracting the validation logic.
+async function validateFontSize(page: Page, size: string, text: string): Promise<boolean> { + if (size === '12px') { + const hasParagraph = await page.locator(`.ql-editor p:has-text("${text}")`).count() + const hasSpan = await page.locator('.ql-editor span[style*="font-size"]').count() + return hasParagraph > 0 && hasSpan === 0 + } + return (await page.locator(`.ql-editor span[style*="font-size: ${size}"]`).count()) > 0 +} test('size collaborative-editing test', async () => { await typeSync(p1, p2, 'HEAD') await p1.locator('.ql-editor').click() await selectAll(p1) const sequence = ['12px', '14px', '14px', '16px', '16px', '18px', '18px', '20px', '20px', '24px', '24px', '32px'] let current = sequence[0] for (let i = 1; i < sequence.length; i++) { const next = sequence[i] if (next === current) { continue } await p1.getByRole('button', { name: current }).click() await p1.getByRole('button', { name: next }).click() const sizeMatch = next.match(/\d+px/) if (!sizeMatch) continue const size = sizeMatch[0] - if (size === '12px') { - await expect.poll(async () => { - const hasParagraph = await p2.locator('.ql-editor p:has-text("HEAD")').count() - const hasSpan = await p2.locator('.ql-editor span[style*="font-size"]').count() - return hasParagraph > 0 && hasSpan === 0 - }).toBeTruthy() - } - else { - await expect.poll(async () => (await p2.locator(`.ql-editor span[style*="font-size: ${size}"]`).count()) > 0).toBeTruthy() - } + await expect.poll(() => validateFontSize(p2, size, 'HEAD')).toBeTruthy() current = next } })
316-323: Consider adding timeout for conflict resolution polling.The polling for text convergence could potentially hang if the collaborative system fails. Consider adding an explicit timeout.
- await expect.poll(async () => { + await expect.poll(async () => { const text1 = await p1.locator('.ql-editor').textContent() const text2 = await p2.locator('.ql-editor').textContent() if (text1 === text2 && (text1 === 'AB' || text1 === 'BA')) { return true } return false - }).toBeTruthy() + }, { timeout: 10000 }).toBeTruthy()
172-182: Consider extracting common test patterns.The format tests follow a very similar pattern. Consider creating a data-driven test to reduce duplication.
+const formatTestCases = [ + { format: 'bold', tag: 'strong' }, + { format: 'italic', tag: 'em' }, + { format: 'underline', tag: 'u' }, + { format: 'strike', tag: '.ql-custom-strike' } +] + -const formatTypes = ['bold', 'italic', 'underline', 'strike'] -formatTypes.forEach((fmt) => { - test(`${fmt} collaborative-editing test`, async () => { - await typeSync(p1, p2, fmt) +formatTestCases.forEach(({ format, tag }) => { + test(`${format} collaborative-editing test`, async () => { + await typeSync(p1, p2, format) await selectAll(p1) - await p1.getByLabel(fmt).click() - const tagMap: Record<string, string> = { bold: 'strong', italic: 'em', underline: 'u', strike: '.ql-custom-strike' } - const tag = tagMap[fmt] + await p1.getByLabel(format).click() await expect.poll(async () => (await p2.locator(`.ql-editor ${tag}`).count()) > 0).toBeTruthy() }) })
42-326: Consider adding more complex conflict scenarios.While the basic simultaneous edit test is good, consider adding tests for more complex conflict scenarios such as:
- Multiple users editing the same line
- Conflicting format changes
- Simultaneous list operations
Would you like me to help create additional conflict resolution test cases to cover more complex collaborative editing scenarios?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/docs/fluent-editor/demos/collaborative-editing.spec.ts(2 hunks)
🔇 Additional comments (3)
packages/docs/fluent-editor/demos/collaborative-editing.spec.ts (3)
91-97: Good helper function for heading verification.The
headingMatchedhelper properly checks for both standard HTML heading elements and Quill's custom heading classes, making the tests more robust.
256-260: Good defensive check for prompt input visibility.The check for prompt input visibility before filling it prevents test failures when the UI behavior changes.
307-324: Excellent conflict resolution test implementation.The simultaneous edit test properly verifies that concurrent edits are handled correctly, accepting either 'AB' or 'BA' as valid outcomes, which correctly tests the conflict resolution behavior of the collaborative editor.
packages/docs/fluent-editor/demos/collaborative-editing.spec.ts
Outdated
Show resolved
Hide resolved
3 participants
PR
PR Type
What kind of change does this PR introduce?
Summary by CodeRabbit
Tests
Chores