-
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 all commits
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,91 @@ | ||
| /** | ||
| * @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 { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; | ||
| import { AngularWorkspace } from '../../../utilities/config'; | ||
| import { type Devserver } from '../devserver'; | ||
| import { Host } from '../host'; | ||
| import { McpToolContext } from '../tools/tool-registry'; | ||
| import { MockHost } from './mock-host'; | ||
|
|
||
| /** | ||
| * Creates a mock implementation of the Host interface for testing purposes. | ||
| * Each method is a Jasmine spy that can be configured. | ||
| */ | ||
| export function createMockHost(): MockHost { | ||
| return { | ||
| runCommand: jasmine.createSpy<Host['runCommand']>('runCommand').and.resolveTo({ logs: [] }), | ||
| stat: jasmine.createSpy<Host['stat']>('stat'), | ||
| existsSync: jasmine.createSpy<Host['existsSync']>('existsSync'), | ||
| spawn: jasmine.createSpy<Host['spawn']>('spawn'), | ||
| getAvailablePort: jasmine | ||
| .createSpy<Host['getAvailablePort']>('getAvailablePort') | ||
| .and.resolveTo(0), | ||
|
Comment on lines
+27
to
+29
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: Could we put the auto-incrementing port number implementation here and potentially reuse it across all affected tests? |
||
| } as unknown as MockHost; | ||
| } | ||
|
|
||
| /** | ||
| * Options for configuring the mock MCP tool context. | ||
| */ | ||
| export interface MockContextOptions { | ||
| /** An optional pre-configured mock host. If not provided, a default mock host will be created. */ | ||
| host?: MockHost; | ||
|
|
||
| /** Initial set of projects to populate the mock workspace with. */ | ||
| projects?: Record<string, workspaces.ProjectDefinition>; | ||
| } | ||
|
|
||
| /** | ||
| * Creates a comprehensive mock for the McpToolContext, including a mock Host, | ||
| * an AngularWorkspace, and a ProjectDefinitionCollection. This simplifies testing | ||
| * MCP tools by providing a consistent and configurable testing environment. | ||
| * @param options Configuration options for the mock context. | ||
| * @returns An object containing the mock host, context, projects collection, and workspace instance. | ||
| */ | ||
| export function createMockContext(options: MockContextOptions = {}): { | ||
| host: MockHost; | ||
| context: McpToolContext; | ||
| projects: workspaces.ProjectDefinitionCollection; | ||
| workspace: AngularWorkspace; | ||
| } { | ||
| const host = options.host ?? createMockHost(); | ||
| const projects = new workspaces.ProjectDefinitionCollection(options.projects); | ||
| const workspace = new AngularWorkspace({ projects, extensions: {} }, '/test/angular.json'); | ||
|
|
||
| const context: McpToolContext = { | ||
| server: {} as unknown as McpServer, | ||
| workspace, | ||
| logger: { warn: () => {} }, | ||
| devservers: new Map<string, Devserver>(), | ||
| host, | ||
| }; | ||
|
|
||
| return { host, context, projects, workspace }; | ||
| } | ||
|
|
||
| /** | ||
| * Adds a project to the provided mock ProjectDefinitionCollection. | ||
| * This is a helper function to easily populate a mock Angular workspace. | ||
| * @param projects The ProjectDefinitionCollection to add the project to. | ||
| * @param name The name of the project. | ||
| * @param targets A record of target definitions for the project (e.g., build, test, e2e). | ||
| * @param root The root path of the project, relative to the workspace root. Defaults to `projects/${name}`. | ||
| */ | ||
| export function addProjectToWorkspace( | ||
| projects: workspaces.ProjectDefinitionCollection, | ||
| name: string, | ||
| targets: Record<string, workspaces.TargetDefinition> = {}, | ||
| root: string = `projects/${name}`, | ||
| ) { | ||
| projects.set(name, { | ||
| root, | ||
| extensions: {}, | ||
| targets: new workspaces.TargetDefinitionCollection(targets), | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,8 +7,7 @@ | |
| */ | ||
|
|
||
| import { z } from 'zod'; | ||
| import { devserverKey } from '../../devserver'; | ||
| import { createStructuredContentOutput } from '../../utils'; | ||
| import { createStructuredContentOutput, getDefaultProjectName } from '../../utils'; | ||
| import { type McpToolContext, type McpToolDeclaration, declareTool } from '../tool-registry'; | ||
|
|
||
| const devserverStopToolInputSchema = z.object({ | ||
|
|
@@ -30,21 +29,41 @@ const devserverStopToolOutputSchema = z.object({ | |
| export type DevserverStopToolOutput = z.infer<typeof devserverStopToolOutputSchema>; | ||
|
|
||
| export function stopDevserver(input: DevserverStopToolInput, context: McpToolContext) { | ||
| const projectKey = devserverKey(input.project); | ||
| const devServer = context.devservers.get(projectKey); | ||
| if (context.devservers.size === 0) { | ||
| return createStructuredContentOutput({ | ||
| message: ['No development servers are currently running.'], | ||
| logs: undefined, | ||
| }); | ||
| } | ||
|
|
||
| let projectName = input.project ?? getDefaultProjectName(context); | ||
|
|
||
| if (!projectName) { | ||
| // This should not happen. But if there's just a single running devserver, stop it. | ||
|
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: If this shouldn't happen, do we want to just Based on the error message below, it seems weird that it would be ignored if there happens to be just one devserver running. |
||
| if (context.devservers.size === 1) { | ||
| projectName = Array.from(context.devservers.keys())[0]; | ||
| } else { | ||
| return createStructuredContentOutput({ | ||
| message: ['Project name not provided, and no default project found.'], | ||
| logs: undefined, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| const devServer = context.devservers.get(projectName); | ||
|
|
||
| if (!devServer) { | ||
| return createStructuredContentOutput({ | ||
| message: `Development server for project '${projectKey}' was not running.`, | ||
| message: `Development server for project '${projectName}' was not running.`, | ||
| logs: undefined, | ||
| }); | ||
| } | ||
|
|
||
| devServer.stop(); | ||
| context.devservers.delete(projectKey); | ||
| context.devservers.delete(projectName); | ||
|
|
||
| return createStructuredContentOutput({ | ||
| message: `Development server for project '${projectKey}' stopped.`, | ||
| message: `Development server for project '${projectName}' stopped.`, | ||
| logs: devServer.getServerLogs(), | ||
| }); | ||
| } | ||
|
|
||
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: Is there a path to a "fake" host instead of a "mock" host? Could we provide fake implementations for
statandexistsSyncif we accept a virtual file system (or create a real one in a temp directly from inputs)?Faking implementation of
runCommandandspawnis likely a bit harder, but we could either delegate the command to an input function or actually run a real subprocess to get more accurate error behavior. I'm thinking something along the lines of:I'm thinking this can avoid the need to define spies unique to each test and increase overall test confidence. If we can make this specific to each test as suggested in a separate comment, it can potentially remove the need to mutate the environment after the
beforeEachwithin a test.Downside is that we do need to explicitly call
disposeto clean up the temp directory if we're using a real file system. If we can run these tests on Node 24, we can potentially get away withusingandSymbol.disposewhich would help.