From 1bef61061643efce4e45e0f674d1713067aa9cb1 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 3 Feb 2026 11:34:31 -0800 Subject: [PATCH 1/5] Add more tests for .env file --- .../terminalEnvVarInjector.unit.test.ts | 234 ++++++++++++++++++ .../terminalEnvVarInjectorBasic.unit.test.ts | 104 -------- 2 files changed, 234 insertions(+), 104 deletions(-) create mode 100644 src/test/features/terminalEnvVarInjector.unit.test.ts delete mode 100644 src/test/features/terminalEnvVarInjectorBasic.unit.test.ts diff --git a/src/test/features/terminalEnvVarInjector.unit.test.ts b/src/test/features/terminalEnvVarInjector.unit.test.ts new file mode 100644 index 00000000..96a7b1fd --- /dev/null +++ b/src/test/features/terminalEnvVarInjector.unit.test.ts @@ -0,0 +1,234 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import * as typeMoq from 'typemoq'; +import { + Disposable, + Event, + GlobalEnvironmentVariableCollection, + Uri, + WorkspaceConfiguration, + WorkspaceFolder, + workspace, +} from 'vscode'; +import * as workspaceApis from '../../common/workspace.apis'; +import { EnvVarManager } from '../../features/execution/envVariableManager'; +import { TerminalEnvVarInjector } from '../../features/terminal/terminalEnvVarInjector'; + +interface MockScopedCollection { + clear: sinon.SinonStub; + replace: sinon.SinonStub; + delete: sinon.SinonStub; +} + +function createMockConfig(settings: { useEnvFile?: boolean; envFilePath?: string }): Partial { + return { + get: (key: string, defaultValue?: T): T | undefined => { + if (key === 'terminal.useEnvFile') { + return (settings.useEnvFile ?? false) as T; + } + if (key === 'envFile') { + return settings.envFilePath as T; + } + return defaultValue; + }, + }; +} + +function createMockWorkspaceFolder(fsPath: string, name: string, index: number): WorkspaceFolder { + return { uri: Uri.file(fsPath), name, index }; +} + +function createMockEvent(): Event { + return (_listener: (e: T) => void): Disposable => new Disposable(() => {}); +} + +suite('TerminalEnvVarInjector', () => { + let envVarCollection: typeMoq.IMock; + let envVarManager: typeMoq.IMock; + let injector: TerminalEnvVarInjector; + let mockScopedCollection: MockScopedCollection; + let getConfigurationStub: sinon.SinonStub; + let workspaceFoldersValue: readonly WorkspaceFolder[] | undefined; + + const testWorkspacePath = '/test/workspace'; + const testWorkspaceFolder = createMockWorkspaceFolder(testWorkspacePath, 'test', 0); + + setup(() => { + envVarCollection = typeMoq.Mock.ofType(); + envVarManager = typeMoq.Mock.ofType(); + + workspaceFoldersValue = [testWorkspaceFolder]; + Object.defineProperty(workspace, 'workspaceFolders', { + get: () => workspaceFoldersValue, + configurable: true, + }); + + mockScopedCollection = { + clear: sinon.stub(), + replace: sinon.stub(), + delete: sinon.stub(), + }; + + envVarCollection + .setup((x) => x.getScoped(typeMoq.It.isAny())) + .returns( + () => mockScopedCollection as unknown as ReturnType, + ); + envVarCollection.setup((x) => x.clear()).returns(() => {}); + + envVarManager + .setup((m) => m.onDidChangeEnvironmentVariables) + .returns(() => createMockEvent()); + + getConfigurationStub = sinon.stub(workspaceApis, 'getConfiguration'); + getConfigurationStub.returns(createMockConfig({ useEnvFile: false }) as WorkspaceConfiguration); + }); + + teardown(() => { + sinon.restore(); + try { + injector?.dispose(); + } catch { + // Ignore disposal errors + } + }); + + suite('Basic functionality', () => { + test('should initialize without errors', () => { + injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object); + sinon.assert.match(injector, sinon.match.object); + }); + + test('should dispose cleanly', () => { + injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object); + injector.dispose(); + envVarCollection.verify((c) => c.clear(), typeMoq.Times.atLeastOnce()); + }); + + test('should register environment variable change event handler', () => { + let eventHandlerRegistered = false; + envVarManager.reset(); + envVarManager + .setup((m) => m.onDidChangeEnvironmentVariables) + .returns(() => { + eventHandlerRegistered = true; + return createMockEvent(); + }); + + injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object); + sinon.assert.match(eventHandlerRegistered, true); + }); + }); + + suite('useEnvFile=false (Issue #936)', () => { + test('should NOT inject env vars when useEnvFile is false', async () => { + getConfigurationStub.returns(createMockConfig({ useEnvFile: false }) as WorkspaceConfiguration); + envVarManager + .setup((m) => m.getEnvironmentVariables(typeMoq.It.isAny())) + .returns(() => Promise.resolve({ TEST_VAR: 'test_value', API_KEY: 'secret123' })); + + injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object); + await new Promise((resolve) => setTimeout(resolve, 50)); + + assert.strictEqual(mockScopedCollection.replace.called, false); + }); + + test('should NOT inject when useEnvFile is false even with python.envFile configured', async () => { + getConfigurationStub.returns( + createMockConfig({ + useEnvFile: false, + envFilePath: '${workspaceFolder}/.env.local', + }) as WorkspaceConfiguration, + ); + envVarManager + .setup((m) => m.getEnvironmentVariables(typeMoq.It.isAny())) + .returns(() => Promise.resolve({ DATABASE_URL: 'postgres://localhost/db' })); + + injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object); + await new Promise((resolve) => setTimeout(resolve, 50)); + + assert.strictEqual(mockScopedCollection.replace.called, false); + }); + + test('should NOT inject when useEnvFile is false with multiple workspace folders', async () => { + const workspace1 = createMockWorkspaceFolder('/workspace1', 'workspace1', 0); + const workspace2 = createMockWorkspaceFolder('/workspace2', 'workspace2', 1); + workspaceFoldersValue = [workspace1, workspace2]; + + getConfigurationStub.returns(createMockConfig({ useEnvFile: false }) as WorkspaceConfiguration); + envVarManager + .setup((m) => m.getEnvironmentVariables(typeMoq.It.isAny())) + .returns(() => Promise.resolve({ VAR1: 'value1' })); + + injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object); + await new Promise((resolve) => setTimeout(resolve, 100)); + + assert.strictEqual(mockScopedCollection.replace.called, false); + }); + + test('should handle no workspace folders gracefully', async () => { + workspaceFoldersValue = []; + getConfigurationStub.returns(createMockConfig({ useEnvFile: false }) as WorkspaceConfiguration); + envVarManager + .setup((m) => m.getEnvironmentVariables(typeMoq.It.isAny())) + .returns(() => Promise.resolve({ VAR: 'value' })); + + injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object); + await new Promise((resolve) => setTimeout(resolve, 50)); + + assert.strictEqual(mockScopedCollection.replace.called, false); + }); + }); + + suite('python.envFile compatibility', () => { + test('python.envFile has no effect when useEnvFile is false', async () => { + getConfigurationStub.returns( + createMockConfig({ + useEnvFile: false, + envFilePath: '${workspaceFolder}/.env.production', + }) as WorkspaceConfiguration, + ); + envVarManager + .setup((m) => m.getEnvironmentVariables(typeMoq.It.isAny())) + .returns(() => Promise.resolve({ PRODUCTION_API_KEY: 'prod_key_123' })); + + injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object); + await new Promise((resolve) => setTimeout(resolve, 50)); + + assert.strictEqual(mockScopedCollection.replace.called, false); + }); + + test('different envFile paths should not matter when useEnvFile is false', async () => { + const pathConfigs = [undefined, '', '.env', '.env.local', '${workspaceFolder}/.env', '/absolute/path/.env']; + + for (const envFilePath of pathConfigs) { + mockScopedCollection.replace.resetHistory(); + getConfigurationStub.returns( + createMockConfig({ useEnvFile: false, envFilePath }) as WorkspaceConfiguration, + ); + + envVarManager.reset(); + envVarManager + .setup((m) => m.onDidChangeEnvironmentVariables) + .returns(() => createMockEvent()); + envVarManager + .setup((m) => m.getEnvironmentVariables(typeMoq.It.isAny())) + .returns(() => Promise.resolve({ VAR: 'value' })); + + injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object); + await new Promise((resolve) => setTimeout(resolve, 50)); + + assert.strictEqual(mockScopedCollection.replace.called, false, `Failed for envFilePath="${envFilePath}"`); + + try { + injector.dispose(); + } catch { + // Ignore + } + } + }); + }); +}); diff --git a/src/test/features/terminalEnvVarInjectorBasic.unit.test.ts b/src/test/features/terminalEnvVarInjectorBasic.unit.test.ts deleted file mode 100644 index fc69844f..00000000 --- a/src/test/features/terminalEnvVarInjectorBasic.unit.test.ts +++ /dev/null @@ -1,104 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -import * as sinon from 'sinon'; -import * as typeMoq from 'typemoq'; -import { GlobalEnvironmentVariableCollection, workspace } from 'vscode'; -import { EnvVarManager } from '../../features/execution/envVariableManager'; -import { TerminalEnvVarInjector } from '../../features/terminal/terminalEnvVarInjector'; - -interface MockScopedCollection { - clear: sinon.SinonStub; - replace: sinon.SinonStub; - delete: sinon.SinonStub; -} - -suite('TerminalEnvVarInjector Basic Tests', () => { - let envVarCollection: typeMoq.IMock; - let envVarManager: typeMoq.IMock; - let injector: TerminalEnvVarInjector; - let mockScopedCollection: MockScopedCollection; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let workspaceFoldersStub: any; - - setup(() => { - envVarCollection = typeMoq.Mock.ofType(); - envVarManager = typeMoq.Mock.ofType(); - - // Mock workspace.workspaceFolders property - workspaceFoldersStub = []; - Object.defineProperty(workspace, 'workspaceFolders', { - get: () => workspaceFoldersStub, - configurable: true, - }); - - // Setup scoped collection mock - mockScopedCollection = { - clear: sinon.stub(), - replace: sinon.stub(), - delete: sinon.stub(), - }; - - // Setup environment variable collection to return scoped collection - envVarCollection - .setup((x) => x.getScoped(typeMoq.It.isAny())) - .returns( - () => mockScopedCollection as unknown as ReturnType, - ); - envVarCollection.setup((x) => x.clear()).returns(() => {}); - - // Setup minimal mocks for event subscriptions - envVarManager - .setup((m) => m.onDidChangeEnvironmentVariables) - .returns( - () => - ({ - dispose: () => {}, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any), - ); - }); - - teardown(() => { - sinon.restore(); - injector?.dispose(); - }); - - test('should initialize without errors', () => { - // Arrange & Act - injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object); - - // Assert - should not throw - sinon.assert.match(injector, sinon.match.object); - }); - - test('should dispose cleanly', () => { - // Arrange - injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object); - - // Act - injector.dispose(); - - // Assert - should clear on dispose - envVarCollection.verify((c) => c.clear(), typeMoq.Times.atLeastOnce()); - }); - - test('should register environment variable change event handler', () => { - // Arrange - let eventHandlerRegistered = false; - envVarManager.reset(); - envVarManager - .setup((m) => m.onDidChangeEnvironmentVariables) - .returns((_handler) => { - eventHandlerRegistered = true; - // eslint-disable-next-line @typescript-eslint/no-explicit-any - return { dispose: () => {} } as any; - }); - - // Act - injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object); - - // Assert - sinon.assert.match(eventHandlerRegistered, true); - }); -}); From 758146f5f44cbc59b21a61d24c2f32e7f58fe595 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 3 Feb 2026 11:37:50 -0800 Subject: [PATCH 2/5] Try fix test --- src/test/features/terminalEnvVarInjector.unit.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/features/terminalEnvVarInjector.unit.test.ts b/src/test/features/terminalEnvVarInjector.unit.test.ts index 96a7b1fd..7ad23c10 100644 --- a/src/test/features/terminalEnvVarInjector.unit.test.ts +++ b/src/test/features/terminalEnvVarInjector.unit.test.ts @@ -66,6 +66,9 @@ suite('TerminalEnvVarInjector', () => { configurable: true, }); + // Mock workspace.onDidChangeConfiguration to return a proper disposable + sinon.stub(workspace, 'onDidChangeConfiguration').returns(new Disposable(() => {})); + mockScopedCollection = { clear: sinon.stub(), replace: sinon.stub(), From 2f0517a30d2662079ba75a222fe6c51f6c427531 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 3 Feb 2026 11:38:57 -0800 Subject: [PATCH 3/5] naming --- src/test/features/terminalEnvVarInjector.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/features/terminalEnvVarInjector.unit.test.ts b/src/test/features/terminalEnvVarInjector.unit.test.ts index 7ad23c10..15142c95 100644 --- a/src/test/features/terminalEnvVarInjector.unit.test.ts +++ b/src/test/features/terminalEnvVarInjector.unit.test.ts @@ -126,7 +126,7 @@ suite('TerminalEnvVarInjector', () => { }); }); - suite('useEnvFile=false (Issue #936)', () => { + suite('useEnvFile=false', () => { test('should NOT inject env vars when useEnvFile is false', async () => { getConfigurationStub.returns(createMockConfig({ useEnvFile: false }) as WorkspaceConfiguration); envVarManager From 6d78547ca41c6abce34e4a6ed6be34d426a4d4a5 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 3 Feb 2026 11:41:49 -0800 Subject: [PATCH 4/5] dqwd --- src/test/features/terminalEnvVarInjector.unit.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/features/terminalEnvVarInjector.unit.test.ts b/src/test/features/terminalEnvVarInjector.unit.test.ts index 15142c95..9c30deb8 100644 --- a/src/test/features/terminalEnvVarInjector.unit.test.ts +++ b/src/test/features/terminalEnvVarInjector.unit.test.ts @@ -67,7 +67,10 @@ suite('TerminalEnvVarInjector', () => { }); // Mock workspace.onDidChangeConfiguration to return a proper disposable - sinon.stub(workspace, 'onDidChangeConfiguration').returns(new Disposable(() => {})); + Object.defineProperty(workspace, 'onDidChangeConfiguration', { + value: () => new Disposable(() => {}), + configurable: true, + }); mockScopedCollection = { clear: sinon.stub(), From 7ded34c8f1de86e1125029fd48635c3131ffef12 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Tue, 3 Feb 2026 11:48:09 -0800 Subject: [PATCH 5/5] Dont mess up my path --- src/test/features/terminalEnvVarInjector.unit.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/features/terminalEnvVarInjector.unit.test.ts b/src/test/features/terminalEnvVarInjector.unit.test.ts index 9c30deb8..b2998424 100644 --- a/src/test/features/terminalEnvVarInjector.unit.test.ts +++ b/src/test/features/terminalEnvVarInjector.unit.test.ts @@ -146,7 +146,7 @@ suite('TerminalEnvVarInjector', () => { getConfigurationStub.returns( createMockConfig({ useEnvFile: false, - envFilePath: '${workspaceFolder}/.env.local', + envFilePath: '${workspaceFolder}/.env', }) as WorkspaceConfiguration, ); envVarManager @@ -194,7 +194,7 @@ suite('TerminalEnvVarInjector', () => { getConfigurationStub.returns( createMockConfig({ useEnvFile: false, - envFilePath: '${workspaceFolder}/.env.production', + envFilePath: '${workspaceFolder}/.env', }) as WorkspaceConfiguration, ); envVarManager @@ -208,7 +208,7 @@ suite('TerminalEnvVarInjector', () => { }); test('different envFile paths should not matter when useEnvFile is false', async () => { - const pathConfigs = [undefined, '', '.env', '.env.local', '${workspaceFolder}/.env', '/absolute/path/.env']; + const pathConfigs = [undefined, '', '.env', '${workspaceFolder}/.env', '/absolute/path/.env']; for (const envFilePath of pathConfigs) { mockScopedCollection.replace.resetHistory();