src/goDebugConfiguration: default to dlv-dap in preview mode

Except when the mode is 'remote'.
Moreover, if the mode is 'remote', fall back to 'legacy' mode
since that's not a supported operation mode in dlv-dap.

Fixes #1539
Fixes #1517

Change-Id: I3df47ae9aafce56a6a8d809b21838cb90f3d03fa
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/325581
Trust: Hyang-Ah Hana Kim <[email protected]>
Reviewed-by: Polina Sokolova <[email protected]>

Author: hyangah

src/goDebugConfiguration.ts

@@ -18,6 +18,7 @@ import {
 	promptForUpdatingTool,
 	shouldUpdateTool
 } from './goInstallTools';
+import { isInPreviewMode } from './goLanguageServer';
 import { packagePathToGoModPathMap } from './goModules';
 import { getTool, getToolAtVersion } from './goTools';
 import { pickProcess, pickProcessByName } from './pickProcess';
@@ -147,8 +148,26 @@ export class GoDebugConfigurationProvider implements vscode.DebugConfigurationPr
 		// Figure out which debugAdapter is being used first, so we can use this to send warnings
 		// for properties that don't apply.
 		if (!debugConfiguration.hasOwnProperty('debugAdapter') && dlvConfig.hasOwnProperty('debugAdapter')) {
-			debugConfiguration['debugAdapter'] = dlvConfig['debugAdapter'];
+			const { globalValue, workspaceValue } = goConfig.inspect('delveConfig.debugAdapter');
+			// user configured the default debug adapter through settings.json.
+			if (globalValue !== undefined || workspaceValue !== undefined) {
+				debugConfiguration['debugAdapter'] = dlvConfig['debugAdapter'];
+			}
+		}
+		if (!debugConfiguration['debugAdapter']) {
+			// for nightly/dev mode, default to dlv-dap.
+			// TODO(hyangah): when we switch the stable version's default to 'dlv-dap', adjust this.
+			debugConfiguration['debugAdapter'] =
+				isInPreviewMode() && debugConfiguration['mode'] !== 'remote' ? 'dlv-dap' : 'legacy';
+		}
+		if (debugConfiguration['debugAdapter'] === 'dlv-dap' && debugConfiguration['mode'] === 'remote') {
+			this.showWarning(
+				'ignoreDlvDAPInRemoteModeWarning',
+				"debugAdapter type of 'dlv-dap' with mode 'remote' is unsupported. Fall back to the 'legacy' debugAdapter for 'remote' mode."
+			);
+			debugConfiguration['debugAdapter'] = 'legacy';
 		}
+
 		const debugAdapter = debugConfiguration['debugAdapter'] === 'dlv-dap' ? 'dlv-dap' : 'dlv';
 
 		let useApiV1 = false;
@@ -270,7 +289,7 @@ export class GoDebugConfigurationProvider implements vscode.DebugConfigurationPr
 				// file path instead of the currently active file.
 				filename = debugConfiguration['program'];
 			}
-			debugConfiguration['mode'] = filename.endsWith('_test.go') ? 'test' : 'debug';
+			debugConfiguration['mode'] = filename?.endsWith('_test.go') ? 'test' : 'debug';
 		}
 
 		if (debugConfiguration['mode'] === 'test' && debugConfiguration['program'].endsWith('_test.go')) {

test/integration/goDebug.test.ts

@@ -308,7 +308,7 @@ const testAll = (ctx: Mocha.Context, isDlvDap: boolean) => {
 		name: 'Attach',
 		type: 'go',
 		request: 'attach',
-		mode: 'remote',
+		mode: 'remote', // This implies debugAdapter = legacy.
 		host: '127.0.0.1',
 		port: 3456
 	};
@@ -475,10 +475,9 @@ const testAll = (ctx: Mocha.Context, isDlvDap: boolean) => {
 				this.skip(); // not working in dlv-dap.
 			}
 
-			if (isDlvDap) {
-				const config = { name: 'Launch', type: 'go', request: 'launch', program: DATA_ROOT };
-				await initializeDebugConfig(config);
-			}
+			// fake config that will be used to initialize fixtures.
+			const config = { name: 'Launch', type: 'go', request: 'launch', program: DATA_ROOT };
+			await initializeDebugConfig(config);
 
 			try {
 				await dc.send('illegal_request');
@@ -491,10 +490,8 @@ const testAll = (ctx: Mocha.Context, isDlvDap: boolean) => {
 
 	suite('initialize', () => {
 		test('should return supported features', async () => {
-			if (isDlvDap) {
-				const config = { name: 'Launch', type: 'go', request: 'launch', program: DATA_ROOT };
-				await initializeDebugConfig(config);
-			}
+			const config = { name: 'Launch', type: 'go', request: 'launch', program: DATA_ROOT };
+			await initializeDebugConfig(config);
 			await dc.initializeRequest().then((response) => {
 				response.body = response.body || {};
 				assert.strictEqual(response.body.supportsConditionalBreakpoints, true);
@@ -511,10 +508,8 @@ const testAll = (ctx: Mocha.Context, isDlvDap: boolean) => {
 				this.skip(); // not working in dlv-dap.
 			}
 
-			if (isDlvDap) {
-				const config = { name: 'Launch', type: 'go', request: 'launch', program: DATA_ROOT };
-				await initializeDebugConfig(config);
-			}
+			const config = { name: 'Launch', type: 'go', request: 'launch', program: DATA_ROOT };
+			await initializeDebugConfig(config);
 			try {
 				await dc.initializeRequest({
 					adapterID: 'mock',
@@ -1993,6 +1988,10 @@ const testAll = (ctx: Mocha.Context, isDlvDap: boolean) => {
 			config['logOutput'] = 'dap,debugger';
 			config['showLog'] = true;
 			config['trace'] = 'verbose';
+		} else {
+			config['debugAdapter'] = 'legacy';
+			// be explicit and prevent resolveDebugConfiguration from picking
+			// a default debugAdapter for us.
 		}
 
 		// Give each test a distinct debug binary. If a previous test

test/integration/goDebugConfiguration.test.ts

@@ -12,6 +12,7 @@ import { GoDebugConfigurationProvider } from '../../src/goDebugConfiguration';
 import { updateGoVarsFromConfig } from '../../src/goInstallTools';
 import { rmdirRecursive } from '../../src/util';
 import goEnv = require('../../src/goEnv');
+import { isInPreviewMode } from '../../src/goLanguageServer';
 
 suite('Debug Environment Variable Merge Test', () => {
 	const debugConfigProvider = new GoDebugConfigurationProvider();
@@ -468,3 +469,54 @@ suite('Debug Configuration Auto Mode', () => {
 		assert.strictEqual(config.program, '/path/to');
 	});
 });
+
+suite('Debug Configuration Default DebugAdapter', () => {
+	const debugConfigProvider = new GoDebugConfigurationProvider();
+	test(`default debugAdapter when isInPreviewMode=${isInPreviewMode()} should be 'dlv-dap'`, () => {
+		const config = {
+			name: 'Launch',
+			type: 'go',
+			request: 'launch',
+			mode: 'auto',
+			program: '/path/to/main.go'
+		};
+
+		debugConfigProvider.resolveDebugConfiguration(undefined, config);
+		const resolvedConfig = config as any;
+		const want = isInPreviewMode() ? 'dlv-dap' : 'legacy';
+		assert.strictEqual(resolvedConfig['debugAdapter'], want);
+	});
+
+	test("default debugAdapter for remote mode should be always 'legacy'", () => {
+		const config = {
+			name: 'Attach',
+			type: 'go',
+			request: 'attach',
+			mode: 'remote',
+			program: '/path/to/main_test.go',
+			cwd: '/path'
+		};
+
+		const want = 'legacy'; // remote mode works only with legacy mode.
+		debugConfigProvider.resolveDebugConfiguration(undefined, config);
+		const resolvedConfig = config as any;
+		assert.strictEqual(resolvedConfig['debugAdapter'], want);
+	});
+
+	test('debugAdapter=dlv-dap should be ignored for remote mode', () => {
+		const config = {
+			name: 'Attach',
+			type: 'go',
+			request: 'attach',
+			mode: 'remote',
+			debugAdapter: 'dlv-dap',
+			program: '/path/to/main_test.go',
+			cwd: '/path'
+		};
+
+		const want = 'legacy'; // remote mode works only with legacy mode.
+		debugConfigProvider.resolveDebugConfiguration(undefined, config);
+		const resolvedConfig = config as any;
+		assert.strictEqual(resolvedConfig['debugAdapter'], want);
+	});
+});