Skip to content

Commit e4e2a25

Browse files
ehedlundehedlund
committed
test: fix Windows CI execution and resolve exposed platform failures (#24476)
1 parent 6fad1e9 commit e4e2a25

21 files changed

+308
-175
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -175,10 +175,10 @@ jobs:
175175
NO_COLOR: true
176176
run: |
177177
if [[ "${{ matrix.shard }}" == "cli" ]]; then
178-
npm run test:ci --workspace @google/gemini-cli
178+
npm run test:ci --workspace "@google/gemini-cli"
179179
else
180180
# Explicitly list non-cli packages to ensure they are sharded correctly
181-
npm run test:ci --workspace @google/gemini-cli-core --workspace @google/gemini-cli-a2a-server --workspace gemini-cli-vscode-ide-companion --workspace @google/gemini-cli-test-utils --if-present -- --coverage.enabled=false
181+
npm run test:ci --workspace "@google/gemini-cli-core" --workspace "@google/gemini-cli-a2a-server" --workspace "gemini-cli-vscode-ide-companion" --workspace "@google/gemini-cli-test-utils" --if-present -- --coverage.enabled=false
182182
npm run test:scripts
183183
fi
184184
@@ -263,10 +263,10 @@ jobs:
263263
NO_COLOR: true
264264
run: |
265265
if [[ "${{ matrix.shard }}" == "cli" ]]; then
266-
npm run test:ci --workspace @google/gemini-cli -- --coverage.enabled=false
266+
npm run test:ci --workspace "@google/gemini-cli" -- --coverage.enabled=false
267267
else
268268
# Explicitly list non-cli packages to ensure they are sharded correctly
269-
npm run test:ci --workspace @google/gemini-cli-core --workspace @google/gemini-cli-a2a-server --workspace gemini-cli-vscode-ide-companion --workspace @google/gemini-cli-test-utils --if-present -- --coverage.enabled=false
269+
npm run test:ci --workspace "@google/gemini-cli-core" --workspace "@google/gemini-cli-a2a-server" --workspace "gemini-cli-vscode-ide-companion" --workspace "@google/gemini-cli-test-utils" --if-present -- --coverage.enabled=false
270270
npm run test:scripts
271271
fi
272272
@@ -429,11 +429,14 @@ jobs:
429429
NODE_ENV: 'test'
430430
run: |
431431
if ("${{ matrix.shard }}" -eq "cli") {
432-
npm run test:ci --workspace @google/gemini-cli -- --coverage.enabled=false
432+
npm run test:ci --workspace "@google/gemini-cli" -- --coverage.enabled=false
433+
if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
433434
} else {
434435
# Explicitly list non-cli packages to ensure they are sharded correctly
435-
npm run test:ci --workspace @google/gemini-cli-core --workspace @google/gemini-cli-a2a-server --workspace gemini-cli-vscode-ide-companion --workspace @google/gemini-cli-test-utils --if-present -- --coverage.enabled=false
436+
npm run test:ci --workspace "@google/gemini-cli-core" --workspace "@google/gemini-cli-a2a-server" --workspace "gemini-cli-vscode-ide-companion" --workspace "@google/gemini-cli-test-utils" --if-present -- --coverage.enabled=false
437+
if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
436438
npm run test:scripts
439+
if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
437440
}
438441
shell: 'pwsh'
439442

‎packages/core/src/agents/auth-provider/api-key-provider.test.ts‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,20 @@
66

77
import { describe, it, expect, afterEach, vi } from 'vitest';
88
import { ApiKeyAuthProvider } from './api-key-provider.js';
9+
import * as resolver from './value-resolver.js';
10+
11+
vi.mock('./value-resolver.js', async (importOriginal) => {
12+
const actual = await importOriginal<typeof import('./value-resolver.js')>();
13+
return {
14+
...actual,
15+
resolveAuthValue: vi.fn(),
16+
};
17+
});
918

1019
describe('ApiKeyAuthProvider', () => {
1120
afterEach(() => {
1221
vi.unstubAllEnvs();
22+
vi.restoreAllMocks();
1323
});
1424

1525
describe('initialization', () => {
@@ -26,6 +36,7 @@ describe('ApiKeyAuthProvider', () => {
2636

2737
it('should resolve API key from environment variable', async () => {
2838
vi.stubEnv('TEST_API_KEY', 'env-api-key');
39+
vi.mocked(resolver.resolveAuthValue).mockResolvedValue('env-api-key');
2940

3041
const provider = new ApiKeyAuthProvider({
3142
type: 'apiKey',
@@ -38,6 +49,10 @@ describe('ApiKeyAuthProvider', () => {
3849
});
3950

4051
it('should throw if environment variable is not set', async () => {
52+
vi.mocked(resolver.resolveAuthValue).mockRejectedValue(
53+
new Error("Environment variable 'MISSING_KEY_12345' is not set"),
54+
);
55+
4156
const provider = new ApiKeyAuthProvider({
4257
type: 'apiKey',
4358
key: '$MISSING_KEY_12345',
@@ -114,6 +129,8 @@ describe('ApiKeyAuthProvider', () => {
114129

115130
it('should return undefined for env-var keys on 403', async () => {
116131
vi.stubEnv('RETRY_TEST_KEY', 'some-key');
132+
vi.mocked(resolver.resolveAuthValue).mockResolvedValue('some-key');
133+
117134
const provider = new ApiKeyAuthProvider({
118135
type: 'apiKey',
119136
key: '$RETRY_TEST_KEY',
@@ -128,9 +145,13 @@ describe('ApiKeyAuthProvider', () => {
128145
});
129146

130147
it('should re-resolve and return headers for command keys on 401', async () => {
148+
vi.mocked(resolver.resolveAuthValue)
149+
.mockResolvedValueOnce('initial-key')
150+
.mockResolvedValueOnce('refreshed-key');
151+
131152
const provider = new ApiKeyAuthProvider({
132153
type: 'apiKey',
133-
key: '!echo refreshed-key',
154+
key: '!some command',
134155
});
135156
await provider.initialize();
136157

@@ -142,9 +163,11 @@ describe('ApiKeyAuthProvider', () => {
142163
});
143164

144165
it('should stop retrying after MAX_AUTH_RETRIES', async () => {
166+
vi.mocked(resolver.resolveAuthValue).mockResolvedValue('rotating-key');
167+
145168
const provider = new ApiKeyAuthProvider({
146169
type: 'apiKey',
147-
key: '!echo rotating-key',
170+
key: '!some command',
148171
});
149172
await provider.initialize();
150173

‎packages/core/src/agents/auth-provider/value-resolver.test.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,16 @@ import {
1010
needsResolution,
1111
maskSensitiveValue,
1212
} from './value-resolver.js';
13+
import * as shellUtils from '../../utils/shell-utils.js';
14+
15+
vi.mock('../../utils/shell-utils.js', async (importOriginal) => {
16+
const actual =
17+
await importOriginal<typeof import('../../utils/shell-utils.js')>();
18+
return {
19+
...actual,
20+
spawnAsync: vi.fn(),
21+
};
22+
});
1323

1424
describe('value-resolver', () => {
1525
describe('resolveAuthValue', () => {
@@ -39,12 +49,24 @@ describe('value-resolver', () => {
3949
});
4050

4151
describe('shell commands', () => {
52+
afterEach(() => {
53+
vi.restoreAllMocks();
54+
});
55+
4256
it('should execute shell command with ! prefix', async () => {
57+
vi.mocked(shellUtils.spawnAsync).mockResolvedValue({
58+
stdout: 'hello\n',
59+
stderr: '',
60+
});
4361
const result = await resolveAuthValue('!echo hello');
4462
expect(result).toBe('hello');
4563
});
4664

4765
it('should trim whitespace from command output', async () => {
66+
vi.mocked(shellUtils.spawnAsync).mockResolvedValue({
67+
stdout: ' hello \n',
68+
stderr: '',
69+
});
4870
const result = await resolveAuthValue('!echo " hello "');
4971
expect(result).toBe('hello');
5072
});
@@ -56,16 +78,32 @@ describe('value-resolver', () => {
5678
});
5779

5880
it('should throw error for command that returns empty output', async () => {
81+
vi.mocked(shellUtils.spawnAsync).mockResolvedValue({
82+
stdout: '',
83+
stderr: '',
84+
});
5985
await expect(resolveAuthValue('!echo -n ""')).rejects.toThrow(
6086
'returned empty output',
6187
);
6288
});
6389

6490
it('should throw error for failed command', async () => {
91+
vi.mocked(shellUtils.spawnAsync).mockRejectedValue(
92+
new Error('Command failed'),
93+
);
6594
await expect(
6695
resolveAuthValue('!nonexistent-command-12345'),
6796
).rejects.toThrow(/Command.*failed/);
6897
});
98+
99+
it('should throw error for timeout', async () => {
100+
const timeoutError = new Error('AbortError');
101+
timeoutError.name = 'AbortError';
102+
vi.mocked(shellUtils.spawnAsync).mockRejectedValue(timeoutError);
103+
await expect(resolveAuthValue('!sleep 100')).rejects.toThrow(
104+
/timed out after/,
105+
);
106+
});
69107
});
70108

71109
describe('literal values', () => {

‎packages/core/src/agents/browser/browserManager.test.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,9 @@ describe('BrowserManager', () => {
159159
expect.objectContaining({
160160
command: 'node',
161161
args: expect.arrayContaining([
162-
expect.stringMatching(/bundled\/chrome-devtools-mcp\.mjs$/),
162+
expect.stringMatching(
163+
/(dist[\\/])?bundled[\\/]chrome-devtools-mcp\.mjs$/,
164+
),
163165
]),
164166
}),
165167
);
@@ -175,7 +177,7 @@ describe('BrowserManager', () => {
175177
command: 'node',
176178
args: expect.arrayContaining([
177179
expect.stringMatching(
178-
/(dist\/)?bundled\/chrome-devtools-mcp\.mjs$/,
180+
/(dist[\\/])?bundled[\\/]chrome-devtools-mcp\.mjs$/,
179181
),
180182
]),
181183
}),

‎packages/core/src/config/storage.test.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ describe('Storage - Security', () => {
103103
});
104104

105105
describe('Storage – additional helpers', () => {
106-
const projectRoot = '/tmp/project';
106+
const projectRoot = resolveToRealPath(path.resolve('/tmp/project'));
107107
const storage = new Storage(projectRoot);
108108

109109
beforeEach(() => {
@@ -308,9 +308,9 @@ describe('Storage – additional helpers', () => {
308308
},
309309
{
310310
name: 'custom absolute path outside throws',
311-
customDir: '/absolute/path/to/plans',
311+
customDir: path.resolve('/absolute/path/to/plans'),
312312
expected: '',
313-
expectedError: `Custom plans directory '/absolute/path/to/plans' resolves to '/absolute/path/to/plans', which is outside the project root '${resolveToRealPath(projectRoot)}'.`,
313+
expectedError: `Custom plans directory '${path.resolve('/absolute/path/to/plans')}' resolves to '${path.resolve('/absolute/path/to/plans')}', which is outside the project root '${resolveToRealPath(projectRoot)}'.`,
314314
},
315315
{
316316
name: 'absolute path that happens to be inside project root',
@@ -349,15 +349,14 @@ describe('Storage – additional helpers', () => {
349349
setup: () => {
350350
vi.mocked(fs.realpathSync).mockImplementation((p: fs.PathLike) => {
351351
if (p.toString().includes('symlink-to-outside')) {
352-
return '/outside/project/root';
352+
return path.resolve('/outside/project/root');
353353
}
354354
return p.toString();
355355
});
356356
return () => vi.mocked(fs.realpathSync).mockRestore();
357357
},
358358
expected: '',
359-
expectedError:
360-
"Custom plans directory 'symlink-to-outside' resolves to '/outside/project/root', which is outside the project root '/tmp/project'.",
359+
expectedError: `Custom plans directory 'symlink-to-outside' resolves to '${path.resolve('/outside/project/root')}', which is outside the project root '${resolveToRealPath(projectRoot)}'.`,
361360
},
362361
];
363362

‎packages/core/src/hooks/hookRunner.test.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,11 @@ describe('HookRunner', () => {
513513
const args = vi.mocked(spawn).mock.calls[
514514
executionOrder.length
515515
][1] as string[];
516-
const command = args[args.length - 1];
516+
let command = args[args.length - 1];
517+
// On Windows, the command is wrapped in PowerShell syntax
518+
if (command.includes('; if ($LASTEXITCODE -ne 0)')) {
519+
command = command.split(';')[0];
520+
}
517521
executionOrder.push(command);
518522
setImmediate(() => callback(0));
519523
}

0 commit comments

Comments
 (0)