Improve information collected by the Python: Report Issue command (#19068)

Kartik Raj · karthiknadig · commit 259dbd5e8ee8 · 2022-05-10T14:09:57.000-07:00
* Improve information collected by the `Python: Report Issue` command

* Fix package.json

* Add test

* Add test for multiroot
diff --git a/news/1 Enhancements/19067.md b/news/1 Enhancements/19067.md
@@ -0,0 +1 @@
+Improve information collected by the `Python: Report Issue` command.
diff --git a/package.json b/package.json
@@ -922,6 +922,7 @@
                     "type": "string"
                 },
                 "python.tensorBoard.logDirectory": {
+                    "default": "",
                     "description": "Set this setting to your preferred TensorBoard log directory to skip log directory prompt when starting TensorBoard.",
                     "scope": "application",
                     "type": "string"
diff --git a/resources/report_issue_template.md b/resources/report_issue_template.md
@@ -26,7 +26,7 @@ XXX
 <p>
 
 ```
-{3}
+{3}{4}
 ```
 
 </p>
diff --git a/resources/report_issue_user_settings.json b/resources/report_issue_user_settings.json
@@ -3,7 +3,7 @@
     "pythonPath": "placeholder",
     "onDidChange": false,
     "defaultInterpreterPath": "placeholder",
-    "defaultLS": true,
+    "defaultLS": false,
     "envFile": "placeholder",
     "venvPath": "placeholder",
     "venvFolders": "placeholder",
@@ -25,7 +25,7 @@
     "linting": {
         "enabled": true,
         "cwd": "placeholder",
-        "Flake8Args": "placeholder",
+        "flake8Args": "placeholder",
         "flake8CategorySeverity": false,
         "flake8Enabled": true,
         "flake8Path": "placeholder",
@@ -85,9 +85,6 @@
     "testing": {
         "cwd": "placeholder",
         "debugPort": true,
-        "nosetestArgs": "placeholder",
-        "nosetestsEnabled": true,
-        "nosetestPath": "placeholder",
         "promptToConfigure": true,
         "pytestArgs": "placeholder",
         "pytestEnabled": true,
diff --git a/src/client/common/application/commands/reportIssueCommand.ts b/src/client/common/application/commands/reportIssueCommand.ts
@@ -7,15 +7,18 @@ import * as fs from 'fs-extra';
 import * as os from 'os';
 import * as path from 'path';
 import { inject, injectable } from 'inversify';
+import { isEqual } from 'lodash';
 import { IExtensionSingleActivationService } from '../../../activation/types';
-import { ICommandManager, IWorkspaceService } from '../types';
+import { IApplicationEnvironment, ICommandManager, IWorkspaceService } from '../types';
 import { EXTENSION_ROOT_DIR } from '../../../constants';
 import { IInterpreterService } from '../../../interpreter/contracts';
 import { Commands } from '../../constants';
 import { IConfigurationService, IPythonSettings } from '../../types';
 import { sendTelemetryEvent } from '../../../telemetry';
 import { EventName } from '../../../telemetry/constants';
 import { EnvironmentType } from '../../../pythonEnvironments/info';
+import { PythonSettings } from '../../configSettings';
+import { SystemVariables } from '../../variables/systemVariables';
 
 /**
  * Allows the user to report an issue related to the Python extension using our template.
@@ -24,12 +27,18 @@ import { EnvironmentType } from '../../../pythonEnvironments/info';
 export class ReportIssueCommandHandler implements IExtensionSingleActivationService {
     public readonly supportedWorkspaceTypes = { untrustedWorkspace: false, virtualWorkspace: true };
 
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    private readonly packageJSONSettings: any;
+
     constructor(
         @inject(ICommandManager) private readonly commandManager: ICommandManager,
         @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService,
         @inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
         @inject(IConfigurationService) protected readonly configurationService: IConfigurationService,
-    ) {}
+        @inject(IApplicationEnvironment) appEnvironment: IApplicationEnvironment,
+    ) {
+        this.packageJSONSettings = appEnvironment.packageJson?.contributes?.configuration?.properties;
+    }
 
     public async activate(): Promise<void> {
         this.commandManager.registerCommand(Commands.ReportIssue, this.openReportIssue, this);
@@ -48,20 +57,31 @@ export class ReportIssueCommandHandler implements IExtensionSingleActivationServ
             const argSetting = argSettings[property];
             if (argSetting) {
                 if (typeof argSetting === 'object') {
-                    userSettings = userSettings.concat(os.EOL, property, os.EOL);
+                    let propertyHeaderAdded = false;
                     const argSettingsDict = (settings[property] as unknown) as Record<string, unknown>;
                     if (typeof argSettingsDict === 'object') {
                         Object.keys(argSetting).forEach((item) => {
                             const prop = argSetting[item];
                             if (prop) {
-                                const value = prop === true ? JSON.stringify(argSettingsDict[item]) : '"<placeholder>"';
-                                userSettings = userSettings.concat('• ', item, ': ', value, os.EOL);
+                                const defaultValue = this.getDefaultValue(`${property}.${item}`);
+                                if (defaultValue === undefined || !isEqual(defaultValue, argSettingsDict[item])) {
+                                    if (!propertyHeaderAdded) {
+                                        userSettings = userSettings.concat(os.EOL, property, os.EOL);
+                                        propertyHeaderAdded = true;
+                                    }
+                                    const value =
+                                        prop === true ? JSON.stringify(argSettingsDict[item]) : '"<placeholder>"';
+                                    userSettings = userSettings.concat('• ', item, ': ', value, os.EOL);
+                                }
                             }
                         });
                     }
                 } else {
-                    const value = argSetting === true ? JSON.stringify(settings[property]) : '"<placeholder>"';
-                    userSettings = userSettings.concat(os.EOL, property, ': ', value, os.EOL);
+                    const defaultValue = this.getDefaultValue(property);
+                    if (defaultValue === undefined || !isEqual(defaultValue, settings[property])) {
+                        const value = argSetting === true ? JSON.stringify(settings[property]) : '"<placeholder>"';
+                        userSettings = userSettings.concat(os.EOL, property, ': ', value, os.EOL);
+                    }
                 }
             }
         });
@@ -72,10 +92,30 @@ export class ReportIssueCommandHandler implements IExtensionSingleActivationServ
             this.workspaceService.getConfiguration('python').get<string>('languageServer') || 'Not Found';
         const virtualEnvKind = interpreter?.envType || EnvironmentType.Unknown;
 
+        const hasMultipleFolders = (this.workspaceService.workspaceFolders?.length ?? 0) > 1;
+        const hasMultipleFoldersText =
+            hasMultipleFolders && userSettings !== ''
+                ? `Multiroot scenario, following user settings may not apply:${os.EOL}`
+                : '';
         await this.commandManager.executeCommand('workbench.action.openIssueReporter', {
             extensionId: 'ms-python.python',
-            issueBody: template.format(pythonVersion, virtualEnvKind, languageServer, userSettings),
+            issueBody: template.format(
+                pythonVersion,
+                virtualEnvKind,
+                languageServer,
+                hasMultipleFoldersText,
+                userSettings,
+            ),
         });
         sendTelemetryEvent(EventName.USE_REPORT_ISSUE_COMMAND, undefined, {});
     }
+
+    private getDefaultValue(settingKey: string) {
+        if (!this.packageJSONSettings) {
+            return undefined;
+        }
+        const resource = PythonSettings.getSettingsUriAndTarget(undefined, this.workspaceService).uri;
+        const systemVariables = new SystemVariables(resource, undefined, this.workspaceService);
+        return systemVariables.resolveAny(this.packageJSONSettings[`python.${settingKey}`]?.default);
+    }
 }
diff --git a/src/test/common/application/commands/issueTemplateVenv1.md b/src/test/common/application/commands/issueTemplateVenv1.md
@@ -28,12 +28,14 @@ XXX
 ```
 
 experiments
-• enabled: true
+• enabled: false
 • optInto: []
 • optOutFrom: []
 
 venvPath: "<placeholder>"
 
+pipenvPath: "<placeholder>"
+
 ```
 
 </p>
diff --git a/src/test/common/application/commands/issueTemplateVenv2.md b/src/test/common/application/commands/issueTemplateVenv2.md
@@ -0,0 +1,39 @@
+<!-- Please fill in all XXX markers -->
+# Behaviour
+## Expected vs. Actual
+
+XXX
+
+## Steps to reproduce:
+
+1. XXX
+
+<!--
+**After** creating the issue on GitHub, you can add screenshots and GIFs of what is happening. Consider tools like https://www.cockos.com/licecap/, https://github.com/phw/peek or https://www.screentogif.com/ for GIF creation.
+-->
+
+<!-- **NOTE**: Everything below is auto-generated; no editing required. -->
+# Diagnostic data
+
+-   Python version (& distribution if applicable, e.g. Anaconda): 3.9.0
+-   Type of virtual environment used (e.g. conda, venv, virtualenv, etc.): Venv
+-   Value of the `python.languageServer` setting: Pylance
+
+<details>
+
+<summary>User Settings</summary>
+
+<p>
+
+```
+Multiroot scenario, following user settings may not apply:
+
+experiments
+• enabled: false
+
+venvPath: "<placeholder>"
+
+```
+
+</p>
+</details>
diff --git a/src/test/common/application/commands/reportIssueCommand.unit.test.ts b/src/test/common/application/commands/reportIssueCommand.unit.test.ts
@@ -1,3 +1,4 @@
+/* eslint-disable global-require */
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
@@ -8,35 +9,42 @@ import * as fs from 'fs-extra';
 import * as path from 'path';
 import { anything, capture, instance, mock, verify, when } from 'ts-mockito';
 import { expect } from 'chai';
+import { WorkspaceFolder } from 'vscode-languageserver-protocol';
 import * as Telemetry from '../../../../client/telemetry';
 import { LanguageServerType } from '../../../../client/activation/types';
 import { CommandManager } from '../../../../client/common/application/commandManager';
 import { ReportIssueCommandHandler } from '../../../../client/common/application/commands/reportIssueCommand';
-import { ICommandManager, IWorkspaceService } from '../../../../client/common/application/types';
+import {
+    IApplicationEnvironment,
+    ICommandManager,
+    IWorkspaceService,
+} from '../../../../client/common/application/types';
 import { WorkspaceService } from '../../../../client/common/application/workspace';
 import { IInterpreterService } from '../../../../client/interpreter/contracts';
 import { MockWorkspaceConfiguration } from '../../../mocks/mockWorkspaceConfig';
-import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../../../constants';
 import { InterpreterService } from '../../../../client/interpreter/interpreterService';
-import { Commands } from '../../../../client/common/constants';
+import { Commands, EXTENSION_ROOT_DIR } from '../../../../client/common/constants';
 import { AllCommands } from '../../../../client/common/application/commands';
 import { ConfigurationService } from '../../../../client/common/configuration/service';
 import { IConfigurationService } from '../../../../client/common/types';
 import { EventName } from '../../../../client/telemetry/constants';
 import { EnvironmentType, PythonEnvironment } from '../../../../client/pythonEnvironments/info';
+import { EXTENSION_ROOT_DIR_FOR_TESTS } from '../../../constants';
 
 suite('Report Issue Command', () => {
     let reportIssueCommandHandler: ReportIssueCommandHandler;
     let cmdManager: ICommandManager;
     let workspaceService: IWorkspaceService;
     let interpreterService: IInterpreterService;
     let configurationService: IConfigurationService;
+    let appEnvironment: IApplicationEnvironment;
 
     setup(async () => {
         workspaceService = mock(WorkspaceService);
         cmdManager = mock(CommandManager);
         interpreterService = mock(InterpreterService);
         configurationService = mock(ConfigurationService);
+        appEnvironment = mock<IApplicationEnvironment>();
 
         when(cmdManager.executeCommand('workbench.action.openIssueReporter', anything())).thenResolve();
         when(workspaceService.getConfiguration('python')).thenReturn(
@@ -51,12 +59,13 @@ suite('Report Issue Command', () => {
         when(interpreterService.getActiveInterpreter()).thenResolve(interpreter);
         when(configurationService.getSettings()).thenReturn({
             experiments: {
-                enabled: true,
+                enabled: false,
                 optInto: [],
                 optOutFrom: [],
             },
             initialize: true,
             venvPath: 'path',
+            pipenvPath: 'pipenv',
             // eslint-disable-next-line @typescript-eslint/no-explicit-any
         } as any);
 
@@ -67,6 +76,7 @@ suite('Report Issue Command', () => {
             instance(workspaceService),
             instance(interpreterService),
             instance(configurationService),
+            instance(appEnvironment),
         );
         await reportIssueCommandHandler.activate();
     });
@@ -75,7 +85,7 @@ suite('Report Issue Command', () => {
         sinon.restore();
     });
 
-    test('Test if issue body is filled', async () => {
+    test('Test if issue body is filled correctly when including all the settings', async () => {
         await reportIssueCommandHandler.openReportIssue();
 
         const templatePath = path.join(
@@ -100,6 +110,45 @@ suite('Report Issue Command', () => {
         const actual = args[1].issueBody;
         expect(actual).to.be.equal(expectedIssueBody);
     });
+
+    test('Test if issue body is filled when only including settings which are explicitly set', async () => {
+        // eslint-disable-next-line import/no-dynamic-require
+        when(appEnvironment.packageJson).thenReturn(require(path.join(EXTENSION_ROOT_DIR, 'package.json')));
+        when(workspaceService.workspaceFolders).thenReturn([
+            instance(mock(WorkspaceFolder)),
+            instance(mock(WorkspaceFolder)),
+        ]); // Multiroot scenario
+        reportIssueCommandHandler = new ReportIssueCommandHandler(
+            instance(cmdManager),
+            instance(workspaceService),
+            instance(interpreterService),
+            instance(configurationService),
+            instance(appEnvironment),
+        );
+        await reportIssueCommandHandler.activate();
+        await reportIssueCommandHandler.openReportIssue();
+
+        const templatePath = path.join(
+            EXTENSION_ROOT_DIR_FOR_TESTS,
+            'src',
+            'test',
+            'common',
+            'application',
+            'commands',
+            'issueTemplateVenv2.md',
+        );
+        const expectedIssueBody = fs.readFileSync(templatePath, 'utf8');
+
+        const args: [string, { extensionId: string; issueBody: string }] = capture<
+            AllCommands,
+            { extensionId: string; issueBody: string }
+        >(cmdManager.executeCommand).last();
+
+        verify(cmdManager.executeCommand('workbench.action.openIssueReporter', anything())).once();
+        expect(args[0]).to.be.equal('workbench.action.openIssueReporter');
+        const actual = args[1].issueBody;
+        expect(actual).to.be.equal(expectedIssueBody);
+    });
     test('Should send telemetry event when run Report Issue Command', async () => {
         const sendTelemetryStub = sinon.stub(Telemetry, 'sendTelemetryEvent');
         await reportIssueCommandHandler.openReportIssue();

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+Improve information collected by the `Python: Report Issue` command.
-Original file line number
+Diff line change
 <p>
 ```
 -{3}
 +{3}{4}
 ```
 </p>