-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: fix e2e test #244
test: fix e2e test #244
Conversation
WalkthroughThis pull request implements changes across documentation and test files. The version insertion function now assigns a default value of Changes
Sequence Diagram(s)sequenceDiagram
participant F as insertDepsVersion
participant D as Document Body
F->>F: Retrieve version variables (Vite, VitePress, Vue, Quill)
alt Version exists
F->>D: Set corresponding data attribute with version value
else
F->>D: Set corresponding data attribute to 'dev'
end
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (3)
packages/docs/fluent-editor/demos/add-toolbar-item.spec.ts (3)
43-43
: Consider translating the Chinese comment to English for consistency.The comment "添加更多测试用例" (which translates to "Add more test cases") could be translated to English to maintain consistency throughout the codebase.
- // 添加更多测试用例 + // Add more test cases
13-20
: Add a test to verify toolbar buttons' visual appearance.Since you're testing custom toolbar items, consider adding a test to verify they have the correct visual properties (icons, tooltips, etc.) to ensure a complete test coverage.
test('should display toolbar buttons with correct visual properties', async ({ page }) => { const goodButton = page.locator('.ql-good') const badButton = page.locator('.ql-bad') // Check if buttons have the expected appearance await expect(goodButton).toBeVisible() await expect(badButton).toBeVisible() // Verify tooltips or other visual attributes if applicable // Example: await expect(goodButton).toHaveAttribute('title', 'Good Format') })
22-41
: Enhance test reliability by awaiting content changes.The test could be more reliable by explicitly waiting for content changes after applying formats, rather than relying on immediate state changes.
test('should apply custom formats when toolbar buttons are clicked', async ({ page }) => { const editor = page.locator('#editor-add-toolbar-item .ql-editor') expect(editor).not.toBeNull() await editor.click() await page.keyboard.type('Test text') const goodButton = page.locator('.ql-good') expect(goodButton).not.toBeNull() await goodButton.click() await page.keyboard.type(' Good') const badButton = page.locator('.ql-bad') expect(badButton).not.toBeNull() await badButton.click() await page.keyboard.type(' Bad') + // Wait for editor content to stabilize after formatting + await page.waitForTimeout(100) const editorHtml = await editor.innerHTML() expect(editorHtml).toContain('<span style="color: rgb(92, 179, 0);"> Good</span>') expect(editorHtml).toContain('<span style="color: rgb(242, 48, 48);"> Bad</span>') })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/docs/fluent-editor/.vitepress/theme/insert-deps-version.ts
(1 hunks)packages/docs/fluent-editor/demos/add-toolbar-item.spec.ts
(1 hunks)packages/docs/fluent-editor/demos/basic-usage.spec.ts
(1 hunks)packages/docs/fluent-editor/demos/image-upload.spec.ts
(2 hunks)packages/docs/playwright.config.ts
(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Playwright Tests
packages/docs/fluent-editor/demos/basic-usage.spec.ts
[error] 5-5: Error: page.goto: net::ERR_CONNECTION_REFUSED at http://localhost:5173/tiny-editor/docs/demo/basic-usage
packages/docs/fluent-editor/demos/image-upload.spec.ts
[error] 10-10: Error: page.goto: net::ERR_CONNECTION_REFUSED at http://localhost:5173/tiny-editor/docs/demo/image-upload
packages/docs/fluent-editor/demos/add-toolbar-item.spec.ts
[error] 5-5: Error: page.goto: net::ERR_CONNECTION_REFUSED at http://localhost:5173/tiny-editor/docs/demo/add-toolbar-item
[error] 5-5: Error: page.goto: net::ERR_CONNECTION_REFUSED at http://localhost:5173/tiny-editor/docs/demo/add-toolbar-item
[error] 5-5: Error: page.goto: net::ERR_CONNECTION_REFUSED at http://localhost:5173/tiny-editor/docs/demo/add-toolbar-item
🔇 Additional comments (5)
packages/docs/fluent-editor/demos/basic-usage.spec.ts (1)
5-5
:❓ Verification inconclusive
URL path has been updated to match new demo structure
The URL has been changed to include
/demo/
in the path, which aligns with the new demo path structure used across test files. This is a good consistency improvement.However, there is a pipeline failure showing that this URL is resulting in a connection refused error. This needs to be verified.
🏁 Script executed:
#!/bin/bash # Check if the server is configured correctly for the new URL path structure # First, find the server config in the project rg -A 3 "tiny-editor/docs/demo" --type ts # Then, check if there are any vite.config or similar files that define routes fd -e ts "config" --exec grep -l "routes\|path" {}Length of output: 1443
Attention: Verify Demo Server Connectivity on Updated URL Structure
- The new URL path (e.g.,
http://localhost:5173/tiny-editor/docs/demo/basic-usage
) is consistently applied across all demo test files.- Our code search confirms that files like
basic-usage.spec.ts
,add-toolbar-item.spec.ts
, andimage-upload.spec.ts
have been updated accordingly.- However, the pipeline failure indicating a “connection refused” error suggests that the demo server may not be starting/updating its routes as expected in the test environment.
- Please manually verify that the demo server is correctly configured, is running on port 5173, and that any route definitions in configuration files (e.g., in
packages/docs/playwright.config.ts
,packages/fluent-editor/vite.config.ts
, etc.) accommodate the/demo/
path. In addition, ensure that any changes in the server startup configuration are reflected in the CI/pipeline environment.🧰 Tools
🪛 GitHub Actions: Playwright Tests
[error] 5-5: Error: page.goto: net::ERR_CONNECTION_REFUSED at http://localhost:5173/tiny-editor/docs/demo/basic-usage
packages/docs/fluent-editor/.vitepress/theme/insert-deps-version.ts (1)
14-17
: Improved robustness with fallback version valuesAdding fallback values for the version attributes is a good defensive programming practice. This ensures that the document attributes will always have a value even if the environment variables are undefined, which prevents potential DOM-related issues.
packages/docs/fluent-editor/demos/image-upload.spec.ts (1)
23-23
: Improved selector specificity for image elementThe selector for the image element has been refined to be more specific by targeting an image within
#editor .ql-editor
rather than just within#editor
. This makes the test more robust against potential DOM structure changes and provides more reliable element targeting.packages/docs/playwright.config.ts (1)
73-73
: Increased test timeout for better stabilityAdding a global timeout of 120 seconds (120,000 milliseconds) is a good practice for end-to-end tests, especially when working with complex UI components like rich text editors. This helps prevent tests from failing due to timing issues in slower environments.
packages/docs/fluent-editor/demos/add-toolbar-item.spec.ts (1)
9-9
: Good migration from deprecatedpage.$
to recommendedpage.locator
API.The updates from
await page.$
topage.locator
align with Playwright's best practices for element selection. This approach provides better chainability and improved selector capabilities.Note that
page.locator()
returns a Locator object immediately (without waiting), so removing theawait
is correct. Actions performed on these locators (likeclick()
) will still need to be awaited.Also applies to: 14-17, 23-24, 28-29, 33-34
由于 vite 服务启动失败,需要 #243 合并后 test 应该可以通过 |
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Tests
Chores