Skip to content

Commit

Permalink
Do not query for version and kind if it's not needed when reporting a…
Browse files Browse the repository at this point in the history
…n issue (#17815)

* Do not query for version and kind if it's not needed when reporting an issue

* News entry
  • Loading branch information
Kartik Raj authored Oct 20, 2021
1 parent e8f2a7d commit 7f9b6e0
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 22 deletions.
1 change: 1 addition & 0 deletions news/3 Code Health/17815.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Do not query for version and kind if it's not needed when reporting an issue.
13 changes: 6 additions & 7 deletions src/client/common/application/commands/reportIssueCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ import { inject, injectable } from 'inversify';
import { IExtensionSingleActivationService } from '../../../activation/types';
import { ICommandManager, IWorkspaceService } from '../types';
import { EXTENSION_ROOT_DIR } from '../../../constants';
import { IInterpreterService, IInterpreterVersionService } from '../../../interpreter/contracts';
import { identifyEnvironment } from '../../../pythonEnvironments/common/environmentIdentifier';
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';

/**
* Allows the user to report an issue related to the Python extension using our template.
Expand All @@ -26,7 +26,6 @@ export class ReportIssueCommandHandler implements IExtensionSingleActivationServ
@inject(ICommandManager) private readonly commandManager: ICommandManager,
@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService,
@inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
@inject(IInterpreterVersionService) private readonly interpreterVersionService: IInterpreterVersionService,
@inject(IConfigurationService) protected readonly configurationService: IConfigurationService,
) {}

Expand Down Expand Up @@ -65,15 +64,15 @@ export class ReportIssueCommandHandler implements IExtensionSingleActivationServ
}
});
const template = await fs.readFile(this.templatePath, 'utf8');
const interpreterPath = (await this.interpreterService.getActiveInterpreter())?.path || 'not-selected';
const pythonVersion = await this.interpreterVersionService.getVersion(interpreterPath, '');
const interpreter = await this.interpreterService.getActiveInterpreter();
const pythonVersion = interpreter?.version?.raw ?? '';
const languageServer =
this.workspaceService.getConfiguration('python').get<string>('languageServer') || 'Not Found';
const virtualEnv = await identifyEnvironment(interpreterPath);
const virtualEnvKind = interpreter?.envType || EnvironmentType.Unknown;

await this.commandManager.executeCommand('workbench.action.openIssueReporter', {
extensionId: 'ms-python.python',
issueBody: template.format(pythonVersion, virtualEnv, languageServer, userSettings),
issueBody: template.format(pythonVersion, virtualEnvKind, languageServer, userSettings),
});
sendTelemetryEvent(EventName.USE_REPORT_ISSUE_COMMAND, undefined, {});
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/common/application/commands/issueTemplateVenv1.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ You can attach such things **after** you create your issue on GitHub.
# Diagnostic data

- Python version (& distribution if applicable, e.g. Anaconda): 3.9.0
- Type of virtual environment used (e.g. conda, venv, virtualenv, etc.): virt-venv
- Type of virtual environment used (e.g. conda, venv, virtualenv, etc.): Venv
- Value of the `python.languageServer` setting: Pylance

<details>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,7 @@ import { CommandManager } from '../../../../client/common/application/commandMan
import { ReportIssueCommandHandler } from '../../../../client/common/application/commands/reportIssueCommand';
import { ICommandManager, IWorkspaceService } from '../../../../client/common/application/types';
import { WorkspaceService } from '../../../../client/common/application/workspace';
import { IInterpreterService, IInterpreterVersionService } from '../../../../client/interpreter/contracts';
import { InterpreterVersionService } from '../../../../client/interpreter/interpreterVersion';
import { PythonEnvKind } from '../../../../client/pythonEnvironments/base/info';
import * as EnvIdentifier from '../../../../client/pythonEnvironments/common/environmentIdentifier';
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';
Expand All @@ -27,32 +24,33 @@ 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';

suite('Report Issue Command', () => {
let reportIssueCommandHandler: ReportIssueCommandHandler;
let cmdManager: ICommandManager;
let workspaceService: IWorkspaceService;
let interpreterVersionService: IInterpreterVersionService;
let interpreterService: IInterpreterService;
let configurationService: IConfigurationService;
let identifyEnvironmentStub: sinon.SinonStub;
let getPythonOutputContentStub: sinon.SinonStub;

setup(async () => {
interpreterVersionService = mock(InterpreterVersionService);
workspaceService = mock(WorkspaceService);
cmdManager = mock(CommandManager);
interpreterService = mock(InterpreterService);
configurationService = mock(ConfigurationService);

when(cmdManager.executeCommand('workbench.action.openIssueReporter', anything())).thenResolve();
when(interpreterVersionService.getVersion(anything(), anything())).thenResolve('3.9.0');
when(workspaceService.getConfiguration('python')).thenReturn(
new MockWorkspaceConfiguration({
languageServer: LanguageServerType.Node,
}),
);
when(interpreterService.getActiveInterpreter(anything())).thenResolve(undefined);
const interpreter = ({
envType: EnvironmentType.Venv,
version: { raw: '3.9.0' },
} as unknown) as PythonEnvironment;
when(interpreterService.getActiveInterpreter()).thenResolve(interpreter);
when(configurationService.getSettings()).thenReturn({
experiments: {
enabled: true,
Expand All @@ -64,8 +62,6 @@ suite('Report Issue Command', () => {
venvPath: 'path',
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as any);
identifyEnvironmentStub = sinon.stub(EnvIdentifier, 'identifyEnvironment');
identifyEnvironmentStub.resolves(PythonEnvKind.Venv);

cmdManager = mock(CommandManager);

Expand All @@ -75,15 +71,13 @@ suite('Report Issue Command', () => {
instance(cmdManager),
instance(workspaceService),
instance(interpreterService),
instance(interpreterVersionService),
instance(configurationService),
);
await reportIssueCommandHandler.activate();
});

teardown(() => {
identifyEnvironmentStub.restore();
getPythonOutputContentStub.restore();
sinon.restore();
});

test('Test if issue body is filled', async () => {
Expand Down

0 comments on commit 7f9b6e0

Please sign in to comment.