From 8da47db1a4e4bbc340fc2c222a08c1a25e8d10b5 Mon Sep 17 00:00:00 2001 From: Sandy Tao Date: Fri, 17 Oct 2025 11:23:26 -0700 Subject: [PATCH] fix(cli): enable and fix types for MCP command tests (#11385) --- packages/cli/src/commands/mcp.test.ts | 39 ++++++++++++++++---- packages/cli/src/commands/mcp/add.test.ts | 15 ++++---- packages/cli/src/commands/mcp/remove.test.ts | 22 ++++++++--- packages/cli/tsconfig.json | 4 -- 4 files changed, 55 insertions(+), 25 deletions(-) diff --git a/packages/cli/src/commands/mcp.test.ts b/packages/cli/src/commands/mcp.test.ts index b4e9980cb9..f94da0febf 100644 --- a/packages/cli/src/commands/mcp.test.ts +++ b/packages/cli/src/commands/mcp.test.ts @@ -17,15 +17,38 @@ describe('mcp command', () => { expect(typeof mcpCommand.handler).toBe('function'); }); - it('should have exactly one option (help flag)', () => { - // Test to ensure that the global 'gemini' flags are not added to the mcp command + it('should show help when no subcommand is provided', async () => { const yargsInstance = yargs(); - const builtYargs = mcpCommand.builder(yargsInstance); - const options = builtYargs.getOptions(); + (mcpCommand.builder as (y: Argv) => Argv)(yargsInstance); - // Should have exactly 1 option (help flag) - expect(Object.keys(options.key).length).toBe(1); - expect(options.key).toHaveProperty('help'); + const parser = yargsInstance.command(mcpCommand).help(); + + // Mock console.log and console.error to catch help output + const consoleLogMock = vi + .spyOn(console, 'log') + .mockImplementation(() => {}); + const consoleErrorMock = vi + .spyOn(console, 'error') + .mockImplementation(() => {}); + + try { + await parser.parse('mcp'); + } catch (_error) { + // yargs might throw an error when demandCommand is not met + } + + // Check if help output is shown + const helpOutput = + consoleLogMock.mock.calls.join('\n') + + consoleErrorMock.mock.calls.join('\n'); + expect(helpOutput).toContain('Manage MCP servers'); + expect(helpOutput).toContain('Commands:'); + expect(helpOutput).toContain('add'); + expect(helpOutput).toContain('remove'); + expect(helpOutput).toContain('list'); + + consoleLogMock.mockRestore(); + consoleErrorMock.mockRestore(); }); it('should register add, remove, and list subcommands', () => { @@ -35,7 +58,7 @@ describe('mcp command', () => { version: vi.fn().mockReturnThis(), }; - mcpCommand.builder(mockYargs as unknown as Argv); + (mcpCommand.builder as (y: Argv) => Argv)(mockYargs as unknown as Argv); expect(mockYargs.command).toHaveBeenCalledTimes(3); diff --git a/packages/cli/src/commands/mcp/add.test.ts b/packages/cli/src/commands/mcp/add.test.ts index 357235b5fa..369d935185 100644 --- a/packages/cli/src/commands/mcp/add.test.ts +++ b/packages/cli/src/commands/mcp/add.test.ts @@ -4,7 +4,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import yargs from 'yargs'; +import { describe, it, expect, vi, type Mock } from 'vitest'; +import yargs, { type Argv } from 'yargs'; import { addCommand } from './add.js'; import { loadSettings, SettingScope } from '../../config/settings.js'; @@ -31,12 +32,12 @@ vi.mock('../../config/settings.js', async () => { }; }); -const mockedLoadSettings = loadSettings as vi.Mock; +const mockedLoadSettings = loadSettings as Mock; describe('mcp add command', () => { - let parser: yargs.Argv; - let mockSetValue: vi.Mock; - let mockConsoleError: vi.Mock; + let parser: Argv; + let mockSetValue: Mock; + let mockConsoleError: Mock; beforeEach(() => { vi.resetAllMocks(); @@ -207,7 +208,7 @@ describe('mcp add command', () => { .spyOn(process, 'exit') .mockImplementation((() => { throw new Error('process.exit called'); - }) as (code?: number) => never); + }) as (code?: number | string | null) => never); await expect( parser.parseAsync(`add ${serverName} ${command}`), @@ -225,7 +226,7 @@ describe('mcp add command', () => { .spyOn(process, 'exit') .mockImplementation((() => { throw new Error('process.exit called'); - }) as (code?: number) => never); + }) as (code?: number | string | null) => never); await expect( parser.parseAsync(`add --scope project ${serverName} ${command}`), diff --git a/packages/cli/src/commands/mcp/remove.test.ts b/packages/cli/src/commands/mcp/remove.test.ts index 3886832f13..34b12ed324 100644 --- a/packages/cli/src/commands/mcp/remove.test.ts +++ b/packages/cli/src/commands/mcp/remove.test.ts @@ -4,8 +4,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'; -import yargs from 'yargs'; +import { + vi, + describe, + it, + expect, + beforeEach, + afterEach, + type Mock, +} from 'vitest'; +import yargs, { type Argv } from 'yargs'; import { SettingScope, type LoadedSettings } from '../../config/settings.js'; import { removeCommand } from './remove.js'; import * as fs from 'node:fs'; @@ -20,8 +28,8 @@ vi.mock('fs/promises', () => ({ describe('mcp remove command', () => { describe('unit tests with mocks', () => { - let parser: yargs.Argv; - let mockSetValue: vi.Mock; + let parser: Argv; + let mockSetValue: Mock; let mockSettings: Record; beforeEach(async () => { @@ -42,7 +50,9 @@ describe('mcp remove command', () => { ).mockReturnValue({ forScope: () => ({ settings: mockSettings }), setValue: mockSetValue, - } as Partial as LoadedSettings); + workspace: { path: '/path/to/project' }, + user: { path: '/home/user' }, + } as unknown as LoadedSettings); const yargsInstance = yargs([]).command(removeCommand); parser = yargsInstance; @@ -73,7 +83,7 @@ describe('mcp remove command', () => { let tempDir: string; let settingsDir: string; let settingsPath: string; - let parser: yargs.Argv; + let parser: Argv; let cwdSpy: ReturnType; beforeEach(() => { diff --git a/packages/cli/tsconfig.json b/packages/cli/tsconfig.json index 9d4afbdfbf..e73b51ab8b 100644 --- a/packages/cli/tsconfig.json +++ b/packages/cli/tsconfig.json @@ -17,10 +17,6 @@ "node_modules", "dist", // TODO(5691): Fix type errors and remove excludes. - "src/commands/mcp.test.ts", - "src/commands/mcp/add.test.ts", - "src/commands/mcp/list.test.ts", - "src/commands/mcp/remove.test.ts", "src/config/config.integration.test.ts", "src/config/config.test.ts", "src/config/extension.test.ts",