MediaMetz/gemini-cli
1
0
Fork 0
mirror of https://github.com/google-gemini/gemini-cli.git synced 2026-03-17 01:21:10 -07:00
Code Issues Packages Projects Releases Wiki Activity

Use consistent param names (#12517)

Browse Source
This commit is contained in:
Tommaso Sciortino
2025-11-06 15:03:52 -08:00
committed by GitHub
parent 5f1208ad81
commit f05d937f39
27 changed files with 553 additions and 525 deletions
Download Patch File Download Diff File Expand all files Collapse all files

124
packages/core/src/tools/shell.test.ts
View File

@@ -15,12 +15,24 @@ import {
type Mock,
} from 'vitest';
const mockPlatform = vi.hoisted(() => vi.fn());
const mockShellExecutionService = vi.hoisted(() => vi.fn());
vi.mock('../services/shellExecutionService.js', () => ({
ShellExecutionService: { execute: mockShellExecutionService },
}));
vi.mock('fs');
vi.mock('os');
vi.mock('node:os', async (importOriginal) => {
const actualOs = await importOriginal<typeof os>();
return {
...actualOs,
default: {
...actualOs,
platform: mockPlatform,
},
platform: mockPlatform,
};
});
vi.mock('crypto');
vi.mock('../utils/summarizer.js');
@@ -43,32 +55,40 @@ import * as summarizer from '../utils/summarizer.js';
import { ToolErrorType } from './tool-error.js';
import { ToolConfirmationOutcome } from './tools.js';
import { OUTPUT_UPDATE_INTERVAL_MS } from './shell.js';
import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js';
import { SHELL_TOOL_NAME } from './tool-names.js';
import { WorkspaceContext } from '../utils/workspaceContext.js';
const originalComSpec = process.env['ComSpec'];
const itWindowsOnly = process.platform === 'win32' ? it : it.skip;
describe('ShellTool', () => {
beforeAll(async () => {
await initializeShellParsers();
});
let shellTool: ShellTool;
let mockConfig: Config;
let mockShellOutputCallback: (event: ShellOutputEvent) => void;
let resolveExecutionPromise: (result: ShellExecutionResult) => void;
let tempRootDir: string;
beforeEach(() => {
vi.clearAllMocks();
tempRootDir = fs.mkdtempSync(path.join(os.tmpdir(), 'shell-test-'));
fs.mkdirSync(path.join(tempRootDir, 'subdir'));
mockConfig = {
getAllowedTools: vi.fn().mockReturnValue([]),
getApprovalMode: vi.fn().mockReturnValue('strict'),
getCoreTools: vi.fn().mockReturnValue([]),
getExcludeTools: vi.fn().mockReturnValue([]),
getDebugMode: vi.fn().mockReturnValue(false),
getTargetDir: vi.fn().mockReturnValue('/test/dir'),
getTargetDir: vi.fn().mockReturnValue(tempRootDir),
getSummarizeToolOutputConfig: vi.fn().mockReturnValue(undefined),
getWorkspaceContext: vi
.fn()
.mockReturnValue(createMockWorkspaceContext('/test/dir')),
.mockReturnValue(new WorkspaceContext(tempRootDir)),
getGeminiClient: vi.fn(),
getEnableInteractiveShell: vi.fn().mockReturnValue(false),
isInteractive: vi.fn().mockReturnValue(true),
@@ -76,8 +96,7 @@ describe('ShellTool', () => {
shellTool = new ShellTool(mockConfig);
vi.mocked(os.platform).mockReturnValue('linux');
vi.mocked(os.tmpdir).mockReturnValue('/tmp');
mockPlatform.mockReturnValue('linux');
(vi.mocked(crypto.randomBytes) as Mock).mockReturnValue(
Buffer.from('abcdef', 'hex'),
);
@@ -97,6 +116,9 @@ describe('ShellTool', () => {
});
afterEach(() => {
if (fs.existsSync(tempRootDir)) {
fs.rmSync(tempRootDir, { recursive: true, force: true });
}
if (originalComSpec === undefined) {
delete process.env['ComSpec'];
} else {
@@ -135,30 +157,27 @@ describe('ShellTool', () => {
);
});
it('should throw an error for a relative directory path', () => {
expect(() =>
shellTool.build({ command: 'ls', directory: 'rel/path' }),
).toThrow('Directory must be an absolute path.');
it('should return an invocation for a valid relative directory path', () => {
const invocation = shellTool.build({
command: 'ls',
dir_path: 'subdir',
});
expect(invocation).toBeDefined();
});
it('should throw an error for a directory outside the workspace', () => {
(mockConfig.getWorkspaceContext as Mock).mockReturnValue(
createMockWorkspaceContext('/test/dir', ['/another/workspace']),
);
const outsidePath = path.resolve(tempRootDir, '../outside');
expect(() =>
shellTool.build({ command: 'ls', directory: '/not/in/workspace' }),
shellTool.build({ command: 'ls', dir_path: outsidePath }),
).toThrow(
"Directory '/not/in/workspace' is not within any of the registered workspace directories.",
`Directory '${outsidePath}' is not within any of the registered workspace directories.`,
);
});
it('should return an invocation for a valid absolute directory path', () => {
(mockConfig.getWorkspaceContext as Mock).mockReturnValue(
createMockWorkspaceContext('/test/dir', ['/another/workspace']),
);
const invocation = shellTool.build({
command: 'ls',
directory: '/test/dir/subdir',
dir_path: path.join(tempRootDir, 'subdir'),
});
expect(invocation).toBeDefined();
});
@@ -189,32 +208,31 @@ describe('ShellTool', () => {
const promise = invocation.execute(mockAbortSignal);
resolveShellExecution({ pid: 54321 });
vi.mocked(fs.existsSync).mockReturnValue(true);
vi.mocked(fs.readFileSync).mockReturnValue(`54321${EOL}54322${EOL}`); // Service PID and background PID
// Simulate pgrep output file creation by the shell command
const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp');
fs.writeFileSync(tmpFile, `54321${EOL}54322${EOL}`);
const result = await promise;
const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp');
const wrappedCommand = `{ my-command & }; __code=$?; pgrep -g 0 >${tmpFile} 2>&1; exit $__code;`;
expect(mockShellExecutionService).toHaveBeenCalledWith(
wrappedCommand,
'/test/dir',
tempRootDir,
expect.any(Function),
mockAbortSignal,
false,
{},
);
expect(result.llmContent).toContain('Background PIDs: 54322');
expect(vi.mocked(fs.unlinkSync)).toHaveBeenCalledWith(tmpFile);
// The file should be deleted by the tool
expect(fs.existsSync(tmpFile)).toBe(false);
});
it('should use the provided directory as cwd', async () => {
(mockConfig.getWorkspaceContext as Mock).mockReturnValue(
createMockWorkspaceContext('/test/dir'),
);
it('should use the provided absolute directory as cwd', async () => {
const subdir = path.join(tempRootDir, 'subdir');
const invocation = shellTool.build({
command: 'ls',
directory: '/test/dir/subdir',
dir_path: subdir,
});
const promise = invocation.execute(mockAbortSignal);
resolveShellExecution();
@@ -224,7 +242,28 @@ describe('ShellTool', () => {
const wrappedCommand = `{ ls; }; __code=$?; pgrep -g 0 >${tmpFile} 2>&1; exit $__code;`;
expect(mockShellExecutionService).toHaveBeenCalledWith(
wrappedCommand,
'/test/dir/subdir',
subdir,
expect.any(Function),
mockAbortSignal,
false,
{},
);
});
it('should use the provided relative directory as cwd', async () => {
const invocation = shellTool.build({
command: 'ls',
dir_path: 'subdir',
});
const promise = invocation.execute(mockAbortSignal);
resolveShellExecution();
await promise;
const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp');
const wrappedCommand = `{ ls; }; __code=$?; pgrep -g 0 >${tmpFile} 2>&1; exit $__code;`;
expect(mockShellExecutionService).toHaveBeenCalledWith(
wrappedCommand,
path.join(tempRootDir, 'subdir'),
expect.any(Function),
mockAbortSignal,
false,
@@ -235,7 +274,7 @@ describe('ShellTool', () => {
itWindowsOnly(
'should not wrap command on windows',
async () => {
vi.mocked(os.platform).mockReturnValue('win32');
mockPlatform.mockReturnValue('win32');
const invocation = shellTool.build({ command: 'dir' });
const promise = invocation.execute(mockAbortSignal);
resolveShellExecution({
@@ -251,7 +290,7 @@ describe('ShellTool', () => {
await promise;
expect(mockShellExecutionService).toHaveBeenCalledWith(
'dir',
'/test/dir',
tempRootDir,
expect.any(Function),
mockAbortSignal,
false,
@@ -303,12 +342,6 @@ describe('ShellTool', () => {
);
});
it('should throw an error for invalid directory', () => {
expect(() =>
shellTool.build({ command: 'ls', directory: 'nonexistent' }),
).toThrow('Directory must be an absolute path.');
});
it('should summarize output when configured', async () => {
(mockConfig.getSummarizeToolOutputConfig as Mock).mockReturnValue({
[SHELL_TOOL_NAME]: { tokenBudget: 1000 },
@@ -345,15 +378,17 @@ describe('ShellTool', () => {
it('should clean up the temp file on synchronous execution error', async () => {
const error = new Error('sync spawn error');
mockShellExecutionService.mockImplementation(() => {
// Create the temp file before throwing to simulate it being left behind
const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp');
fs.writeFileSync(tmpFile, '');
throw error;
});
vi.mocked(fs.existsSync).mockReturnValue(true); // Pretend the file exists
const invocation = shellTool.build({ command: 'a-command' });
await expect(invocation.execute(mockAbortSignal)).rejects.toThrow(error);
const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp');
expect(vi.mocked(fs.unlinkSync)).toHaveBeenCalledWith(tmpFile);
expect(fs.existsSync(tmpFile)).toBe(false);
});
describe('Streaming to `updateOutput`', () => {
@@ -501,18 +536,15 @@ describe('ShellTool', () => {
describe('getDescription', () => {
it('should return the windows description when on windows', () => {
vi.mocked(os.platform).mockReturnValue('win32');
mockPlatform.mockReturnValue('win32');
const shellTool = new ShellTool(mockConfig);
expect(shellTool.description).toMatchSnapshot();
});
it('should return the non-windows description when not on windows', () => {
vi.mocked(os.platform).mockReturnValue('linux');
mockPlatform.mockReturnValue('linux');
const shellTool = new ShellTool(mockConfig);
expect(shellTool.description).toMatchSnapshot();
});
});
});
beforeAll(async () => {
await initializeShellParsers();
});