chore(mcp): add experimental MCP interface to test server #35312
+3,886
−430
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Skn0tt
commented
Mar 21, 2025
| @@ -36,3 +36,12 @@ export const chokidar = chokidarLibrary; | |||
|
|
|||
| import * as getEastAsianWidthLibrary from 'get-east-asian-width'; | |||
| export const getEastAsianWidth = getEastAsianWidthLibrary; | |||
|
|
|||
| import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; | |||
There was a problem hiding this comment.
This grows the utils bundle from 200kb to 300kb. If that's too much, I guess we could write our own implementation. Not sure if that's worth it though.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces an experimental MCP interface for the test server, adding support for MCP-based tools in both the test harness and the server.
- Adds integration tests for MCP functionality in Playwright tests.
- Implements MCP server functionality with tool registration for listing and running tests.
- Updates common fixtures and utility bundles to support MCP client and server components.
Reviewed Changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/playwright-test/ui-mode-mcp.spec.ts | Adds integration tests that leverage the experimental MCP interface. |
| packages/playwright/src/runner/mcp.ts | Introduces MCP server functionality with tool registration (“listTests” and “runTests”). |
| tests/config/commonFixtures.ts | Implements an attachMCP function to support MCP in test processes. |
| packages/playwright/src/utilsBundle.ts | Exports MCP-related utilities and types. |
| packages/playwright/bundles/utils/src/utilsBundleImpl.ts | Re-exports MCP server components and zod schema validation. |
| packages/playwright/src/runner/testServer.ts | Starts the MCP server when the PW_MCP flag is set. |
Files not reviewed (2)
- package.json: Language not supported
- packages/playwright/bundles/utils/package.json: Language not supported
| try { | ||
| const parsed = JSON.parse(message); | ||
| transport.onmessage?.(parsed); | ||
| } catch {} |
There was a problem hiding this comment.
Swallowing errors in the JSON.parse block may hinder debugging issues with incoming messages. Consider logging the error or providing a minimal handling mechanism to help diagnose malformed input.
Suggested change
| } catch {} | |
| } catch (error) { | |
| console.error('Failed to parse message:', message, error); | |
| } |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Test results for "tests 1"5 flaky38822 passed, 809 skipped Merge workflow run. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.