Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/1 Enhancements/19067.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve information collected by the `Python: Report Issue` command.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion resources/report_issue_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ XXX
<p>

```
{3}
{3}{4}
```

</p>
Expand Down
7 changes: 2 additions & 5 deletions resources/report_issue_user_settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"pythonPath": "placeholder",
"onDidChange": false,
"defaultInterpreterPath": "placeholder",
"defaultLS": true,
"defaultLS": false,
"envFile": "placeholder",
"venvPath": "placeholder",
"venvFolders": "placeholder",
Expand All @@ -25,7 +25,7 @@
"linting": {
"enabled": true,
"cwd": "placeholder",
"Flake8Args": "placeholder",
"flake8Args": "placeholder",
"flake8CategorySeverity": false,
"flake8Enabled": true,
"flake8Path": "placeholder",
Expand Down Expand Up @@ -85,9 +85,6 @@
"testing": {
"cwd": "placeholder",
"debugPort": true,
"nosetestArgs": "placeholder",
"nosetestsEnabled": true,
"nosetestPath": "placeholder",
"promptToConfigure": true,
"pytestArgs": "placeholder",
"pytestEnabled": true,
Expand Down
56 changes: 48 additions & 8 deletions src/client/common/application/commands/reportIssueCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
Expand All @@ -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);
}
}
}
});
Expand All @@ -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);
}
}
4 changes: 3 additions & 1 deletion src/test/common/application/commands/issueTemplateVenv1.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@ XXX
```

experiments
• enabled: true
• enabled: false
• optInto: []
• optOutFrom: []

venvPath: "<placeholder>"

pipenvPath: "<placeholder>"

Comment on lines 30 to +38
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Old way.

```

</p>
Expand Down
39 changes: 39 additions & 0 deletions src/test/common/application/commands/issueTemplateVenv2.md
Original file line number Diff line number Diff line change
@@ -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>"
Comment on lines +24 to +34
Copy link
Author

@karrtikr karrtikr May 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New behavior.

```

</p>
</details>
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable global-require */
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

Expand All @@ -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(
Expand All @@ -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);

Expand All @@ -67,6 +76,7 @@ suite('Report Issue Command', () => {
instance(workspaceService),
instance(interpreterService),
instance(configurationService),
instance(appEnvironment),
);
await reportIssueCommandHandler.activate();
});
Expand All @@ -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(
Expand All @@ -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();
Expand Down