Skip to content

Commit db6cd4b

Browse files
author
Kartik Raj
authored
Ensure experiment framework can be created using standard utils (microsoft#15150)
* Ensure experiment framework can be created using standard utils * Delete unused mock * Code reviews * Edit * Oops * Fix ESLint errors * Fix lint * Fix prettier
1 parent a06f21b commit db6cd4b

6 files changed

Lines changed: 141 additions & 94 deletions

File tree

‎.eslintignore‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,6 @@ src/test/common/terminals/environmentActivationProviders/pipEnvActivationProvide
200200
src/test/common/terminals/environmentActivationProviders/terminalActivation.testvirtualenvs.ts
201201
src/test/common/socketStream.test.ts
202202
src/test/common/configSettings.test.ts
203-
src/test/common/experiments/service.unit.test.ts
204203
src/test/common/experiments/manager.unit.test.ts
205204
src/test/common/experiments/telemetry.unit.test.ts
206205
src/test/common/platform/filesystem.unit.test.ts
@@ -373,7 +372,6 @@ src/client/interpreter/display/interpreterSelectionTip.ts
373372

374373
src/client/api.ts
375374
src/client/extension.ts
376-
src/client/extensionActivation.ts
377375
src/client/sourceMapSupport.ts
378376
src/client/startupTelemetry.ts
379377

@@ -553,7 +551,6 @@ src/client/common/markdown/restTextConverter.ts
553551
src/client/common/featureDeprecationManager.ts
554552
src/client/common/experiments/manager.ts
555553
src/client/common/experiments/telemetry.ts
556-
src/client/common/experiments/service.ts
557554
src/client/common/refBool.ts
558555
src/client/common/open.ts
559556
src/client/common/platform/serviceRegistry.ts
@@ -611,7 +608,6 @@ src/client/common/dotnet/services/linuxCompatibilityService.ts
611608
src/client/common/dotnet/services/windowsCompatibilityService.ts
612609
src/client/common/types.ts
613610
src/client/common/logger.ts
614-
src/client/common/configSettings.ts
615611
src/client/common/constants.ts
616612
src/client/common/variables/serviceRegistry.ts
617613
src/client/common/variables/environment.ts

‎src/client/common/configSettings.ts‎

Lines changed: 62 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use strict';
22

3+
// eslint-disable-next-line camelcase
34
import * as child_process from 'child_process';
45
import * as path from 'path';
56
import {
@@ -13,7 +14,7 @@ import {
1314
WorkspaceConfiguration,
1415
} from 'vscode';
1516
import { LanguageServerType } from '../activation/types';
16-
import '../common/extensions';
17+
import './extensions';
1718
import { IInterpreterAutoSeletionProxyService, IInterpreterSecurityService } from '../interpreter/autoSelection/types';
1819
import { LogLevel } from '../logging/levels';
1920
import { sendTelemetryEvent } from '../telemetry';
@@ -56,6 +57,7 @@ export class PythonSettings implements IPythonSettings {
5657
public get pythonPath(): string {
5758
return this._pythonPath;
5859
}
60+
5961
public set pythonPath(value: string) {
6062
if (this._pythonPath === value) {
6163
return;
@@ -72,6 +74,7 @@ export class PythonSettings implements IPythonSettings {
7274
public get defaultInterpreterPath(): string {
7375
return this._defaultInterpreterPath;
7476
}
77+
7578
public set defaultInterpreterPath(value: string) {
7679
if (this._defaultInterpreterPath === value) {
7780
return;
@@ -84,41 +87,73 @@ export class PythonSettings implements IPythonSettings {
8487
this._defaultInterpreterPath = value;
8588
}
8689
}
90+
8791
private static pythonSettings: Map<string, PythonSettings> = new Map<string, PythonSettings>();
92+
8893
public showStartPage = true;
94+
8995
public downloadLanguageServer = true;
96+
9097
public jediPath = '';
98+
9199
public jediMemoryLimit = 1024;
100+
92101
public envFile = '';
102+
93103
public venvPath = '';
104+
94105
public venvFolders: string[] = [];
106+
95107
public condaPath = '';
108+
96109
public pipenvPath = '';
110+
97111
public poetryPath = '';
112+
98113
public devOptions: string[] = [];
114+
99115
public linting!: ILintingSettings;
116+
100117
public formatting!: IFormattingSettings;
118+
101119
public autoComplete!: IAutoCompleteSettings;
120+
102121
public testing!: ITestingSettings;
122+
103123
public terminal!: ITerminalSettings;
124+
104125
public sortImports!: ISortImportSettings;
126+
105127
public workspaceSymbols!: IWorkspaceSymbolSettings;
128+
106129
public disableInstallationChecks = false;
130+
107131
public globalModuleInstallation = false;
132+
108133
public analysis!: IAnalysisSettings;
109-
public autoUpdateLanguageServer: boolean = true;
134+
135+
public autoUpdateLanguageServer = true;
136+
110137
public insidersChannel!: ExtensionChannels;
138+
111139
public experiments!: IExperiments;
140+
112141
public languageServer: LanguageServerType = LanguageServerType.Microsoft;
142+
113143
public logging: ILoggingSettings = { level: LogLevel.Error };
114-
public useIsolation: boolean = true;
144+
145+
public useIsolation = true;
115146

116147
protected readonly changed = new EventEmitter<void>();
148+
117149
private workspaceRoot: Resource;
150+
118151
private disposables: Disposable[] = [];
119152

120153
private _pythonPath = '';
154+
121155
private _defaultInterpreterPath = '';
156+
122157
private readonly workspace: IWorkspaceService;
123158

124159
constructor(
@@ -158,7 +193,8 @@ export class PythonSettings implements IPythonSettings {
158193
PythonSettings.pythonSettings.set(workspaceFolderKey, settings);
159194
// Pass null to avoid VSC from complaining about not passing in a value.
160195

161-
const config = workspace.getConfiguration('editor', resource ? resource : (null as any));
196+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
197+
const config = workspace.getConfiguration('editor', resource || (null as any));
162198
const formatOnType = config ? config.get('formatOnType', false) : false;
163199
sendTelemetryEvent(EventName.COMPLETION_ADD_BRACKETS, undefined, {
164200
enabled: settings.autoComplete ? settings.autoComplete.addBrackets : false,
@@ -185,7 +221,7 @@ export class PythonSettings implements IPythonSettings {
185221
return { uri: workspaceFolderUri, target };
186222
}
187223

188-
public static dispose() {
224+
public static dispose(): void {
189225
if (!isTestExecution()) {
190226
throw new Error('Dispose can only be called from unit tests');
191227
}
@@ -195,6 +231,7 @@ export class PythonSettings implements IPythonSettings {
195231
}
196232

197233
public static toSerializable(settings: IPythonSettings): IPythonSettings {
234+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
198235
const clone: any = {};
199236
const keys = Object.entries(settings);
200237
keys.forEach((e) => {
@@ -207,19 +244,19 @@ export class PythonSettings implements IPythonSettings {
207244
return clone as IPythonSettings;
208245
}
209246

210-
public dispose() {
247+
public dispose(): void {
211248
this.disposables.forEach((disposable) => disposable && disposable.dispose());
212249
this.disposables = [];
213250
}
214251

215-
protected update(pythonSettings: WorkspaceConfiguration) {
252+
protected update(pythonSettings: WorkspaceConfiguration): void {
216253
const workspaceRoot = this.workspaceRoot?.fsPath;
217254
const systemVariables: SystemVariables = new SystemVariables(undefined, workspaceRoot, this.workspace);
218255

219256
this.pythonPath = this.getPythonPath(pythonSettings, systemVariables, workspaceRoot);
220257

221258
const defaultInterpreterPath = systemVariables.resolveAny(pythonSettings.get<string>('defaultInterpreterPath'));
222-
this.defaultInterpreterPath = defaultInterpreterPath ? defaultInterpreterPath : DEFAULT_INTERPRETER_SETTING;
259+
this.defaultInterpreterPath = defaultInterpreterPath || DEFAULT_INTERPRETER_SETTING;
223260
this.defaultInterpreterPath = getAbsolutePath(this.defaultInterpreterPath, workspaceRoot);
224261

225262
this.venvPath = systemVariables.resolveAny(pythonSettings.get<string>('venvPath'))!;
@@ -259,9 +296,11 @@ export class PythonSettings implements IPythonSettings {
259296
this.envFile = systemVariables.resolveAny(envFileSetting)!;
260297
sendSettingTelemetry(this.workspace, envFileSetting);
261298

299+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
262300
this.devOptions = systemVariables.resolveAny(pythonSettings.get<any[]>('devOptions'))!;
263301
this.devOptions = Array.isArray(this.devOptions) ? this.devOptions : [];
264302

303+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
265304
const loggingSettings = systemVariables.resolveAny(pythonSettings.get<any>('logging'))!;
266305
loggingSettings.level = convertSettingTypeToLogLevel(loggingSettings.level);
267306
if (this.logging) {
@@ -520,6 +559,8 @@ export class PythonSettings implements IPythonSettings {
520559
} else {
521560
this.experiments = experiments;
522561
}
562+
// Note we directly access experiment settings using workspace service in ExperimentService class.
563+
// Any changes here specific to these settings should propogate their as well.
523564
this.experiments = this.experiments
524565
? this.experiments
525566
: {
@@ -536,11 +577,13 @@ export class PythonSettings implements IPythonSettings {
536577
this.insidersChannel = pythonSettings.get<ExtensionChannels>('insidersChannel')!;
537578
}
538579

539-
protected getPythonExecutable(pythonPath: string) {
580+
// eslint-disable-next-line class-methods-use-this
581+
protected getPythonExecutable(pythonPath: string): string {
540582
return getPythonExecutable(pythonPath);
541583
}
542-
protected onWorkspaceFoldersChanged() {
543-
//If an activated workspace folder was removed, delete its key
584+
585+
protected onWorkspaceFoldersChanged(): void {
586+
// If an activated workspace folder was removed, delete its key
544587
const workspaceKeys = this.workspace.workspaceFolders!.map((workspaceFolder) => workspaceFolder.uri.fsPath);
545588
const activatedWkspcKeys = Array.from(PythonSettings.pythonSettings.keys());
546589
const activatedWkspcFoldersRemoved = activatedWkspcKeys.filter((item) => workspaceKeys.indexOf(item) < 0);
@@ -550,6 +593,7 @@ export class PythonSettings implements IPythonSettings {
550593
}
551594
}
552595
}
596+
553597
protected initialize(): void {
554598
const onDidChange = () => {
555599
const currentConfig = this.workspace.getConfiguration('python', this.workspaceRoot);
@@ -582,8 +626,9 @@ export class PythonSettings implements IPythonSettings {
582626
this.update(initialConfig);
583627
}
584628
}
629+
585630
@debounceSync(1)
586-
protected debounceChangeNotification() {
631+
protected debounceChangeNotification(): void {
587632
this.changed.fire();
588633
}
589634

@@ -625,13 +670,11 @@ export class PythonSettings implements IPythonSettings {
625670
.setWorkspaceInterpreter(this.workspaceRoot, autoSelectedPythonInterpreter)
626671
.ignoreErrors();
627672
}
628-
} else {
629-
if (autoSelectedPythonInterpreter && this.workspaceRoot) {
630-
this.pythonPath = autoSelectedPythonInterpreter.path;
631-
this.interpreterAutoSelectionService
632-
.setWorkspaceInterpreter(this.workspaceRoot, autoSelectedPythonInterpreter)
633-
.ignoreErrors();
634-
}
673+
} else if (autoSelectedPythonInterpreter && this.workspaceRoot) {
674+
this.pythonPath = autoSelectedPythonInterpreter.path;
675+
this.interpreterAutoSelectionService
676+
.setWorkspaceInterpreter(this.workspaceRoot, autoSelectedPythonInterpreter)
677+
.ignoreErrors();
635678
}
636679
}
637680
if (inExperiment && this.pythonPath === DEFAULT_INTERPRETER_SETTING) {

‎src/client/common/experiments/service.ts‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,9 @@ import { Memento } from 'vscode';
88
import { getExperimentationService, IExperimentationService, TargetPopulation } from 'vscode-tas-client';
99
import { sendTelemetryEvent } from '../../telemetry';
1010
import { EventName } from '../../telemetry/constants';
11-
import { IApplicationEnvironment } from '../application/types';
11+
import { IApplicationEnvironment, IWorkspaceService } from '../application/types';
1212
import { PVSC_EXTENSION_ID, STANDARD_OUTPUT_CHANNEL } from '../constants';
13-
import {
14-
GLOBAL_MEMENTO,
15-
IConfigurationService,
16-
IExperimentService,
17-
IMemento,
18-
IOutputChannel,
19-
IPythonSettings,
20-
} from '../types';
13+
import { GLOBAL_MEMENTO, IExperimentService, IMemento, IOutputChannel } from '../types';
2114
import { Experiments } from '../utils/localize';
2215
import { ExperimentationTelemetry } from './telemetry';
2316

@@ -29,30 +22,32 @@ export class ExperimentService implements IExperimentService {
2922
* Experiments the user requested to opt into manually.
3023
*/
3124
public _optInto: string[] = [];
25+
3226
/**
3327
* Experiments the user requested to opt out from manually.
3428
*/
3529
public _optOutFrom: string[] = [];
3630

3731
private readonly experimentationService?: IExperimentationService;
38-
private readonly settings: IPythonSettings;
3932

4033
constructor(
41-
@inject(IConfigurationService) readonly configurationService: IConfigurationService,
34+
@inject(IWorkspaceService) readonly workspaceService: IWorkspaceService,
4235
@inject(IApplicationEnvironment) private readonly appEnvironment: IApplicationEnvironment,
4336
@inject(IMemento) @named(GLOBAL_MEMENTO) private readonly globalState: Memento,
4437
@inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly output: IOutputChannel,
4538
) {
46-
this.settings = configurationService.getSettings(undefined);
47-
39+
const settings = this.workspaceService.getConfiguration('python');
4840
// Users can only opt in or out of experiment groups, not control groups.
49-
const optInto = this.settings.experiments.optInto;
50-
const optOutFrom = this.settings.experiments.optOutFrom;
41+
const optInto = settings.get<string[]>('experiments.optInto') || [];
42+
const optOutFrom = settings.get<string[]>('experiments.optOutFrom') || [];
5143
this._optInto = optInto.filter((exp) => !exp.endsWith('control'));
5244
this._optOutFrom = optOutFrom.filter((exp) => !exp.endsWith('control'));
5345

5446
// Don't initialize the experiment service if the extension's experiments setting is disabled.
55-
const enabled = this.settings.experiments.enabled;
47+
let enabled = settings.get<boolean>('experiments.enabled');
48+
if (enabled === undefined) {
49+
enabled = true;
50+
}
5651
if (!enabled) {
5752
return;
5853
}
@@ -106,7 +101,7 @@ export class ExperimentService implements IExperimentService {
106101

107102
public async getExperimentValue<T extends boolean | number | string>(experiment: string): Promise<T | undefined> {
108103
if (!this.experimentationService || this._optOutFrom.includes('All') || this._optOutFrom.includes(experiment)) {
109-
return;
104+
return undefined;
110105
}
111106

112107
return this.experimentationService.getTreatmentVariableAsync('vscode', experiment);
@@ -121,7 +116,8 @@ export class ExperimentService implements IExperimentService {
121116
// short circuit and return. So, printing out additional experiment info might cause
122117
// confusion. So skip printing out any specific experiment details to the log.
123118
return;
124-
} else if (this._optInto.includes('All')) {
119+
}
120+
if (this._optInto.includes('All')) {
125121
// Only if 'All' is not in optOut then check if it is in Opt In.
126122
this.output.appendLine(Experiments.inGroup().format('All'));
127123

0 commit comments

Comments
 (0)