From b0680820b68a320d5312c64bd8b215622b86bd30 Mon Sep 17 00:00:00 2001 From: Amaad Martin Date: Sun, 2 Aug 2026 23:45:07 -0700 Subject: [PATCH 1/2] Fix: pin GOOGLE_CLOUD_* in cli_create_test so gcloud defaults are testable getGcpProject()/getGcpRegion() read GOOGLE_CLOUD_PROJECT/GOOGLE_CLOUD_LOCATION before shelling out to gcloud, so on a developer machine that exports them the execSync mock in this suite was never consulted and the gcloud-defaults test asserted against the developer's real project id. Pin both variables to absent for the whole suite with vi.stubEnv(name, undefined) and restore them with vi.unstubAllEnvs() in afterEach (vi.restoreAllMocks() restores spies only). Add coverage for the two branches the empty CI environment never reached: env vars winning over gcloud, and gcloud being unavailable. No production code changes. --- dev/test/cli/cli_create_test.ts | 76 +++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/dev/test/cli/cli_create_test.ts b/dev/test/cli/cli_create_test.ts index 0fc37ed95..0149ead62 100644 --- a/dev/test/cli/cli_create_test.ts +++ b/dev/test/cli/cli_create_test.ts @@ -70,11 +70,16 @@ describe('createAgent', () => { beforeEach(() => { vi.clearAllMocks(); + // createAgent() reads these before shelling out to gcloud, so a developer's + // exported values would shadow the execSync mock. + vi.stubEnv('GOOGLE_CLOUD_PROJECT', undefined); + vi.stubEnv('GOOGLE_CLOUD_LOCATION', undefined); (isCancel as unknown as Mock).mockReturnValue(false); (listFiles as Mock).mockResolvedValue(['file1', 'file2']); }); afterEach(() => { + vi.unstubAllEnvs(); vi.restoreAllMocks(); }); @@ -213,15 +218,56 @@ describe('createAgent', () => { expect(text).toHaveBeenCalledWith( expect.objectContaining({ + message: 'Enter the Google Cloud Project ID', initialValue: 'gcloud-project', }), ); + expect(text).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Enter the Google Cloud Region', + initialValue: 'gcloud-region', + }), + ); expect(saveToFile).toHaveBeenCalledWith( expect.stringContaining('.env'), expect.stringContaining('GOOGLE_CLOUD_PROJECT=gcloud-project'), ); }); + it('should prefer the GCP environment variables over the gcloud defaults', async () => { + vi.stubEnv('GOOGLE_CLOUD_PROJECT', 'env-project'); + vi.stubEnv('GOOGLE_CLOUD_LOCATION', 'env-region'); + + (select as Mock).mockResolvedValueOnce('gemini-2.5-flash'); + (select as Mock).mockResolvedValueOnce('ts'); + (select as Mock).mockResolvedValueOnce('vertex'); // Backend + + (execSync as Mock).mockImplementation((cmd: string) => { + if (cmd.includes('project')) return 'gcloud-project\n'; + if (cmd.includes('region')) return 'gcloud-region\n'; + return ''; + }); + + (text as Mock).mockResolvedValueOnce('env-project'); + (text as Mock).mockResolvedValueOnce('env-region'); + + await createAgent(getFreshOptions()); + + expect(text).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Enter the Google Cloud Project ID', + initialValue: 'env-project', + }), + ); + expect(text).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Enter the Google Cloud Region', + initialValue: 'env-region', + }), + ); + expect(execSync).not.toHaveBeenCalled(); + }); + it('should exit without writing files if project prompt is cancelled', async () => { // Mirror clack's contract: only the raw cancel symbol counts as a cancel. (isCancel as unknown as Mock).mockImplementation( @@ -238,6 +284,36 @@ describe('createAgent', () => { expect(saveToFile).not.toHaveBeenCalled(); }); + + // Last in this block: the throwing execSync implementation survives + // vi.clearAllMocks(), which resets call history but not implementations. + it('should fall back to an empty initial value when gcloud is unavailable', async () => { + (select as Mock).mockResolvedValueOnce('gemini-2.5-flash'); + (select as Mock).mockResolvedValueOnce('ts'); + (select as Mock).mockResolvedValueOnce('vertex'); // Backend + + (execSync as Mock).mockImplementation(() => { + throw new Error('gcloud not found'); + }); + + (text as Mock).mockResolvedValueOnce(''); + (text as Mock).mockResolvedValueOnce(''); + + await createAgent(getFreshOptions()); + + expect(text).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Enter the Google Cloud Project ID', + initialValue: '', + }), + ); + expect(text).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Enter the Google Cloud Region', + initialValue: '', + }), + ); + }); }); describe('Folder Handling', () => { From be2aa23f18ffbd8be24a1ff65746025c55cb1dd7 Mon Sep 17 00:00:00 2001 From: Amaad Martin Date: Mon, 3 Aug 2026 00:18:33 -0700 Subject: [PATCH 2/2] Drop dead execSync setup and the incorrect mock-ordering comment The env-precedence test also asserts execSync is never called, so the execSync implementation it installed could never run. Remove it. Remove the comment claiming the throwing execSync implementation survives into later tests. Measured on the pinned vitest 3.2.6: clearAllMocks() alone does keep implementations, but the restoreAllMocks() already in this suite's afterEach clears them, so the ordering of that test is not load-bearing. --- dev/test/cli/cli_create_test.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/dev/test/cli/cli_create_test.ts b/dev/test/cli/cli_create_test.ts index 0149ead62..7ddd57b78 100644 --- a/dev/test/cli/cli_create_test.ts +++ b/dev/test/cli/cli_create_test.ts @@ -242,12 +242,6 @@ describe('createAgent', () => { (select as Mock).mockResolvedValueOnce('ts'); (select as Mock).mockResolvedValueOnce('vertex'); // Backend - (execSync as Mock).mockImplementation((cmd: string) => { - if (cmd.includes('project')) return 'gcloud-project\n'; - if (cmd.includes('region')) return 'gcloud-region\n'; - return ''; - }); - (text as Mock).mockResolvedValueOnce('env-project'); (text as Mock).mockResolvedValueOnce('env-region'); @@ -285,8 +279,6 @@ describe('createAgent', () => { expect(saveToFile).not.toHaveBeenCalled(); }); - // Last in this block: the throwing execSync implementation survives - // vi.clearAllMocks(), which resets call history but not implementations. it('should fall back to an empty initial value when gcloud is unavailable', async () => { (select as Mock).mockResolvedValueOnce('gemini-2.5-flash'); (select as Mock).mockResolvedValueOnce('ts');