Eliminate createServerConfig() (#821)

This commit is contained in:
Tommaso Sciortino 2025-06-07 13:49:00 -07:00 committed by GitHub
parent 10b52ac4e8
commit d6cf4d5b0b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 5 additions and 87 deletions

View File

@ -34,7 +34,7 @@ vi.mock('@gemini-code/core', async () => {
return { return {
...actualServer, ...actualServer,
loadEnvironment: vi.fn(), loadEnvironment: vi.fn(),
createServerConfig: vi.fn((params) => ({ Config: vi.fn((params) => ({
// Mock the config object and its methods // Mock the config object and its methods
getApiKey: () => params.apiKey, getApiKey: () => params.apiKey,
getModel: () => params.model, getModel: () => params.model,

View File

@ -10,9 +10,7 @@ import process from 'node:process';
import { import {
Config, Config,
loadEnvironment, loadEnvironment,
createServerConfig,
loadServerHierarchicalMemory, loadServerHierarchicalMemory,
ConfigParameters,
setGeminiMdFilename as setServerGeminiMdFilename, setGeminiMdFilename as setServerGeminiMdFilename,
getCurrentGeminiMdFilename, getCurrentGeminiMdFilename,
ApprovalMode, ApprovalMode,
@ -175,7 +173,7 @@ export async function loadCliConfig(
modelToUse = await getEffectiveModel(apiKeyForServer, modelToUse); modelToUse = await getEffectiveModel(apiKeyForServer, modelToUse);
} }
const configParams: ConfigParameters = { return new Config({
apiKey: apiKeyForServer, apiKey: apiKeyForServer,
model: modelToUse, model: modelToUse,
embeddingModel: DEFAULT_GEMINI_EMBEDDING_MODEL, embeddingModel: DEFAULT_GEMINI_EMBEDDING_MODEL,
@ -206,7 +204,5 @@ export async function loadCliConfig(
fileFilteringRespectGitIgnore: settings.fileFiltering?.respectGitIgnore, fileFilteringRespectGitIgnore: settings.fileFiltering?.respectGitIgnore,
fileFilteringAllowBuildArtifacts: fileFilteringAllowBuildArtifacts:
settings.fileFiltering?.allowBuildArtifacts, settings.fileFiltering?.allowBuildArtifacts,
}; });
return createServerConfig(configParams);
} }

View File

@ -5,7 +5,7 @@
*/ */
import { describe, it, expect, vi, beforeEach } from 'vitest'; import { describe, it, expect, vi, beforeEach } from 'vitest';
import { Config, createServerConfig, ConfigParameters } from './config.js'; import { Config, ConfigParameters } from './config.js';
import * as path from 'path'; import * as path from 'path';
import { setGeminiMdFilename as mockSetGeminiMdFilename } from '../tools/memoryTool.js'; import { setGeminiMdFilename as mockSetGeminiMdFilename } from '../tools/memoryTool.js';
@ -87,51 +87,6 @@ describe('Server Config (config.ts)', () => {
expect(config.getUserMemory()).toBe(''); expect(config.getUserMemory()).toBe('');
}); });
it('createServerConfig should pass userMemory to Config constructor', () => {
const config = createServerConfig(baseParams);
// Check the result of the factory function
expect(config).toBeInstanceOf(Config);
expect(config.getUserMemory()).toBe(USER_MEMORY);
expect(config.getApiKey()).toBe(API_KEY);
expect(config.getUserAgent()).toBe(USER_AGENT);
});
it('createServerConfig should default userMemory if omitted', () => {
const paramsWithoutMemory: ConfigParameters = { ...baseParams };
delete paramsWithoutMemory.userMemory;
const config = createServerConfig(paramsWithoutMemory);
expect(config).toBeInstanceOf(Config);
expect(config.getUserMemory()).toBe(''); // Should default to empty string
});
it('createServerConfig should resolve targetDir', () => {
const relativeDir = './relative/path';
const expectedResolvedDir = path.resolve(relativeDir);
const paramsWithRelativeDir: ConfigParameters = {
...baseParams,
targetDir: relativeDir,
};
const config = createServerConfig(paramsWithRelativeDir);
expect(config.getTargetDir()).toBe(expectedResolvedDir);
});
it('createServerConfig should call setGeminiMdFilename with contextFileName if provided', () => {
const contextFileName = 'CUSTOM_AGENTS.md';
const paramsWithContextFile: ConfigParameters = {
...baseParams,
contextFileName,
};
createServerConfig(paramsWithContextFile);
expect(mockSetGeminiMdFilename).toHaveBeenCalledWith(contextFileName);
});
it('createServerConfig should not call setGeminiMdFilename if contextFileName is not provided', () => {
createServerConfig(baseParams); // baseParams does not have contextFileName
expect(mockSetGeminiMdFilename).not.toHaveBeenCalled();
});
it('Config constructor should call setGeminiMdFilename with contextFileName if provided', () => { it('Config constructor should call setGeminiMdFilename with contextFileName if provided', () => {
const contextFileName = 'CUSTOM_AGENTS.md'; const contextFileName = 'CUSTOM_AGENTS.md';
const paramsWithContextFile: ConfigParameters = { const paramsWithContextFile: ConfigParameters = {
@ -189,31 +144,6 @@ describe('Server Config (config.ts)', () => {
expect(config.getTelemetryEnabled()).toBe(TELEMETRY); expect(config.getTelemetryEnabled()).toBe(TELEMETRY);
}); });
it('createServerConfig should pass telemetry to Config constructor when true', () => {
const paramsWithTelemetry: ConfigParameters = {
...baseParams,
telemetry: true,
};
const config = createServerConfig(paramsWithTelemetry);
expect(config.getTelemetryEnabled()).toBe(true);
});
it('createServerConfig should pass telemetry to Config constructor when false', () => {
const paramsWithTelemetry: ConfigParameters = {
...baseParams,
telemetry: false,
};
const config = createServerConfig(paramsWithTelemetry);
expect(config.getTelemetryEnabled()).toBe(false);
});
it('createServerConfig should default telemetry (to false via Config constructor) if omitted', () => {
const paramsWithoutTelemetry: ConfigParameters = { ...baseParams };
delete paramsWithoutTelemetry.telemetry;
const config = createServerConfig(paramsWithoutTelemetry);
expect(config.getTelemetryEnabled()).toBe(TELEMETRY);
});
it('should have a getFileService method that returns FileDiscoveryService', async () => { it('should have a getFileService method that returns FileDiscoveryService', async () => {
const config = new Config(baseParams); const config = new Config(baseParams);
const fileService = await config.getFileService(); const fileService = await config.getFileService();

View File

@ -126,7 +126,7 @@ export class Config {
this.toolCallCommand = params.toolCallCommand; this.toolCallCommand = params.toolCallCommand;
this.mcpServerCommand = params.mcpServerCommand; this.mcpServerCommand = params.mcpServerCommand;
this.mcpServers = params.mcpServers; this.mcpServers = params.mcpServers;
this.userAgent = params.userAgent; this.userAgent = params.userAgent ?? 'GeminiCLI/unknown';
this.userMemory = params.userMemory ?? ''; this.userMemory = params.userMemory ?? '';
this.geminiMdFileCount = params.geminiMdFileCount ?? 0; this.geminiMdFileCount = params.geminiMdFileCount ?? 0;
this.approvalMode = params.approvalMode ?? ApprovalMode.DEFAULT; this.approvalMode = params.approvalMode ?? ApprovalMode.DEFAULT;
@ -329,14 +329,6 @@ export function loadEnvironment(): void {
} }
} }
export function createServerConfig(params: ConfigParameters): Config {
return new Config({
...params,
targetDir: path.resolve(params.targetDir), // Ensure targetDir is resolved
userAgent: params.userAgent ?? 'GeminiCLI/unknown', // Default user agent
});
}
export function createToolRegistry(config: Config): Promise<ToolRegistry> { export function createToolRegistry(config: Config): Promise<ToolRegistry> {
const registry = new ToolRegistry(config); const registry = new ToolRegistry(config);
const targetDir = config.getTargetDir(); const targetDir = config.getTargetDir();