Move .env file and environment variable parser into the extension (#6773

) * Move getEnvironmentVariables out of debug adapter * Linter fixes * Simplify how debug env helper is used * Add new item * Fix Tests * Apply commit suggestion Co-Authored-By: Don Jayamanne <[email protected]>
microsoft · Jul 31, 2019 · e095693 · e095693
1 parent 6f5c3c9
commit e095693
Show file tree

Hide file tree

Showing 8 changed files with 67 additions and 53 deletions.
diff --git a/news/3 Code Health/6770.md b/news/3 Code Health/6770.md
@@ -0,0 +1 @@
+Move .env file handling into the extension. This is in preparation to switch to the out-of-proc debug adapter from ptvsd.
diff --git a/src/client/debugger/debugAdapter/DebugClients/LocalDebugClient.ts b/src/client/debugger/debugAdapter/DebugClients/LocalDebugClient.ts
@@ -3,19 +3,14 @@ import * as path from 'path';
 import { DebugSession, OutputEvent } from 'vscode-debugadapter';
 import { DebugProtocol } from 'vscode-debugprotocol';
 import { open } from '../../../common/open';
-import { IS_WINDOWS } from '../../../common/platform/constants';
-import { PathUtils } from '../../../common/platform/pathUtils';
-import { CurrentProcess } from '../../../common/process/currentProcess';
 import { noop } from '../../../common/utils/misc';
-import { EnvironmentVariablesService } from '../../../common/variables/environment';
 import { IServiceContainer } from '../../../ioc/types';
 import { LaunchRequestArguments } from '../../types';
 import { IDebugServer } from '../Common/Contracts';
 import { BaseDebugServer } from '../DebugServers/BaseDebugServer';
 import { LocalDebugServerV2 } from '../DebugServers/LocalDebugServerV2';
 import { ILocalDebugLauncherScriptProvider } from '../types';
 import { DebugClient, DebugType } from './DebugClient';
-import { DebugClientHelper } from './helper';
 
 enum DebugServerStatus {
     Unknown = 1,
@@ -67,11 +62,6 @@ export class LocalDebugClient extends DebugClient<LaunchRequestArguments> {
     }
     // tslint:disable-next-line:max-func-body-length member-ordering no-any
     public async LaunchApplicationToDebug(dbgServer: IDebugServer): Promise<any> {
-        const pathUtils = new PathUtils(IS_WINDOWS);
-        const currentProcess = new CurrentProcess();
-        const environmentVariablesService = new EnvironmentVariablesService(pathUtils);
-        const helper = new DebugClientHelper(environmentVariablesService, pathUtils, currentProcess);
-        const environmentVariables = await helper.getEnvironmentVariables(this.args);
         // tslint:disable-next-line:max-func-body-length cyclomatic-complexity no-any
         return new Promise<any>((resolve, reject) => {
             const fileDir = this.args && this.args.program ? path.dirname(this.args.program) : '';
@@ -84,15 +74,16 @@ export class LocalDebugClient extends DebugClient<LaunchRequestArguments> {
                 pythonPath = this.args.pythonPath;
             }
             const args = this.buildLaunchArguments(processCwd, dbgServer.port);
+            const envVars = this.args.env ? { ...this.args.env } : {};
             switch (this.args.console) {
                 case 'externalTerminal':
                 case 'integratedTerminal': {
                     const isSudo = Array.isArray(this.args.debugOptions) && this.args.debugOptions.some(opt => opt === 'Sudo');
-                    this.launchExternalTerminal(isSudo, processCwd, pythonPath, args, environmentVariables).then(resolve).catch(reject);
+                    this.launchExternalTerminal(isSudo, processCwd, pythonPath, args, envVars).then(resolve).catch(reject);
                     break;
                 }
                 default: {
-                    this.pyProc = spawn(pythonPath, args, { cwd: processCwd, env: environmentVariables });
+                    this.pyProc = spawn(pythonPath, args, { cwd: processCwd, env: envVars });
                     this.handleProcessOutput(this.pyProc!, reject);
 
                     // Here we wait for the application to connect to the socket server.

diff --git a/...ugger/debugAdapter/DebugClients/helper.ts → ...tension/configuration/resolvers/helper.ts b/...ugger/debugAdapter/DebugClients/helper.ts → ...tension/configuration/resolvers/helper.ts
@@ -1,10 +1,25 @@
-import { ICurrentProcess, IPathUtils } from '../../../common/types';
-import { EnvironmentVariables, IEnvironmentVariablesService } from '../../../common/variables/types';
-import { LaunchRequestArguments } from '../../types';
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
 
-export class DebugClientHelper {
-    constructor(private envParser: IEnvironmentVariablesService, private pathUtils: IPathUtils,
-        private process: ICurrentProcess) { }
+'use strict';
+
+import { inject, injectable } from 'inversify';
+import { ICurrentProcess, IPathUtils } from '../../../../common/types';
+import { EnvironmentVariables, IEnvironmentVariablesService } from '../../../../common/variables/types';
+import { LaunchRequestArguments } from '../../../types';
+
+export const IDebugEnvironmentVariablesService = Symbol('IDebugEnvironmentVariablesService');
+export interface IDebugEnvironmentVariablesService {
+    getEnvironmentVariables(args: LaunchRequestArguments): Promise<EnvironmentVariables>;
+}
+
+@injectable()
+export class DebugEnvironmentVariablesHelper implements IDebugEnvironmentVariablesService {
+    constructor(
+        @inject(IEnvironmentVariablesService) private envParser: IEnvironmentVariablesService,
+        @inject(IPathUtils) private pathUtils: IPathUtils,
+        @inject(ICurrentProcess) private process: ICurrentProcess
+    ) { }
     public async getEnvironmentVariables(args: LaunchRequestArguments): Promise<EnvironmentVariables> {
         const pathVariableName = this.pathUtils.getPathVariableName();
 

diff --git a/src/client/debugger/extension/configuration/resolvers/launch.ts b/src/client/debugger/extension/configuration/resolvers/launch.ts
@@ -13,6 +13,7 @@ import { IConfigurationService } from '../../../../common/types';
 import { DebuggerTypeName } from '../../../constants';
 import { DebugOptions, LaunchRequestArguments } from '../../../types';
 import { BaseConfigurationResolver } from './base';
+import { IDebugEnvironmentVariablesService } from './helper';
 
 @injectable()
 export class LaunchConfigurationResolver extends BaseConfigurationResolver<LaunchRequestArguments> {
@@ -21,7 +22,8 @@ export class LaunchConfigurationResolver extends BaseConfigurationResolver<Launc
         @inject(IDocumentManager) documentManager: IDocumentManager,
         @inject(IDiagnosticsService) @named(InvalidPythonPathInDebuggerServiceId) private readonly invalidPythonPathInDebuggerService: IInvalidPythonPathInDebuggerService,
         @inject(IPlatformService) private readonly platformService: IPlatformService,
-        @inject(IConfigurationService) configurationService: IConfigurationService
+        @inject(IConfigurationService) configurationService: IConfigurationService,
+        @inject(IDebugEnvironmentVariablesService) private readonly debugEnvHelper: IDebugEnvironmentVariablesService
     ) {
         super(workspaceService, documentManager, configurationService);
     }
@@ -63,6 +65,10 @@ export class LaunchConfigurationResolver extends BaseConfigurationResolver<Launc
             const settings = this.configurationService.getSettings(workspaceFolder);
             debugConfiguration.envFile = settings.envFile;
         }
+        // Extract environment variables from .env file in the vscode context and
+        // set the "env" debug configuration argument. This expansion should be
+        // done here before handing of the environment settings to the debug adapter
+        debugConfiguration.env = await this.debugEnvHelper.getEnvironmentVariables(debugConfiguration);
         if (typeof debugConfiguration.stopOnEntry !== 'boolean') {
             debugConfiguration.stopOnEntry = false;
         }

diff --git a/src/client/debugger/extension/serviceRegistry.ts b/src/client/debugger/extension/serviceRegistry.ts
@@ -18,6 +18,7 @@ import { DebugConfigurationProviderFactory } from './configuration/providers/pro
 import { PyramidLaunchDebugConfigurationProvider } from './configuration/providers/pyramidLaunch';
 import { RemoteAttachDebugConfigurationProvider } from './configuration/providers/remoteAttach';
 import { AttachConfigurationResolver } from './configuration/resolvers/attach';
+import { DebugEnvironmentVariablesHelper, IDebugEnvironmentVariablesService } from './configuration/resolvers/helper';
 import { LaunchConfigurationResolver } from './configuration/resolvers/launch';
 import { IDebugConfigurationProviderFactory, IDebugConfigurationResolver } from './configuration/types';
 import { ChildProcessAttachEventHandler } from './hooks/childProcessAttachHandler';
@@ -41,4 +42,5 @@ export function registerTypes(serviceManager: IServiceManager) {
     serviceManager.addSingleton<IDebugConfigurationProvider>(IDebugConfigurationProvider, RemoteAttachDebugConfigurationProvider, DebugConfigurationType.remoteAttach);
     serviceManager.addSingleton<IDebugConfigurationProvider>(IDebugConfigurationProvider, ModuleLaunchDebugConfigurationProvider, DebugConfigurationType.launchModule);
     serviceManager.addSingleton<IDebugConfigurationProvider>(IDebugConfigurationProvider, PyramidLaunchDebugConfigurationProvider, DebugConfigurationType.launchPyramid);
+    serviceManager.addSingleton<IDebugEnvironmentVariablesService>(IDebugEnvironmentVariablesService, DebugEnvironmentVariablesHelper);
 }
diff --git a/src/test/debugger/envVars.test.ts b/src/test/debugger/envVars.test.ts
@@ -9,7 +9,7 @@ import * as path from 'path';
 import * as shortid from 'shortid';
 import { ICurrentProcess, IPathUtils } from '../../client/common/types';
 import { IEnvironmentVariablesService } from '../../client/common/variables/types';
-import { DebugClientHelper } from '../../client/debugger/debugAdapter/DebugClients/helper';
+import { DebugEnvironmentVariablesHelper, IDebugEnvironmentVariablesService } from '../../client/debugger/extension/configuration/resolvers/helper';
 import { ConsoleType, LaunchRequestArguments } from '../../client/debugger/types';
 import { isOs, OSType } from '../common';
 import { closeActiveWindows, initialize, initializeTest, IS_MULTI_ROOT_TEST, TEST_DEBUGGER } from '../initialize';
@@ -19,7 +19,7 @@ use(chaiAsPromised);
 
 suite('Resolving Environment Variables when Debugging', () => {
     let ioc: UnitTestIocContainer;
-    let helper: DebugClientHelper;
+    let debugEnvParser: IDebugEnvironmentVariablesService;
     let pathVariableName: string;
     let mockProcess: ICurrentProcess;
 
@@ -37,7 +37,7 @@ suite('Resolving Environment Variables when Debugging', () => {
         const envParser = ioc.serviceContainer.get<IEnvironmentVariablesService>(IEnvironmentVariablesService);
         const pathUtils = ioc.serviceContainer.get<IPathUtils>(IPathUtils);
         mockProcess = ioc.serviceContainer.get<ICurrentProcess>(ICurrentProcess);
-        helper = new DebugClientHelper(envParser, pathUtils, mockProcess);
+        debugEnvParser = new DebugEnvironmentVariablesHelper(envParser, pathUtils, mockProcess);
         pathVariableName = pathUtils.getPathVariableName();
     });
     suiteTeardown(closeActiveWindows);
@@ -60,7 +60,7 @@ suite('Resolving Environment Variables when Debugging', () => {
             // tslint:disable-next-line:no-any
         } as any as LaunchRequestArguments;
 
-        const envVars = await helper.getEnvironmentVariables(args);
+        const envVars = await debugEnvParser.getEnvironmentVariables(args);
         expect(envVars).not.be.undefined;
         expect(Object.keys(envVars)).lengthOf(expectedNumberOfVariables, 'Incorrect number of variables');
         expect(envVars).to.have.property('PYTHONUNBUFFERED', '1', 'Property not found');
@@ -97,7 +97,7 @@ suite('Resolving Environment Variables when Debugging', () => {
             // tslint:disable-next-line:no-any
         } as any as LaunchRequestArguments;
 
-        const envVars = await helper.getEnvironmentVariables(args);
+        const envVars = await debugEnvParser.getEnvironmentVariables(args);
 
         // tslint:disable-next-line:no-unused-expression chai-vague-errors
         expect(envVars).not.be.undefined;
@@ -154,7 +154,7 @@ suite('Resolving Environment Variables when Debugging', () => {
             console, env
         } as any as LaunchRequestArguments;
 
-        const envVars = await helper.getEnvironmentVariables(args);
+        const envVars = await debugEnvParser.getEnvironmentVariables(args);
         expect(envVars).not.be.undefined;
         expect(Object.keys(envVars)).lengthOf(expectedNumberOfVariables, 'Incorrect number of variables');
         expect(envVars).to.have.property('PYTHONPATH');

diff --git a/src/test/debugger/extension/configuration/resolvers/launch.unit.test.ts b/src/test/debugger/extension/configuration/resolvers/launch.unit.test.ts
@@ -9,31 +9,27 @@ import { expect } from 'chai';
 import * as path from 'path';
 import * as TypeMoq from 'typemoq';
 import { DebugConfiguration, DebugConfigurationProvider, TextDocument, TextEditor, Uri, WorkspaceFolder } from 'vscode';
-import { InvalidPythonPathInDebuggerServiceId } from '../../../../../client/application/diagnostics/checks/invalidPythonPathInDebugger';
-import { IDiagnosticsService, IInvalidPythonPathInDebuggerService } from '../../../../../client/application/diagnostics/types';
-import { IApplicationShell, IDocumentManager, IWorkspaceService } from '../../../../../client/common/application/types';
+import { IInvalidPythonPathInDebuggerService } from '../../../../../client/application/diagnostics/types';
+import { IDocumentManager, IWorkspaceService } from '../../../../../client/common/application/types';
 import { PYTHON_LANGUAGE } from '../../../../../client/common/constants';
-import { IFileSystem, IPlatformService } from '../../../../../client/common/platform/types';
+import { IPlatformService } from '../../../../../client/common/platform/types';
 import { IPythonExecutionFactory, IPythonExecutionService } from '../../../../../client/common/process/types';
-import { IConfigurationService, ILogger, IPythonSettings } from '../../../../../client/common/types';
+import { IConfigurationService, IPythonSettings } from '../../../../../client/common/types';
 import { DebuggerTypeName } from '../../../../../client/debugger/constants';
+import { IDebugEnvironmentVariablesService } from '../../../../../client/debugger/extension/configuration/resolvers/helper';
 import { LaunchConfigurationResolver } from '../../../../../client/debugger/extension/configuration/resolvers/launch';
 import { DebugOptions, LaunchRequestArguments } from '../../../../../client/debugger/types';
 import { IInterpreterHelper } from '../../../../../client/interpreter/contracts';
-import { IServiceContainer } from '../../../../../client/ioc/types';
 
 suite('Debugging - Config Resolver Launch', () => {
-    let serviceContainer: TypeMoq.IMock<IServiceContainer>;
     let debugProvider: DebugConfigurationProvider;
     let platformService: TypeMoq.IMock<IPlatformService>;
-    let fileSystem: TypeMoq.IMock<IFileSystem>;
-    let appShell: TypeMoq.IMock<IApplicationShell>;
     let pythonExecutionService: TypeMoq.IMock<IPythonExecutionService>;
-    let logger: TypeMoq.IMock<ILogger>;
     let helper: TypeMoq.IMock<IInterpreterHelper>;
     let workspaceService: TypeMoq.IMock<IWorkspaceService>;
     let documentManager: TypeMoq.IMock<IDocumentManager>;
     let diagnosticsService: TypeMoq.IMock<IInvalidPythonPathInDebuggerService>;
+    let debugEnvHelper: TypeMoq.IMock<IDebugEnvironmentVariablesService>;
     function createMoqWorkspaceFolder(folderPath: string) {
         const folder = TypeMoq.Mock.ofType<WorkspaceFolder>();
         folder.setup(f => f.uri).returns(() => Uri.file(folderPath));
@@ -43,13 +39,10 @@ suite('Debugging - Config Resolver Launch', () => {
         const confgService = TypeMoq.Mock.ofType<IConfigurationService>();
         workspaceService = TypeMoq.Mock.ofType<IWorkspaceService>();
         documentManager = TypeMoq.Mock.ofType<IDocumentManager>();
-        serviceContainer = TypeMoq.Mock.ofType<IServiceContainer>();
 
         platformService = TypeMoq.Mock.ofType<IPlatformService>();
-        fileSystem = TypeMoq.Mock.ofType<IFileSystem>();
-        appShell = TypeMoq.Mock.ofType<IApplicationShell>();
-        logger = TypeMoq.Mock.ofType<ILogger>();
         diagnosticsService = TypeMoq.Mock.ofType<IInvalidPythonPathInDebuggerService>();
+        debugEnvHelper = TypeMoq.Mock.ofType<IDebugEnvironmentVariablesService>();
 
         pythonExecutionService = TypeMoq.Mock.ofType<IPythonExecutionService>();
         helper = TypeMoq.Mock.ofType<IInterpreterHelper>();
@@ -61,24 +54,22 @@ suite('Debugging - Config Resolver Launch', () => {
             .setup(h => h.validatePythonPath(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
             .returns(() => Promise.resolve(true));
 
-        serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPythonExecutionFactory))).returns(() => factory.object);
-        serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IConfigurationService))).returns(() => confgService.object);
-        serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IPlatformService))).returns(() => platformService.object);
-        serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IFileSystem))).returns(() => fileSystem.object);
-        serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IApplicationShell))).returns(() => appShell.object);
-        serviceContainer.setup(c => c.get(TypeMoq.It.isValue(ILogger))).returns(() => logger.object);
-        serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IInterpreterHelper))).returns(() => helper.object);
-        serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IDiagnosticsService), TypeMoq.It.isValue(InvalidPythonPathInDebuggerServiceId))).returns(() => diagnosticsService.object);
-
         const settings = TypeMoq.Mock.ofType<IPythonSettings>();
         settings.setup(s => s.pythonPath).returns(() => pythonPath);
         if (workspaceFolder) {
             settings.setup(s => s.envFile).returns(() => path.join(workspaceFolder!.uri.fsPath, '.env2'));
         }
         confgService.setup(c => c.getSettings(TypeMoq.It.isAny())).returns(() => settings.object);
         setupOs(isWindows, isMac, isLinux);
-
-        debugProvider = new LaunchConfigurationResolver(workspaceService.object, documentManager.object, diagnosticsService.object, platformService.object, confgService.object);
+        debugEnvHelper.setup(x => x.getEnvironmentVariables(TypeMoq.It.isAny())).returns(() => Promise.resolve({}));
+
+        debugProvider = new LaunchConfigurationResolver(
+            workspaceService.object,
+            documentManager.object,
+            diagnosticsService.object,
+            platformService.object,
+            confgService.object,
+            debugEnvHelper.object);
     }
     function setupActiveEditor(fileName: string | undefined, languageId: string) {
         if (fileName) {
@@ -91,12 +82,10 @@ suite('Debugging - Config Resolver Launch', () => {
         } else {
             documentManager.setup(d => d.activeTextEditor).returns(() => undefined);
         }
-        serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IDocumentManager))).returns(() => documentManager.object);
     }
     function setupWorkspaces(folders: string[]) {
         const workspaceFolders = folders.map(createMoqWorkspaceFolder);
         workspaceService.setup(w => w.workspaceFolders).returns(() => workspaceFolders);
-        serviceContainer.setup(c => c.get(TypeMoq.It.isValue(IWorkspaceService))).returns(() => workspaceService.object);
     }
     function setupOs(isWindows: boolean, isMac: boolean, isLinux: boolean) {
         platformService.setup(p => p.isWindows).returns(() => isWindows);
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Move .env file handling into the extension. This is in preparation to switch to the out-of-proc debug adapter from ptvsd.