Skip to content

Commit dd42893

Browse files
SandyTao520SandyTao520
authored
fix(config): Enable type checking for config tests (#11436)
1 parent 725b312 commit dd42893

4 files changed

Lines changed: 70 additions & 73 deletions

File tree

‎packages/cli/src/config/config.integration.test.ts‎

Lines changed: 37 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,12 @@ import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest';
88
import * as fs from 'node:fs';
99
import * as path from 'node:path';
1010
import { tmpdir } from 'node:os';
11-
import type {
12-
ConfigParameters,
13-
ContentGeneratorConfig,
14-
} from '@google/gemini-cli-core';
11+
import type { ConfigParameters } from '@google/gemini-cli-core';
1512
import {
1613
Config,
1714
DEFAULT_FILE_FILTERING_OPTIONS,
1815
} from '@google/gemini-cli-core';
16+
import type { Settings } from './settingsSchema.js';
1917
import { http, HttpResponse } from 'msw';
2018
import { setupServer } from 'msw/node';
2119

@@ -36,12 +34,6 @@ afterAll(() => {
3634

3735
const CLEARCUT_URL = 'https://play.googleapis.com/log';
3836

39-
const TEST_CONTENT_GENERATOR_CONFIG: ContentGeneratorConfig = {
40-
apiKey: 'test-key',
41-
model: 'test-model',
42-
userAgent: 'test-agent',
43-
};
44-
4537
// Mock file discovery service and tool registry
4638
vi.mock('@google/gemini-cli-core', async () => {
4739
const actual = await vi.importActual('@google/gemini-cli-core');
@@ -75,13 +67,13 @@ describe('Configuration Integration Tests', () => {
7567
describe('File Filtering Configuration', () => {
7668
it('should load default file filtering settings', async () => {
7769
const configParams: ConfigParameters = {
70+
sessionId: 'test-session',
7871
cwd: '/tmp',
79-
contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
72+
model: 'test-model',
8073
embeddingModel: 'test-embedding-model',
81-
sandbox: false,
74+
sandbox: undefined,
8275
targetDir: tempDir,
8376
debugMode: false,
84-
fileFilteringRespectGitIgnore: undefined, // Should default to DEFAULT_FILE_FILTERING_OPTIONS
8577
};
8678

8779
const config = new Config(configParams);
@@ -93,10 +85,11 @@ describe('Configuration Integration Tests', () => {
9385

9486
it('should load custom file filtering settings from configuration', async () => {
9587
const configParams: ConfigParameters = {
88+
sessionId: 'test-session',
9689
cwd: '/tmp',
97-
contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
90+
model: 'test-model',
9891
embeddingModel: 'test-embedding-model',
99-
sandbox: false,
92+
sandbox: undefined,
10093
targetDir: tempDir,
10194
debugMode: false,
10295
fileFiltering: {
@@ -111,10 +104,11 @@ describe('Configuration Integration Tests', () => {
111104

112105
it('should merge user and workspace file filtering settings', async () => {
113106
const configParams: ConfigParameters = {
107+
sessionId: 'test-session',
114108
cwd: '/tmp',
115-
contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
109+
model: 'test-model',
116110
embeddingModel: 'test-embedding-model',
117-
sandbox: false,
111+
sandbox: undefined,
118112
targetDir: tempDir,
119113
debugMode: false,
120114
fileFiltering: {
@@ -131,10 +125,11 @@ describe('Configuration Integration Tests', () => {
131125
describe('Configuration Integration', () => {
132126
it('should handle partial configuration objects gracefully', async () => {
133127
const configParams: ConfigParameters = {
128+
sessionId: 'test-session',
134129
cwd: '/tmp',
135-
contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
130+
model: 'test-model',
136131
embeddingModel: 'test-embedding-model',
137-
sandbox: false,
132+
sandbox: undefined,
138133
targetDir: tempDir,
139134
debugMode: false,
140135
fileFiltering: {
@@ -150,10 +145,11 @@ describe('Configuration Integration Tests', () => {
150145

151146
it('should handle empty configuration objects gracefully', async () => {
152147
const configParams: ConfigParameters = {
148+
sessionId: 'test-session',
153149
cwd: '/tmp',
154-
contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
150+
model: 'test-model',
155151
embeddingModel: 'test-embedding-model',
156-
sandbox: false,
152+
sandbox: undefined,
157153
targetDir: tempDir,
158154
debugMode: false,
159155
fileFiltering: {},
@@ -169,10 +165,11 @@ describe('Configuration Integration Tests', () => {
169165

170166
it('should handle missing configuration sections gracefully', async () => {
171167
const configParams: ConfigParameters = {
168+
sessionId: 'test-session',
172169
cwd: '/tmp',
173-
contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
170+
model: 'test-model',
174171
embeddingModel: 'test-embedding-model',
175-
sandbox: false,
172+
sandbox: undefined,
176173
targetDir: tempDir,
177174
debugMode: false,
178175
// Missing fileFiltering configuration
@@ -190,10 +187,11 @@ describe('Configuration Integration Tests', () => {
190187
describe('Real-world Configuration Scenarios', () => {
191188
it('should handle a security-focused configuration', async () => {
192189
const configParams: ConfigParameters = {
190+
sessionId: 'test-session',
193191
cwd: '/tmp',
194-
contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
192+
model: 'test-model',
195193
embeddingModel: 'test-embedding-model',
196-
sandbox: false,
194+
sandbox: undefined,
197195
targetDir: tempDir,
198196
debugMode: false,
199197
fileFiltering: {
@@ -208,10 +206,11 @@ describe('Configuration Integration Tests', () => {
208206

209207
it('should handle a CI/CD environment configuration', async () => {
210208
const configParams: ConfigParameters = {
209+
sessionId: 'test-session',
211210
cwd: '/tmp',
212-
contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
211+
model: 'test-model',
213212
embeddingModel: 'test-embedding-model',
214-
sandbox: false,
213+
sandbox: undefined,
215214
targetDir: tempDir,
216215
debugMode: false,
217216
fileFiltering: {
@@ -228,10 +227,11 @@ describe('Configuration Integration Tests', () => {
228227
describe('Checkpointing Configuration', () => {
229228
it('should enable checkpointing when the setting is true', async () => {
230229
const configParams: ConfigParameters = {
230+
sessionId: 'test-session',
231231
cwd: '/tmp',
232-
contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
232+
model: 'test-model',
233233
embeddingModel: 'test-embedding-model',
234-
sandbox: false,
234+
sandbox: undefined,
235235
targetDir: tempDir,
236236
debugMode: false,
237237
checkpointing: true,
@@ -246,10 +246,11 @@ describe('Configuration Integration Tests', () => {
246246
describe('Extension Context Files', () => {
247247
it('should have an empty array for extension context files by default', () => {
248248
const configParams: ConfigParameters = {
249+
sessionId: 'test-session',
249250
cwd: '/tmp',
250-
contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
251+
model: 'test-model',
251252
embeddingModel: 'test-embedding-model',
252-
sandbox: false,
253+
sandbox: undefined,
253254
targetDir: tempDir,
254255
debugMode: false,
255256
};
@@ -260,10 +261,11 @@ describe('Configuration Integration Tests', () => {
260261
it('should correctly store and return extension context file paths', () => {
261262
const contextFiles = ['/path/to/file1.txt', '/path/to/file2.js'];
262263
const configParams: ConfigParameters = {
264+
sessionId: 'test-session',
263265
cwd: '/tmp',
264-
contentGeneratorConfig: TEST_CONTENT_GENERATOR_CONFIG,
266+
model: 'test-model',
265267
embeddingModel: 'test-embedding-model',
266-
sandbox: false,
268+
sandbox: undefined,
267269
targetDir: tempDir,
268270
debugMode: false,
269271
extensionContextFilePaths: contextFiles,
@@ -274,11 +276,11 @@ describe('Configuration Integration Tests', () => {
274276
});
275277

276278
describe('Approval Mode Integration Tests', () => {
277-
let parseArguments: typeof import('./config').parseArguments;
279+
let parseArguments: typeof import('./config.js').parseArguments;
278280

279281
beforeEach(async () => {
280282
// Import the argument parsing function for integration testing
281-
const { parseArguments: parseArgs } = await import('./config');
283+
const { parseArguments: parseArgs } = await import('./config.js');
282284
parseArguments = parseArgs;
283285
});
284286

‎packages/cli/src/config/config.test.ts‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,7 @@ import {
1616
type GeminiCLIExtension,
1717
SHELL_TOOL_NAME,
1818
} from '@google/gemini-cli-core';
19-
import {
20-
loadCliConfig,
21-
loadHierarchicalGeminiMemory,
22-
parseArguments,
23-
type CliArgs,
24-
} from './config.js';
19+
import { loadCliConfig, parseArguments, type CliArgs } from './config.js';
2520
import type { Settings } from './settings.js';
2621
import * as ServerConfig from '@google/gemini-cli-core';
2722
import { isWorkspaceTrusted } from './trustedFolders.js';
@@ -681,6 +676,9 @@ describe('Hierarchical Memory Loading (config.ts) - Placeholder Suite', () => {
681676
// 3. Spies on console functions (for logger output) are correctly set up if needed.
682677
// Example of a previously failing test structure:
683678
it.skip('should correctly use mocked homedir for global path', async () => {
679+
// This test is skipped because mockFs and fsPromises are not properly imported/mocked
680+
// TODO: Fix this test by properly setting up mock-fs and fs/promises mocks
681+
/*
684682
const MOCK_GEMINI_DIR_LOCAL = path.join(
685683
'/mock/home/user',
686684
ServerConfig.GEMINI_DIR,
@@ -699,6 +697,7 @@ describe('Hierarchical Memory Loading (config.ts) - Placeholder Suite', () => {
699697
MOCK_GLOBAL_PATH_LOCAL,
700698
'utf-8',
701699
);
700+
*/
702701
});
703702
});
704703

@@ -2567,8 +2566,11 @@ describe('loadCliConfig fileFiltering', () => {
25672566
vi.restoreAllMocks();
25682567
});
25692568

2569+
type FileFilteringSettings = NonNullable<
2570+
NonNullable<Settings['context']>['fileFiltering']
2571+
>;
25702572
const testCases: Array<{
2571-
property: keyof NonNullable<Settings['fileFiltering']>;
2573+
property: keyof FileFilteringSettings;
25722574
getter: (config: ServerConfig.Config) => boolean;
25732575
value: boolean;
25742576
}> = [

‎packages/cli/src/config/settings.test.ts‎

Lines changed: 24 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ import {
4646
afterEach,
4747
type Mocked,
4848
type Mock,
49-
fail,
5049
} from 'vitest';
5150
import * as fs from 'node:fs'; // fs will be mocked separately
5251
import stripJsonComments from 'strip-json-comments'; // Will be mocked separately
@@ -1266,7 +1265,7 @@ describe('Settings Loading and Merging', () => {
12661265

12671266
try {
12681267
loadSettings(MOCK_WORKSPACE_DIR);
1269-
fail('loadSettings should have thrown a FatalConfigError');
1268+
throw new Error('loadSettings should have thrown a FatalConfigError');
12701269
} catch (e) {
12711270
expect(e).toBeInstanceOf(FatalConfigError);
12721271
const error = e as FatalConfigError;
@@ -1336,9 +1335,10 @@ describe('Settings Loading and Merging', () => {
13361335
expect((settings.workspace.settings as TestSettings)['endpoint']).toBe(
13371336
'workspace_endpoint_from_env/api',
13381337
);
1339-
expect(
1340-
(settings.workspace.settings as TestSettings)['nested']['value'],
1341-
).toBe('workspace_endpoint_from_env');
1338+
const nested = (settings.workspace.settings as TestSettings)[
1339+
'nested'
1340+
] as Record<string, unknown>;
1341+
expect(nested['value']).toBe('workspace_endpoint_from_env');
13421342
expect((settings.merged as TestSettings)['endpoint']).toBe(
13431343
'workspace_endpoint_from_env/api',
13441344
);
@@ -1586,21 +1586,14 @@ describe('Settings Loading and Merging', () => {
15861586
(settings.user.settings as TestSettings)['undefinedVal'],
15871587
).toBeUndefined();
15881588

1589-
expect(
1590-
(settings.user.settings as TestSettings)['nestedObj']['nestedNull'],
1591-
).toBeNull();
1592-
expect(
1593-
(settings.user.settings as TestSettings)['nestedObj']['nestedBool'],
1594-
).toBe(true);
1595-
expect(
1596-
(settings.user.settings as TestSettings)['nestedObj']['nestedNum'],
1597-
).toBe(0);
1598-
expect(
1599-
(settings.user.settings as TestSettings)['nestedObj']['nestedString'],
1600-
).toBe('literal');
1601-
expect(
1602-
(settings.user.settings as TestSettings)['nestedObj']['anotherEnv'],
1603-
).toBe('env_string_nested_value');
1589+
const nestedObj = (settings.user.settings as TestSettings)[
1590+
'nestedObj'
1591+
] as Record<string, unknown>;
1592+
expect(nestedObj['nestedNull']).toBeNull();
1593+
expect(nestedObj['nestedBool']).toBe(true);
1594+
expect(nestedObj['nestedNum']).toBe(0);
1595+
expect(nestedObj['nestedString']).toBe('literal');
1596+
expect(nestedObj['anotherEnv']).toBe('env_string_nested_value');
16041597

16051598
delete process.env['MY_ENV_STRING'];
16061599
delete process.env['MY_ENV_STRING_NESTED'];
@@ -2136,14 +2129,14 @@ describe('Settings Loading and Merging', () => {
21362129
vimMode: false,
21372130
},
21382131
model: {
2139-
maxSessionTurns: 0,
2132+
maxSessionTurns: -1,
21402133
},
21412134
context: {
21422135
includeDirectories: [],
21432136
},
21442137
security: {
21452138
folderTrust: {
2146-
enabled: null,
2139+
enabled: undefined,
21472140
},
21482141
},
21492142
};
@@ -2152,9 +2145,13 @@ describe('Settings Loading and Merging', () => {
21522145

21532146
expect(v1Settings).toEqual({
21542147
vimMode: false,
2155-
maxSessionTurns: 0,
2148+
maxSessionTurns: -1,
21562149
includeDirectories: [],
2157-
folderTrust: null,
2150+
security: {
2151+
folderTrust: {
2152+
enabled: undefined,
2153+
},
2154+
},
21582155
});
21592156
});
21602157

@@ -2342,9 +2339,9 @@ describe('Settings Loading and Merging', () => {
23422339
});
23432340

23442341
describe('migrateDeprecatedSettings', () => {
2345-
let mockFsExistsSync: Mocked<typeof fs.existsSync>;
2346-
let mockFsReadFileSync: Mocked<typeof fs.readFileSync>;
2347-
let mockDisableExtension: Mocked<typeof disableExtension>;
2342+
let mockFsExistsSync: Mock;
2343+
let mockFsReadFileSync: Mock;
2344+
let mockDisableExtension: Mock;
23482345

23492346
beforeEach(() => {
23502347
vi.resetAllMocks();

‎packages/cli/tsconfig.json‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,6 @@
1717
"node_modules",
1818
"dist",
1919
// TODO(5691): Fix type errors and remove excludes.
20-
"src/config/config.integration.test.ts",
21-
"src/config/config.test.ts",
22-
"src/config/extension.test.ts",
23-
"src/config/settings.test.ts",
2420
"src/nonInteractiveCli.test.ts",
2521
"src/services/FileCommandLoader.test.ts",
2622
"src/services/prompt-processors/argumentProcessor.test.ts",

0 commit comments

Comments
 (0)