Add DA and ptvsd experiments support to remote debugging API

news/1 Enhancements/7549.md

@@ -0,0 +1 @@
+Add support for ptvsd and debug adapter experiments in remote debugging API.

src/client/api.ts

@@ -3,8 +3,11 @@
 
 'use strict';
 
+import { DebugAdapterDescriptorFactory as DebugAdapterExperiment } from './common/experimentGroups';
 import { traceError } from './common/logger';
+import { IConfigurationService, IExperimentsManager } from './common/types';
 import { RemoteDebuggerExternalLauncherScriptProvider } from './debugger/debugAdapter/DebugClients/launcherProvider';
+import { IDebugAdapterDescriptorFactory } from './debugger/extension/types';
 
 /*
  * Do not introduce any breaking changes to this API.
@@ -33,19 +36,26 @@ export interface IExtensionApi {
 }
 
 // tslint:disable-next-line:no-any
-export function buildApi(ready: Promise<any>) {
+export function buildApi(ready: Promise<any>, experimentsManager: IExperimentsManager, debugFactory: IDebugAdapterDescriptorFactory, configuration: IConfigurationService) {
     return {
         // 'ready' will propogate the exception, but we must log it here first.
         ready: ready.catch((ex) => {
             traceError('Failure during activation.', ex);
             return Promise.reject(ex);
         }),
         debug: {
-            // tslint:disable-next-line:no-suspicious-comment
-            // TODO: Add support for ptvsd wheels experiment, see https://github.com/microsoft/vscode-python/issues/7549
             async getRemoteLauncherCommand(host: string, port: number, waitUntilDebuggerAttaches: boolean = true): Promise<string[]> {
+                const pythonSettings = configuration.getSettings(undefined);
+                const useNewDAPtvsd = experimentsManager.inExperiment(DebugAdapterExperiment.experiment) && (await debugFactory.useNewPtvsd(pythonSettings.pythonPath));
+
+                if (!useNewDAPtvsd) {
                 return new RemoteDebuggerExternalLauncherScriptProvider().getLauncherArgs({ host, port, waitUntilDebuggerAttaches });
             }
+
+                // Same logic as in RemoteDebuggerExternalLauncherScriptProvider, but eventually launcherProvider.ts will be deleted.
+                const args = debugFactory.getRemotePtvsdArgs({ host, port, waitUntilDebuggerAttaches });
+                return [await debugFactory.getPtvsdPath(pythonSettings.pythonPath), ...args];
+        }
         }
     };
 }

src/client/debugger/extension/adapter/factory.ts

@@ -17,6 +17,7 @@ import { IPythonExecutionFactory } from '../../../common/process/types';
 import { IExperimentsManager, IExtensions, IPersistentStateFactory } from '../../../common/types';
 import { EXTENSION_ROOT_DIR } from '../../../constants';
 import { IInterpreterService } from '../../../interpreter/contracts';
+import { RemoteDebugOptions } from '../../debugAdapter/types';
 import { AttachRequestArguments, LaunchRequestArguments } from '../../types';
 import { DebugAdapterPtvsdPathInfo, IDebugAdapterDescriptorFactory } from '../types';
 
@@ -40,7 +41,7 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
             // If logToFile is set in the debug config then pass --log-dir <path-to-extension-dir> when launching the debug adapter.
             const logArgs = configuration.logToFile ? ['--log-dir', EXTENSION_ROOT_DIR] : [];
             const ptvsdPathToUse = await this.getPtvsdPath(pythonPath);
-            return new DebugAdapterExecutable(`${pythonPath}`, [ptvsdPathToUse, ...logArgs]);
+            return new DebugAdapterExecutable(`${pythonPath}`, [path.join(ptvsdPathToUse, 'adapter'), ...logArgs]);
         }
 
         // Use the Node debug adapter (and ptvsd_launcher.py)
@@ -52,10 +53,10 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
     }
 
     /**
-     * Check and return whether the user is in the PTVSD wheels experiment or not.
+     * Check and return whether the user should and can use the new PTVSD wheels or not.
      *
      * @param {string} pythonPath Path to the python executable used to launch the Python Debug Adapter (result of `this.getPythonPath()`)
-     * @returns {Promise<boolean>} Whether the user is in the experiment or not.
+     * @returns {Promise<boolean>} Whether the user should and can use the new PTVSD wheels or not.
      * @memberof DebugAdapterDescriptorFactory
      */
     public async useNewPtvsd(pythonPath: string): Promise<boolean> {
@@ -80,7 +81,12 @@ export class DebugAdapterDescriptorFactory implements IDebugAdapterDescriptorFac
             ptvsdPathToUse = path.join(EXTENSION_ROOT_DIR, 'pythonFiles');
         }
 
-        return path.join(ptvsdPathToUse, 'ptvsd', 'adapter');
+        return path.join(ptvsdPathToUse, 'ptvsd');
+    }
+
+    public getRemotePtvsdArgs(remoteDebugOptions: RemoteDebugOptions): string[] {
+        const waitArgs = remoteDebugOptions.waitUntilDebuggerAttaches ? ['--wait'] : [];
+        return ['--default', '--host', remoteDebugOptions.host, '--port', remoteDebugOptions.port.toString(), ...waitArgs];
     }
 
     /**

src/client/debugger/extension/types.ts

@@ -5,6 +5,7 @@
 
 import { CancellationToken, DebugAdapterDescriptorFactory, DebugConfigurationProvider, WorkspaceFolder } from 'vscode';
 import { InputStep, MultiStepInput } from '../../common/utils/multiStepInput';
+import { RemoteDebugOptions } from '../debugAdapter/types';
 import { DebugConfigurationArguments } from '../types';
 
 export const IDebugConfigurationService = Symbol('IDebugConfigurationService');
@@ -36,6 +37,10 @@ export enum PythonPathSource {
 }
 
 export const IDebugAdapterDescriptorFactory = Symbol('IDebugAdapterDescriptorFactory');
-export interface IDebugAdapterDescriptorFactory extends DebugAdapterDescriptorFactory {}
+export interface IDebugAdapterDescriptorFactory extends DebugAdapterDescriptorFactory {
+    useNewPtvsd(pythonPath: string): Promise<boolean>;
+    getPtvsdPath(pythonPath: string): Promise<string>;
+    getRemotePtvsdArgs(remoteDebugOptions: RemoteDebugOptions): string[];
+}
 
 export type DebugAdapterPtvsdPathInfo = { extensionVersion: string; ptvsdPath: string };

src/client/extension.ts

@@ -69,7 +69,7 @@ import { DebuggerTypeName } from './debugger/constants';
 import { DebugSessionEventDispatcher } from './debugger/extension/hooks/eventHandlerDispatcher';
 import { IDebugSessionEventHandlers } from './debugger/extension/hooks/types';
 import { registerTypes as debugConfigurationRegisterTypes } from './debugger/extension/serviceRegistry';
-import { IDebugConfigurationService, IDebuggerBanner } from './debugger/extension/types';
+import { IDebugAdapterDescriptorFactory, IDebugConfigurationService, IDebuggerBanner } from './debugger/extension/types';
 import { registerTypes as formattersRegisterTypes } from './formatters/serviceRegistry';
 import { AutoSelectionRule, IInterpreterAutoSelectionRule, IInterpreterAutoSelectionService } from './interpreter/autoSelection/types';
 import { IInterpreterSelector } from './interpreter/configuration/types';
@@ -194,7 +194,12 @@ async function activateUnsafe(context: ExtensionContext): Promise<IExtensionApi>
     durations.endActivateTime = stopWatch.elapsedTime;
     activationDeferred.resolve();
 
-    const api = buildApi(Promise.all([activationDeferred.promise, activationPromise]));
+    const api = buildApi(
+        Promise.all([activationDeferred.promise, activationPromise]),
+        serviceContainer.get<IExperimentsManager>(IExperimentsManager),
+        serviceContainer.get<IDebugAdapterDescriptorFactory>(IDebugAdapterDescriptorFactory),
+        configuration
+    );
     // In test environment return the DI Container.
     if (isTestExecution()) {
         // tslint:disable:no-any

src/test/extension.unit.test.ts

@@ -6,22 +6,95 @@
 // tslint:disable:no-any
 
 import { expect } from 'chai';
+import * as path from 'path';
+import { anyString, anything, instance, mock, when } from 'ts-mockito';
 import { buildApi } from '../client/api';
 import { ApplicationEnvironment } from '../client/common/application/applicationEnvironment';
 import { IApplicationEnvironment } from '../client/common/application/types';
+import { ConfigurationService } from '../client/common/configuration/service';
 import { EXTENSION_ROOT_DIR } from '../client/common/constants';
-
-const expectedPath = `${EXTENSION_ROOT_DIR.fileToCommandArgument()}/pythonFiles/ptvsd_launcher.py`;
+import { ExperimentsManager } from '../client/common/experiments';
+import { IConfigurationService, IExperimentsManager, IPythonSettings } from '../client/common/types';
+import { DebugAdapterDescriptorFactory } from '../client/debugger/extension/adapter/factory';
+import { IDebugAdapterDescriptorFactory } from '../client/debugger/extension/types';
 
 suite('Extension API Debugger', () => {
-    test('Test debug launcher args (no-wait)', async () => {
-        const args = await buildApi(Promise.resolve()).debug.getRemoteLauncherCommand('something', 1234, false);
-        const expectedArgs = [expectedPath, '--default', '--host', 'something', '--port', '1234'];
+    const expectedLauncherPath = `${EXTENSION_ROOT_DIR.fileToCommandArgument()}/pythonFiles/ptvsd_launcher.py`;
+    const ptvsdPath = path.join('path', 'to', 'ptvsd');
+
+    let experimentsManager: IExperimentsManager;
+    let debugAdapterFactory: IDebugAdapterDescriptorFactory;
+    let configurationService: IConfigurationService;
+
+    setup(() => {
+        experimentsManager = mock(ExperimentsManager);
+        debugAdapterFactory = mock(DebugAdapterDescriptorFactory);
+        configurationService = mock(ConfigurationService);
+    });
+
+    function mockInExperiment() {
+        when(experimentsManager.inExperiment(anyString())).thenReturn(true);
+        when(debugAdapterFactory.useNewPtvsd(anyString())).thenResolve(true);
+        when(debugAdapterFactory.getPtvsdPath(anyString())).thenResolve(ptvsdPath);
+        when(configurationService.getSettings(undefined)).thenReturn(({ pythonPath: 'python' } as any) as IPythonSettings);
+    }
+
+    function mockNotInExperiment() {
+        when(experimentsManager.inExperiment(anyString())).thenReturn(false);
+        when(debugAdapterFactory.useNewPtvsd(anyString())).thenResolve(false);
+    }
+
+    test('Test debug launcher args (no-wait and not in experiment)', async () => {
+        mockNotInExperiment();
+
+        const args = await buildApi(Promise.resolve(), instance(experimentsManager), instance(debugAdapterFactory), instance(configurationService)).debug.getRemoteLauncherCommand(
+            'something',
+            1234,
+            false
+        );
+        const expectedArgs = [expectedLauncherPath, '--default', '--host', 'something', '--port', '1234'];
+
         expect(args).to.be.deep.equal(expectedArgs);
     });
-    test('Test debug launcher args (wait)', async () => {
-        const args = await buildApi(Promise.resolve()).debug.getRemoteLauncherCommand('something', 1234, true);
-        const expectedArgs = [expectedPath, '--default', '--host', 'something', '--port', '1234', '--wait'];
+
+    test('Test debug launcher args (no-wait and in experiment)', async () => {
+        mockInExperiment();
+        when(debugAdapterFactory.getRemotePtvsdArgs(anything())).thenReturn(['--default', '--host', 'something', '--port', '1234']);
+
+        const args = await buildApi(Promise.resolve(), instance(experimentsManager), instance(debugAdapterFactory), instance(configurationService)).debug.getRemoteLauncherCommand(
+            'something',
+            1234,
+            false
+        );
+        const expectedArgs = [ptvsdPath, '--default', '--host', 'something', '--port', '1234'];
+
+        expect(args).to.be.deep.equal(expectedArgs);
+    });
+
+    test('Test debug launcher args (wait and not in experiment)', async () => {
+        mockNotInExperiment();
+
+        const args = await buildApi(Promise.resolve(), instance(experimentsManager), instance(debugAdapterFactory), instance(configurationService)).debug.getRemoteLauncherCommand(
+            'something',
+            1234,
+            true
+        );
+        const expectedArgs = [expectedLauncherPath, '--default', '--host', 'something', '--port', '1234', '--wait'];
+
+        expect(args).to.be.deep.equal(expectedArgs);
+    });
+
+    test('Test debug launcher args (wait and in experiment)', async () => {
+        mockInExperiment();
+        when(debugAdapterFactory.getRemotePtvsdArgs(anything())).thenReturn(['--default', '--host', 'something', '--port', '1234', '--wait']);
+
+        const args = await buildApi(Promise.resolve(), instance(experimentsManager), instance(debugAdapterFactory), instance(configurationService)).debug.getRemoteLauncherCommand(
+            'something',
+            1234,
+            true
+        );
+        const expectedArgs = [ptvsdPath, '--default', '--host', 'something', '--port', '1234', '--wait'];
+
         expect(args).to.be.deep.equal(expectedArgs);
     });
 });