Skip to content

Commit 5e1dab6

Browse files
committed
refactor(@angular/cli): clean up MCP tests and use real project name for default projects
1 parent c61bfb9 commit 5e1dab6

File tree

11 files changed

+245
-100
lines changed

11 files changed

+245
-100
lines changed

packages/angular/cli/src/commands/mcp/devserver.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,6 @@ export interface Devserver {
6464
port: number;
6565
}
6666

67-
export function devserverKey(project?: string) {
68-
return project ?? '<default>';
69-
}
70-
7167
/**
7268
* A local Angular development server managed by the MCP server.
7369
*/
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import { workspaces } from '@angular-devkit/core';
10+
import { AngularWorkspace } from '../../../utilities/config';
11+
import { type Devserver } from '../devserver';
12+
import { Host } from '../host';
13+
import { McpToolContext } from '../tools/tool-registry';
14+
import { MockHost } from './mock-host';
15+
16+
/**
17+
* Creates a mock implementation of the Host interface for testing purposes.
18+
* Each method is a Jasmine spy that can be configured.
19+
*/
20+
export function createMockHost(): MockHost {
21+
return {
22+
runCommand: jasmine.createSpy<Host['runCommand']>('runCommand').and.resolveTo({ logs: [] }),
23+
stat: jasmine.createSpy<Host['stat']>('stat'),
24+
existsSync: jasmine.createSpy<Host['existsSync']>('existsSync'),
25+
spawn: jasmine.createSpy<Host['spawn']>('spawn'),
26+
getAvailablePort: jasmine
27+
.createSpy<Host['getAvailablePort']>('getAvailablePort')
28+
.and.resolveTo(0),
29+
} as unknown as MockHost;
30+
}
31+
32+
/**
33+
* Options for configuring the mock MCP tool context.
34+
*/
35+
export interface MockContextOptions {
36+
/** An optional pre-configured mock host. If not provided, a default mock host will be created. */
37+
host?: MockHost;
38+
/** Initial set of projects to populate the mock workspace with. */
39+
projects?: Record<string, workspaces.ProjectDefinition>;
40+
}
41+
42+
/**
43+
* Creates a comprehensive mock for the McpToolContext, including a mock Host,
44+
* an AngularWorkspace, and a ProjectDefinitionCollection. This simplifies testing
45+
* MCP tools by providing a consistent and configurable testing environment.
46+
* @param options Configuration options for the mock context.
47+
* @returns An object containing the mock host, context, projects collection, and workspace instance.
48+
*/
49+
export function createMockContext(options: MockContextOptions = {}): {
50+
host: MockHost;
51+
context: McpToolContext;
52+
projects: workspaces.ProjectDefinitionCollection;
53+
workspace: AngularWorkspace;
54+
} {
55+
const host = options.host ?? createMockHost();
56+
const projects = new workspaces.ProjectDefinitionCollection(options.projects);
57+
const workspace = new AngularWorkspace({ projects, extensions: {} }, '/test/angular.json');
58+
59+
const context: McpToolContext = {
60+
server: {} as any,
61+
workspace,
62+
logger: { warn: () => {} },
63+
devservers: new Map<string, Devserver>(),
64+
host,
65+
};
66+
67+
return { host, context, projects, workspace };
68+
}
69+
70+
/**
71+
* Adds a project to the provided mock ProjectDefinitionCollection.
72+
* This is a helper function to easily populate a mock Angular workspace.
73+
* @param projects The ProjectDefinitionCollection to add the project to.
74+
* @param name The name of the project.
75+
* @param targets A record of target definitions for the project (e.g., build, test, e2e).
76+
* @param root The root path of the project, relative to the workspace root. Defaults to `projects/${name}`.
77+
*/
78+
export function addProjectToWorkspace(
79+
projects: workspaces.ProjectDefinitionCollection,
80+
name: string,
81+
targets: Record<string, workspaces.TargetDefinition> = {},
82+
root: string = `projects/${name}`,
83+
) {
84+
projects.set(name, {
85+
root,
86+
extensions: {},
87+
targets: new workspaces.TargetDefinitionCollection(targets),
88+
});
89+
}

packages/angular/cli/src/commands/mcp/tools/build_spec.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,16 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import { CommandError, Host } from '../host';
9+
import { CommandError } from '../host';
1010
import type { MockHost } from '../testing/mock-host';
1111
import { runBuild } from './build';
12+
import { createMockHost } from '../testing/test-utils';
1213

1314
describe('Build Tool', () => {
1415
let mockHost: MockHost;
1516

1617
beforeEach(() => {
17-
mockHost = {
18-
runCommand: jasmine.createSpy<Host['runCommand']>('runCommand').and.resolveTo({ logs: [] }),
19-
stat: jasmine.createSpy<Host['stat']>('stat'),
20-
existsSync: jasmine.createSpy<Host['existsSync']>('existsSync'),
21-
} as MockHost;
18+
mockHost = createMockHost();
2219
});
2320

2421
it('should construct the command correctly with default configuration', async () => {

packages/angular/cli/src/commands/mcp/tools/devserver/devserver-start.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
*/
88

99
import { z } from 'zod';
10-
import { LocalDevserver, devserverKey } from '../../devserver';
11-
import { createStructuredContentOutput } from '../../utils';
10+
import { LocalDevserver } from '../../devserver';
11+
import { createStructuredContentOutput, getDefaultProjectName } from '../../utils';
1212
import { type McpToolContext, type McpToolDeclaration, declareTool } from '../tool-registry';
1313

1414
const devserverStartToolInputSchema = z.object({
@@ -39,12 +39,18 @@ function localhostAddress(port: number) {
3939
}
4040

4141
export async function startDevserver(input: DevserverStartToolInput, context: McpToolContext) {
42-
const projectKey = devserverKey(input.project);
42+
const projectName = input.project ?? getDefaultProjectName(context);
4343

44-
let devserver = context.devservers.get(projectKey);
44+
if (!projectName) {
45+
return createStructuredContentOutput({
46+
message: ['Project name not provided, and no default project found.'],
47+
});
48+
}
49+
50+
let devserver = context.devservers.get(projectName);
4551
if (devserver) {
4652
return createStructuredContentOutput({
47-
message: `Development server for project '${projectKey}' is already running.`,
53+
message: `Development server for project '${projectName}' is already running.`,
4854
address: localhostAddress(devserver.port),
4955
});
5056
}
@@ -54,10 +60,10 @@ export async function startDevserver(input: DevserverStartToolInput, context: Mc
5460
devserver = new LocalDevserver({ host: context.host, project: input.project, port });
5561
devserver.start();
5662

57-
context.devservers.set(projectKey, devserver);
63+
context.devservers.set(projectName, devserver);
5864

5965
return createStructuredContentOutput({
60-
message: `Development server for project '${projectKey}' started and watching for workspace changes.`,
66+
message: `Development server for project '${projectName}' started and watching for workspace changes.`,
6167
address: localhostAddress(port),
6268
});
6369
}

packages/angular/cli/src/commands/mcp/tools/devserver/devserver-stop.ts

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@
77
*/
88

99
import { z } from 'zod';
10-
import { devserverKey } from '../../devserver';
11-
import { createStructuredContentOutput } from '../../utils';
10+
import { createStructuredContentOutput, getDefaultProjectName } from '../../utils';
1211
import { type McpToolContext, type McpToolDeclaration, declareTool } from '../tool-registry';
1312

1413
const devserverStopToolInputSchema = z.object({
@@ -30,21 +29,41 @@ const devserverStopToolOutputSchema = z.object({
3029
export type DevserverStopToolOutput = z.infer<typeof devserverStopToolOutputSchema>;
3130

3231
export function stopDevserver(input: DevserverStopToolInput, context: McpToolContext) {
33-
const projectKey = devserverKey(input.project);
34-
const devServer = context.devservers.get(projectKey);
32+
if (context.devservers.size === 0) {
33+
return createStructuredContentOutput({
34+
message: ['No development servers are currently running.'],
35+
logs: undefined,
36+
});
37+
}
38+
39+
let projectName = input.project ?? getDefaultProjectName(context);
40+
41+
if (!projectName) {
42+
// This should not happen. But if there's just a single running devserver, stop it.
43+
if (context.devservers.size === 1) {
44+
projectName = Array.from(context.devservers.keys())[0];
45+
} else {
46+
return createStructuredContentOutput({
47+
message: ['Project name not provided, and no default project found.'],
48+
logs: undefined,
49+
});
50+
}
51+
}
52+
53+
const devServer = context.devservers.get(projectName);
3554

3655
if (!devServer) {
3756
return createStructuredContentOutput({
38-
message: `Development server for project '${projectKey}' was not running.`,
57+
message: `Development server for project '${projectName}' was not running.`,
3958
logs: undefined,
4059
});
4160
}
4261

4362
devServer.stop();
44-
context.devservers.delete(projectKey);
63+
context.devservers.delete(projectName);
4564

4665
return createStructuredContentOutput({
47-
message: `Development server for project '${projectKey}' stopped.`,
66+
message: `Development server for project '${projectName}' stopped.`,
4867
logs: devServer.getServerLogs(),
4968
});
5069
}

packages/angular/cli/src/commands/mcp/tools/devserver/devserver-wait-for-build.ts

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@
77
*/
88

99
import { z } from 'zod';
10-
import { devserverKey } from '../../devserver';
11-
import { createStructuredContentOutput } from '../../utils';
10+
import { createStructuredContentOutput, getDefaultProjectName } from '../../utils';
1211
import { type McpToolContext, type McpToolDeclaration, declareTool } from '../tool-registry';
1312

1413
/**
@@ -60,21 +59,43 @@ export async function waitForDevserverBuild(
6059
input: DevserverWaitForBuildToolInput,
6160
context: McpToolContext,
6261
) {
63-
const projectKey = devserverKey(input.project);
64-
const devServer = context.devservers.get(projectKey);
65-
const deadline = Date.now() + input.timeout;
62+
if (context.devservers.size === 0) {
63+
return createStructuredContentOutput({
64+
status: 'no_devserver_found',
65+
logs: undefined,
66+
});
67+
}
68+
69+
let projectName = input.project ?? getDefaultProjectName(context);
70+
71+
if (!projectName) {
72+
// This should not happen. But if there's just a single running devserver, wait for it.
73+
if (context.devservers.size === 1) {
74+
projectName = Array.from(context.devservers.keys())[0];
75+
} else {
76+
return createStructuredContentOutput({
77+
status: 'no_devserver_found',
78+
logs: undefined,
79+
});
80+
}
81+
}
82+
83+
const devServer = context.devservers.get(projectName);
6684

6785
if (!devServer) {
6886
return createStructuredContentOutput<DevserverWaitForBuildToolOutput>({
6987
status: 'no_devserver_found',
88+
logs: undefined,
7089
});
7190
}
7291

92+
const deadline = Date.now() + input.timeout;
7393
await wait(WATCH_DELAY);
7494
while (devServer.isBuilding()) {
7595
if (Date.now() > deadline) {
7696
return createStructuredContentOutput<DevserverWaitForBuildToolOutput>({
7797
status: 'timeout',
98+
logs: undefined,
7899
});
79100
}
80101
await wait(WATCH_DELAY);

packages/angular/cli/src/commands/mcp/tools/devserver/devserver_spec.ts

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import { EventEmitter } from 'events';
1010
import type { ChildProcess } from 'node:child_process';
1111
import type { MockHost } from '../../testing/mock-host';
12+
import { addProjectToWorkspace, createMockContext } from '../../testing/test-utils';
1213
import type { McpToolContext } from '../tool-registry';
1314
import { startDevserver } from './devserver-start';
1415
import { stopDevserver } from './devserver-stop';
@@ -29,29 +30,30 @@ describe('Serve Tools', () => {
2930
beforeEach(() => {
3031
portCounter = 12345;
3132
mockProcess = new MockChildProcess();
32-
mockHost = {
33-
spawn: jasmine.createSpy('spawn').and.returnValue(mockProcess as unknown as ChildProcess),
34-
getAvailablePort: jasmine.createSpy('getAvailablePort').and.callFake(() => {
35-
return Promise.resolve(portCounter++);
36-
}),
37-
} as MockHost;
38-
39-
mockContext = {
40-
devservers: new Map(),
41-
host: mockHost,
42-
} as Partial<McpToolContext> as McpToolContext;
33+
34+
const mock = createMockContext();
35+
mockHost = mock.host;
36+
mockContext = mock.context;
37+
38+
// Customize host spies
39+
mockHost.spawn.and.returnValue(mockProcess as unknown as ChildProcess);
40+
mockHost.getAvailablePort.and.callFake(() => Promise.resolve(portCounter++));
41+
42+
// Setup default project
43+
addProjectToWorkspace(mock.projects, 'my-app');
44+
mockContext.workspace!.extensions['defaultProject'] = 'my-app';
4345
});
4446

4547
it('should start and stop a dev server', async () => {
4648
const startResult = await startDevserver({}, mockContext);
4749
expect(startResult.structuredContent.message).toBe(
48-
`Development server for project '<default>' started and watching for workspace changes.`,
50+
`Development server for project 'my-app' started and watching for workspace changes.`,
4951
);
5052
expect(mockHost.spawn).toHaveBeenCalledWith('ng', ['serve', '--port=12345'], { stdio: 'pipe' });
5153

5254
const stopResult = stopDevserver({}, mockContext);
5355
expect(stopResult.structuredContent.message).toBe(
54-
`Development server for project '<default>' stopped.`,
56+
`Development server for project 'my-app' stopped.`,
5557
);
5658
expect(mockProcess.kill).toHaveBeenCalled();
5759
});
@@ -78,6 +80,11 @@ describe('Serve Tools', () => {
7880
});
7981

8082
it('should handle multiple dev servers', async () => {
83+
// Add extra projects
84+
const projects = mockContext.workspace!.projects;
85+
addProjectToWorkspace(projects, 'app-one');
86+
addProjectToWorkspace(projects, 'app-two');
87+
8188
// Start server for project 1. This uses the basic mockProcess created for the tests.
8289
const startResult1 = await startDevserver({ project: 'app-one' }, mockContext);
8390
expect(startResult1.structuredContent.message).toBe(
@@ -117,6 +124,7 @@ describe('Serve Tools', () => {
117124
});
118125

119126
it('should handle server crash', async () => {
127+
addProjectToWorkspace(mockContext.workspace!.projects, 'crash-app');
120128
await startDevserver({ project: 'crash-app' }, mockContext);
121129

122130
// Simulate a crash with exit code 1
@@ -129,6 +137,7 @@ describe('Serve Tools', () => {
129137
});
130138

131139
it('wait should timeout if build takes too long', async () => {
140+
addProjectToWorkspace(mockContext.workspace!.projects, 'timeout-app');
132141
await startDevserver({ project: 'timeout-app' }, mockContext);
133142
const waitResult = await waitForDevserverBuild(
134143
{ project: 'timeout-app', timeout: 10 },

0 commit comments

Comments
 (0)