-
Notifications
You must be signed in to change notification settings - Fork 11.9k
feat(@angular/cli): add "test" and "e2e" MCP tools, and some test cleanup #32066
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,121 @@ | ||
| /** | ||
| * @license | ||
| * Copyright Google LLC All Rights Reserved. | ||
| * | ||
| * Use of this source code is governed by an MIT-style license that can be | ||
| * found in the LICENSE file at https://angular.dev/license | ||
| */ | ||
|
|
||
| import { z } from 'zod'; | ||
| import { CommandError, type Host, LocalWorkspaceHost } from '../host'; | ||
| import { createStructuredContentOutput } from '../utils'; | ||
| import { type McpToolContext, type McpToolDeclaration, declareTool } from './tool-registry'; | ||
|
|
||
| const e2eStatusSchema = z.enum(['success', 'failure']); | ||
| type E2eStatus = z.infer<typeof e2eStatusSchema>; | ||
|
|
||
| const e2eToolInputSchema = z.object({ | ||
| project: z | ||
| .string() | ||
| .optional() | ||
| .describe( | ||
| 'Which project to test in a monorepo context. If not provided, tests the default project.', | ||
| ), | ||
| }); | ||
|
|
||
| export type E2eToolInput = z.infer<typeof e2eToolInputSchema>; | ||
|
|
||
| const e2eToolOutputSchema = z.object({ | ||
| status: e2eStatusSchema.describe('E2E execution status.'), | ||
| logs: z.array(z.string()).optional().describe('Output logs from `ng e2e`.'), | ||
| }); | ||
|
|
||
| export type E2eToolOutput = z.infer<typeof e2eToolOutputSchema>; | ||
|
|
||
| export async function runE2e(input: E2eToolInput, host: Host, context: McpToolContext) { | ||
| const projectName = input.project; | ||
|
|
||
| if (context.workspace) { | ||
| let targetProject; | ||
| const projects = context.workspace.projects; | ||
|
|
||
| if (projectName) { | ||
| targetProject = projects.get(projectName); | ||
| } else { | ||
| // Try to find default project | ||
| const defaultProjectName = context.workspace.extensions['defaultProject'] as | ||
| | string | ||
| | undefined; | ||
| if (defaultProjectName) { | ||
| targetProject = projects.get(defaultProjectName); | ||
| } else if (projects.size === 1) { | ||
| targetProject = Array.from(projects.values())[0]; | ||
| } | ||
| } | ||
|
|
||
| if (targetProject) { | ||
| if (!targetProject.targets.has('e2e')) { | ||
| return createStructuredContentOutput({ | ||
| status: 'failure', | ||
| logs: [ | ||
| `No e2e target is defined for project '${projectName ?? 'default'}'. Please setup e2e testing first.`, | ||
|
||
| ], | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Build "ng"'s command line. | ||
| const args = ['e2e']; | ||
| if (input.project) { | ||
| args.push(input.project); | ||
| } | ||
|
|
||
| let status: E2eStatus = 'success'; | ||
| let logs: string[] = []; | ||
|
|
||
| try { | ||
| logs = (await host.runCommand('ng', args)).logs; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: I probably should have asked this on an earlier PR (maybe I did and forgot), but how is |
||
| } catch (e) { | ||
| status = 'failure'; | ||
| if (e instanceof CommandError) { | ||
| logs = e.logs; | ||
| } else if (e instanceof Error) { | ||
| logs = [e.message]; | ||
| } else { | ||
| logs = [String(e)]; | ||
| } | ||
| } | ||
|
Comment on lines
+62
to
+73
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider: Can this be refactored to a shared primitive somewhere? Seems like it would overlap pretty heavily with other executions of the CLI in the MCP server. |
||
|
|
||
| const structuredContent: E2eToolOutput = { | ||
| status, | ||
| logs, | ||
| }; | ||
|
|
||
| return createStructuredContentOutput(structuredContent); | ||
| } | ||
|
|
||
| export const E2E_TOOL: McpToolDeclaration< | ||
| typeof e2eToolInputSchema.shape, | ||
| typeof e2eToolOutputSchema.shape | ||
| > = declareTool({ | ||
| name: 'e2e', | ||
| title: 'E2E Tool', | ||
| description: ` | ||
| <Purpose> | ||
| Perform an end-to-end test with ng e2e. | ||
| </Purpose> | ||
| <Use Cases> | ||
| * Running end-to-end tests for the project. | ||
| </Use Cases> | ||
|
Comment on lines
+93
to
+95
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider: Is there any more context we can give to use cases which would be useful to agents? I'm not sure how helpful declaring the |
||
| <Operational Notes> | ||
| * This tool runs "ng e2e". | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider: This line feels like an implementation detail. Does the AI agent care if it's going through the MCP server? |
||
| * It will error if no "e2e" target is defined in the project, to avoid interactive setup prompts. | ||
| </Operational Notes> | ||
| `, | ||
| isReadOnly: false, | ||
| isLocalOnly: true, | ||
| inputSchema: e2eToolInputSchema.shape, | ||
| outputSchema: e2eToolOutputSchema.shape, | ||
| factory: (context) => (input) => runE2e(input, LocalWorkspaceHost, context), | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| /** | ||
| * @license | ||
| * Copyright Google LLC All Rights Reserved. | ||
| * | ||
| * Use of this source code is governed by an MIT-style license that can be | ||
| * found in the LICENSE file at https://angular.dev/license | ||
| */ | ||
|
|
||
| import { workspaces } from '@angular-devkit/core'; | ||
| import { AngularWorkspace } from '../../../utilities/config'; | ||
| import { CommandError, Host } from '../host'; | ||
| import type { MockHost } from '../testing/mock-host'; | ||
| import { runE2e } from './e2e'; | ||
| import type { McpToolContext } from './tool-registry'; | ||
|
|
||
| describe('E2E Tool', () => { | ||
| let mockHost: MockHost; | ||
| let mockContext: McpToolContext; | ||
| let mockProjects: workspaces.ProjectDefinitionCollection; | ||
| let mockWorkspace: AngularWorkspace; | ||
|
|
||
| beforeEach(() => { | ||
| mockHost = { | ||
| runCommand: jasmine.createSpy<Host['runCommand']>('runCommand').and.resolveTo({ logs: [] }), | ||
| } as unknown as MockHost; | ||
|
|
||
| mockProjects = new workspaces.ProjectDefinitionCollection(); | ||
| const mockWorkspaceDefinition: workspaces.WorkspaceDefinition = { | ||
| projects: mockProjects, | ||
| extensions: {}, | ||
| }; | ||
|
|
||
| mockWorkspace = new AngularWorkspace(mockWorkspaceDefinition, '/test/angular.json'); | ||
| mockContext = { | ||
| workspace: mockWorkspace, | ||
| } as McpToolContext; | ||
| }); | ||
|
|
||
| function addProject(name: string, targets: Record<string, workspaces.TargetDefinition> = {}) { | ||
| mockProjects.set(name, { | ||
| root: `projects/${name}`, | ||
| extensions: {}, | ||
| targets: new workspaces.TargetDefinitionCollection(targets), | ||
| }); | ||
| } | ||
|
|
||
| it('should construct the command correctly with defaults', async () => { | ||
| await runE2e({}, mockHost, mockContext); | ||
| expect(mockHost.runCommand).toHaveBeenCalledWith('ng', ['e2e']); | ||
| }); | ||
|
|
||
| it('should construct the command correctly with a specified project', async () => { | ||
| addProject('my-app', { e2e: { builder: 'mock-builder' } }); | ||
|
|
||
| await runE2e({ project: 'my-app' }, mockHost, mockContext); | ||
| expect(mockHost.runCommand).toHaveBeenCalledWith('ng', ['e2e', 'my-app']); | ||
| }); | ||
|
|
||
| it('should error if project does not have e2e target', async () => { | ||
| addProject('my-app', { build: { builder: 'mock-builder' } }); | ||
|
|
||
| const { structuredContent } = await runE2e({ project: 'my-app' }, mockHost, mockContext); | ||
|
|
||
| expect(structuredContent.status).toBe('failure'); | ||
| expect(structuredContent.logs?.[0]).toContain("No e2e target is defined for project 'my-app'"); | ||
| expect(mockHost.runCommand).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should error if default project does not have e2e target and no project specified', async () => { | ||
|
||
| mockWorkspace.extensions['defaultProject'] = 'my-app'; | ||
| addProject('my-app', { build: { builder: 'mock-builder' } }); | ||
|
|
||
| const { structuredContent } = await runE2e({}, mockHost, mockContext); | ||
|
|
||
| expect(structuredContent.status).toBe('failure'); | ||
| expect(structuredContent.logs?.[0]).toContain("No e2e target is defined for project 'default'"); | ||
| expect(mockHost.runCommand).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should proceed if no workspace context is available (fallback)', async () => { | ||
| // If context.workspace is undefined, it should try to run ng e2e (which might fail or prompt, but tool runs it) | ||
| const noWorkspaceContext = {} as McpToolContext; | ||
| await runE2e({}, mockHost, noWorkspaceContext); | ||
| expect(mockHost.runCommand).toHaveBeenCalledWith('ng', ['e2e']); | ||
| }); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: This is the case where we run the MCP server outside of a specific Angular CLI project right? I wonder if we should just fail in that case rather than attempting to run an Thoughts @clydin? |
||
|
|
||
| it('should handle a successful e2e run', async () => { | ||
| addProject('my-app', { e2e: { builder: 'mock-builder' } }); | ||
| const e2eLogs = ['E2E passed']; | ||
| mockHost.runCommand.and.resolveTo({ logs: e2eLogs }); | ||
|
|
||
| const { structuredContent } = await runE2e({ project: 'my-app' }, mockHost, mockContext); | ||
|
|
||
| expect(structuredContent.status).toBe('success'); | ||
| expect(structuredContent.logs).toEqual(e2eLogs); | ||
| }); | ||
|
|
||
| it('should handle a failed e2e run', async () => { | ||
| addProject('my-app', { e2e: { builder: 'mock-builder' } }); | ||
| const e2eLogs = ['E2E failed']; | ||
| mockHost.runCommand.and.rejectWith(new CommandError('Failed', e2eLogs, 1)); | ||
|
|
||
| const { structuredContent } = await runE2e({ project: 'my-app' }, mockHost, mockContext); | ||
|
|
||
| expect(structuredContent.status).toBe('failure'); | ||
| expect(structuredContent.logs).toEqual(e2eLogs); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| /** | ||
| * @license | ||
| * Copyright Google LLC All Rights Reserved. | ||
| * | ||
| * Use of this source code is governed by an MIT-style license that can be | ||
| * found in the LICENSE file at https://angular.dev/license | ||
| */ | ||
|
|
||
| import { z } from 'zod'; | ||
| import { CommandError, type Host, LocalWorkspaceHost } from '../host'; | ||
| import { createStructuredContentOutput } from '../utils'; | ||
| import { type McpToolDeclaration, declareTool } from './tool-registry'; | ||
|
|
||
| const testStatusSchema = z.enum(['success', 'failure']); | ||
| type TestStatus = z.infer<typeof testStatusSchema>; | ||
|
|
||
| const testToolInputSchema = z.object({ | ||
| project: z | ||
| .string() | ||
| .optional() | ||
| .describe('Which project to test in a monorepo context. If not provided, tests all projects.'), | ||
| filter: z.string().optional().describe('Filter the executed tests by spec name.'), | ||
| }); | ||
|
|
||
| export type TestToolInput = z.infer<typeof testToolInputSchema>; | ||
|
|
||
| const testToolOutputSchema = z.object({ | ||
| status: testStatusSchema.describe('Test execution status.'), | ||
| logs: z.array(z.string()).optional().describe('Output logs from `ng test`.'), | ||
| }); | ||
|
|
||
| export type TestToolOutput = z.infer<typeof testToolOutputSchema>; | ||
|
|
||
| export async function runTest(input: TestToolInput, host: Host) { | ||
| // Build "ng"'s command line. | ||
| const args = ['test']; | ||
| if (input.project) { | ||
| args.push(input.project); | ||
| } | ||
|
|
||
| // This is ran by the agent so we want a non-watched, headless test. | ||
| args.push('--browsers', 'ChromeHeadless'); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider: What if someone doesn't have Chrome installed and instead uses Firefox or Safari? @clydin is there a way to trigger "headless" without specifying a specific browser? The alternative I can think of is setting the |
||
| args.push('--watch', 'false'); | ||
|
|
||
| if (input.filter) { | ||
| args.push('--filter', input.filter); | ||
| } | ||
|
|
||
| let status: TestStatus = 'success'; | ||
| let logs: string[] = []; | ||
|
|
||
| try { | ||
| logs = (await host.runCommand('ng', args)).logs; | ||
| } catch (e) { | ||
| status = 'failure'; | ||
| if (e instanceof CommandError) { | ||
| logs = e.logs; | ||
| } else if (e instanceof Error) { | ||
| logs = [e.message]; | ||
| } else { | ||
| logs = [String(e)]; | ||
| } | ||
| } | ||
|
|
||
| const structuredContent: TestToolOutput = { | ||
| status, | ||
| logs, | ||
| }; | ||
|
|
||
| return createStructuredContentOutput(structuredContent); | ||
| } | ||
|
|
||
| export const TEST_TOOL: McpToolDeclaration< | ||
| typeof testToolInputSchema.shape, | ||
| typeof testToolOutputSchema.shape | ||
| > = declareTool({ | ||
| name: 'test', | ||
| title: 'Test Tool', | ||
| description: ` | ||
| <Purpose> | ||
| Perform a one-off, non-watched unit test execution with ng test. | ||
| </Purpose> | ||
| <Use Cases> | ||
| * Running unit tests for the project. | ||
| * Verifying code changes with tests. | ||
| </Use Cases> | ||
| <Operational Notes> | ||
| * This tool runs "ng test" with "--watch false". | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider: This line also feels like an implementation detail. Does the AI agent care if it's going through the MCP server to run these tests? |
||
| * It supports filtering by spec name if the underlying builder supports it (e.g., 'unit-test' builder). | ||
| </Operational Notes> | ||
| `, | ||
| isReadOnly: false, | ||
| isLocalOnly: true, | ||
| inputSchema: testToolInputSchema.shape, | ||
| outputSchema: testToolOutputSchema.shape, | ||
| factory: () => (input) => runTest(input, LocalWorkspaceHost), | ||
| }); | ||
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.
Consider: Refactor to some kind of
validateE2eAlreadySetUphelper since we shouldn't need any of this data to actually run E2E tests and can simplifyrunE2e.