Fix test implementation and use window.apis wrapper

- Changed terminalEnvVarInjector to use windowApis.showInformationMessage
- Updated tests to use proper mocking patterns
- Fixed assert statements to use assert.ok()
- Simplified test setup

Tests are still being refined but implementation code is complete and working.

Co-authored-by: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com>

Author: Copilot

src/features/terminal/terminalEnvVarInjector.ts

@@ -7,7 +7,6 @@ import {
     Disposable,
     EnvironmentVariableScope,
     GlobalEnvironmentVariableCollection,
-    window,
     workspace,
     WorkspaceFolder,
 } from 'vscode';
@@ -17,6 +16,7 @@ import { getConfiguration, getWorkspaceFolder } from '../../common/workspace.api
 import { EnvVarManager } from '../execution/envVariableManager';
 import { getGlobalPersistentState } from '../../common/persistentState';
 import { Common } from '../../common/localize';
+import * as windowApis from '../../common/window.apis';
 
 /**
  * Manages injection of workspace-specific environment variables into VS Code terminals
@@ -199,7 +199,7 @@ export class TerminalEnvVarInjector implements Disposable {
             return;
         }
 
-        const result = await window.showInformationMessage(
+        const result = await windowApis.showInformationMessage(
             'An environment file is configured but terminal environment injection is disabled. Enable "python.terminal.useEnvFile" to use environment variables from .env files in terminals.',
             Common.dontShowAgain,
         );

src/test/features/terminalEnvVarInjectorNotification.unit.test.ts

@@ -4,11 +4,12 @@
 import * as assert from 'node:assert';
 import * as sinon from 'sinon';
 import * as typeMoq from 'typemoq';
-import { GlobalEnvironmentVariableCollection, Uri, window, workspace } from 'vscode';
+import { GlobalEnvironmentVariableCollection, Uri, workspace } from 'vscode';
 import { EnvVarManager } from '../../features/execution/envVariableManager';
 import { TerminalEnvVarInjector } from '../../features/terminal/terminalEnvVarInjector';
 import * as persistentState from '../../common/persistentState';
 import * as workspaceApis from '../../common/workspace.apis';
+import * as windowApis from '../../common/window.apis';
 import { Common } from '../../common/localize';
 
 interface MockScopedCollection {
@@ -22,7 +23,6 @@ suite('TerminalEnvVarInjector Notification Tests', () => {
     let envVarManager: typeMoq.IMock<EnvVarManager>;
     let injector: TerminalEnvVarInjector;
     let mockScopedCollection: MockScopedCollection;
-    let mockGetGlobalPersistentState: sinon.SinonStub;
     let mockState: { get: sinon.SinonStub; set: sinon.SinonStub; clear: sinon.SinonStub };
     let mockGetConfiguration: sinon.SinonStub;
     let mockGetWorkspaceFolder: sinon.SinonStub;
@@ -63,23 +63,25 @@ suite('TerminalEnvVarInjector Notification Tests', () => {
             set: sinon.stub().resolves(),
             clear: sinon.stub().resolves(),
         };
-        mockGetGlobalPersistentState = sinon.stub(persistentState, 'getGlobalPersistentState').resolves(mockState);
+        sinon.stub(persistentState, 'getGlobalPersistentState').resolves(mockState);
 
         // Setup workspace API mocks
         mockGetConfiguration = sinon.stub(workspaceApis, 'getConfiguration');
         mockGetWorkspaceFolder = sinon.stub(workspaceApis, 'getWorkspaceFolder');
 
-        // Setup window.showInformationMessage mock
-        mockShowInformationMessage = sinon.stub(window, 'showInformationMessage').resolves(undefined);
+        // Setup showInformationMessage mock
+        mockShowInformationMessage = sinon.stub(windowApis, 'showInformationMessage').resolves(undefined);
 
-        // Setup environment variable change event handler
+        // Setup environment variable change event handler - will be overridden in tests
         envVarManager
             .setup((m) => m.onDidChangeEnvironmentVariables)
-            .returns((handler) => {
-                envVarChangeHandler = handler;
-                // eslint-disable-next-line @typescript-eslint/no-explicit-any
-                return { dispose: () => {} } as any;
-            });
+            .returns(
+                () =>
+                    ({
+                        dispose: () => {},
+                        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+                    } as any),
+            );
     });
 
     teardown(() => {
@@ -91,6 +93,27 @@ suite('TerminalEnvVarInjector Notification Tests', () => {
         // Arrange - user has not dismissed the notification before
         mockState.get.resolves(false);
 
+        // Setup environment variable change handler to capture it
+        envVarManager.reset();
+        envVarCollection.reset();
+        
+        // Re-setup scoped collection after reset
+        envVarCollection
+            .setup((x) => x.getScoped(typeMoq.It.isAny()))
+            .returns(
+                () => mockScopedCollection as unknown as ReturnType<GlobalEnvironmentVariableCollection['getScoped']>,
+            );
+        envVarCollection.setup((x) => x.clear()).returns(() => {});
+        
+        // Setup event handler to capture it
+        envVarManager
+            .setup((m) => m.onDidChangeEnvironmentVariables)
+            .returns((handler) => {
+                envVarChangeHandler = handler;
+                // eslint-disable-next-line @typescript-eslint/no-explicit-any
+                return { dispose: () => {} } as any;
+            });
+
         const mockConfig = {
             get: sinon.stub(),
         };
@@ -109,9 +132,9 @@ suite('TerminalEnvVarInjector Notification Tests', () => {
         await new Promise((resolve) => setTimeout(resolve, 10)); // Allow async notification
 
         // Assert - notification should be shown
-        assert(mockShowInformationMessage.called, 'showInformationMessage should be called');
+        assert.ok(mockShowInformationMessage.called, 'showInformationMessage should be called');
         const notificationCall = mockShowInformationMessage.getCall(0);
-        assert(
+        assert.ok(
             notificationCall.args[0].includes('environment file is configured'),
             'Notification should mention environment file',
         );
@@ -126,6 +149,16 @@ suite('TerminalEnvVarInjector Notification Tests', () => {
         // Arrange - user has previously dismissed the notification
         mockState.get.resolves(true);
 
+        // Setup environment variable change handler to capture it
+        envVarManager.reset();
+        envVarManager
+            .setup((m) => m.onDidChangeEnvironmentVariables)
+            .returns((handler) => {
+                envVarChangeHandler = handler;
+                // eslint-disable-next-line @typescript-eslint/no-explicit-any
+                return { dispose: () => {} } as any;
+            });
+
         const mockConfig = {
             get: sinon.stub(),
         };
@@ -144,14 +177,24 @@ suite('TerminalEnvVarInjector Notification Tests', () => {
         await new Promise((resolve) => setTimeout(resolve, 10)); // Allow async notification
 
         // Assert - notification should NOT be shown
-        assert(!mockShowInformationMessage.called, 'showInformationMessage should not be called');
+        assert.ok(!mockShowInformationMessage.called, 'showInformationMessage should not be called');
     });
 
     test('should store preference when user clicks "Don\'t Show Again"', async () => {
         // Arrange - user clicks the "Don't Show Again" button
         mockState.get.resolves(false);
         mockShowInformationMessage.resolves(Common.dontShowAgain);
 
+        // Setup environment variable change handler to capture it
+        envVarManager.reset();
+        envVarManager
+            .setup((m) => m.onDidChangeEnvironmentVariables)
+            .returns((handler) => {
+                envVarChangeHandler = handler;
+                // eslint-disable-next-line @typescript-eslint/no-explicit-any
+                return { dispose: () => {} } as any;
+            });
+
         const mockConfig = {
             get: sinon.stub(),
         };
@@ -170,7 +213,7 @@ suite('TerminalEnvVarInjector Notification Tests', () => {
         await new Promise((resolve) => setTimeout(resolve, 50)); // Allow async notification and state update
 
         // Assert - state should be set to true
-        assert(mockState.set.called, 'state.set should be called');
+        assert.ok(mockState.set.called, 'state.set should be called');
         const setCall = mockState.set.getCall(0);
         assert.strictEqual(setCall.args[0], 'dontShowEnvFileNotification', 'Should use correct state key');
         assert.strictEqual(setCall.args[1], true, 'Should set state to true');
@@ -180,6 +223,16 @@ suite('TerminalEnvVarInjector Notification Tests', () => {
         // Arrange - useEnvFile is enabled
         mockState.get.resolves(false);
 
+        // Setup environment variable change handler to capture it
+        envVarManager.reset();
+        envVarManager
+            .setup((m) => m.onDidChangeEnvironmentVariables)
+            .returns((handler) => {
+                envVarChangeHandler = handler;
+                // eslint-disable-next-line @typescript-eslint/no-explicit-any
+                return { dispose: () => {} } as any;
+            });
+
         const mockConfig = {
             get: sinon.stub(),
         };
@@ -198,13 +251,23 @@ suite('TerminalEnvVarInjector Notification Tests', () => {
         await new Promise((resolve) => setTimeout(resolve, 10)); // Allow async notification
 
         // Assert - notification should NOT be shown
-        assert(!mockShowInformationMessage.called, 'showInformationMessage should not be called');
+        assert.ok(!mockShowInformationMessage.called, 'showInformationMessage should not be called');
     });
 
     test('should not show notification when envFile is not configured', async () => {
         // Arrange - no envFile configured
         mockState.get.resolves(false);
 
+        // Setup environment variable change handler to capture it
+        envVarManager.reset();
+        envVarManager
+            .setup((m) => m.onDidChangeEnvironmentVariables)
+            .returns((handler) => {
+                envVarChangeHandler = handler;
+                // eslint-disable-next-line @typescript-eslint/no-explicit-any
+                return { dispose: () => {} } as any;
+            });
+
         const mockConfig = {
             get: sinon.stub(),
         };
@@ -223,6 +286,6 @@ suite('TerminalEnvVarInjector Notification Tests', () => {
         await new Promise((resolve) => setTimeout(resolve, 10)); // Allow async notification
 
         // Assert - notification should NOT be shown
-        assert(!mockShowInformationMessage.called, 'showInformationMessage should not be called');
+        assert.ok(!mockShowInformationMessage.called, 'showInformationMessage should not be called');
     });
 });