Skip to content

Commit 0bc0e38

Browse files
authored
Use the experiment service framework for the linter prompt experiment (#14858)
* Use Experiment Service * Add 'deprecated' on Experiments Manager
1 parent 7ea2af7 commit 0bc0e38

File tree

4 files changed

+27
-23
lines changed

4 files changed

+27
-23
lines changed

src/client/common/experiments/manager.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ export const EXPERIMENTS_EFFORT_TIMEOUT_MS = 2000;
4444
export const oldExperimentSalts = ['ShowExtensionSurveyPrompt', 'ShowPlayIcon', 'AlwaysDisplayTestExplorer', 'LS'];
4545

4646
/**
47-
* Manages and stores experiments, implements the AB testing functionality
47+
* <DEPRECATED> Manages and stores experiments, implements the AB testing functionality
48+
* @deprecated
4849
*/
4950
@injectable()
5051
export class ExperimentsManager implements IExperimentsManager {

src/client/common/installer/productInstaller.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { IProcessServiceFactory, IPythonExecutionFactory } from '../process/type
1919
import { ITerminalServiceFactory } from '../terminal/types';
2020
import {
2121
IConfigurationService,
22-
IExperimentsManager,
22+
IExperimentService,
2323
IInstaller,
2424
InstallerResponse,
2525
IOutputChannel,
@@ -238,12 +238,12 @@ export class LinterInstaller extends BaseInstaller {
238238
// we immediately show this prompt again saying install flake8, while the
239239
// installation is on going.
240240
private static promptSeen: boolean = false;
241-
private readonly experimentsManager: IExperimentsManager;
241+
private readonly experimentsManager: IExperimentService;
242242
private readonly linterManager: ILinterManager;
243243

244244
constructor(protected serviceContainer: IServiceContainer, protected outputChannel: OutputChannel) {
245245
super(serviceContainer, outputChannel);
246-
this.experimentsManager = serviceContainer.get<IExperimentsManager>(IExperimentsManager);
246+
this.experimentsManager = serviceContainer.get<IExperimentService>(IExperimentService);
247247
this.linterManager = serviceContainer.get<ILinterManager>(ILinterManager);
248248
}
249249

@@ -273,7 +273,7 @@ export class LinterInstaller extends BaseInstaller {
273273
// 2. The default linter should be pylint
274274

275275
if (!this.isLinterSetInAnyScope() && product === Product.pylint) {
276-
if (this.experimentsManager.inExperiment(LinterInstallationPromptVariants.noPrompt)) {
276+
if (await this.experimentsManager.inExperiment(LinterInstallationPromptVariants.noPrompt)) {
277277
// We won't show a prompt, so tell the extension to treat as though user
278278
// ignored the prompt.
279279
sendTelemetryEvent(EventName.LINTER_INSTALL_PROMPT, undefined, {
@@ -284,9 +284,9 @@ export class LinterInstaller extends BaseInstaller {
284284
traceInfo(`Linter ${productName} is not installed.`);
285285

286286
return InstallerResponse.Ignore;
287-
} else if (this.experimentsManager.inExperiment(LinterInstallationPromptVariants.pylintFirst)) {
287+
} else if (await this.experimentsManager.inExperiment(LinterInstallationPromptVariants.pylintFirst)) {
288288
return this.newPromptForInstallation(true, resource, cancel);
289-
} else if (this.experimentsManager.inExperiment(LinterInstallationPromptVariants.flake8First)) {
289+
} else if (await this.experimentsManager.inExperiment(LinterInstallationPromptVariants.flake8First)) {
290290
return this.newPromptForInstallation(false, resource, cancel);
291291
}
292292
}

src/client/common/types.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -543,6 +543,9 @@ export type ABExperiments = {
543543
* Interface used to implement AB testing
544544
*/
545545
export const IExperimentsManager = Symbol('IExperimentsManager');
546+
/**
547+
* @deprecated Use IExperimentService instead
548+
*/
546549
export interface IExperimentsManager {
547550
/**
548551
* Checks if experiments are enabled, sets required environment to be used for the experiments, logs experiment groups

src/test/common/installer/installer.unit.test.ts

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { WorkspaceService } from '../../../client/common/application/workspace';
1616
import { ConfigurationService } from '../../../client/common/configuration/service';
1717
import { Commands } from '../../../client/common/constants';
1818
import { LinterInstallationPromptVariants } from '../../../client/common/experiments/groups';
19-
import { ExperimentsManager } from '../../../client/common/experiments/manager';
19+
import { ExperimentService } from '../../../client/common/experiments/service';
2020
import '../../../client/common/extensions';
2121
import {
2222
CTagsInstallationScript,
@@ -46,7 +46,7 @@ import { ITerminalService, ITerminalServiceFactory } from '../../../client/commo
4646
import {
4747
IConfigurationService,
4848
IDisposableRegistry,
49-
IExperimentsManager,
49+
IExperimentService,
5050
InstallerResponse,
5151
IOutputChannel,
5252
IPersistentState,
@@ -980,7 +980,7 @@ suite('Module Installer only', () => {
980980
let workspaceService: IWorkspaceService;
981981
let productService: IProductService;
982982
let cmdManager: ICommandManager;
983-
let experimentsManager: IExperimentsManager;
983+
let experimentsService: IExperimentService;
984984
let linterManager: ILinterManager;
985985
let serviceContainer: IServiceContainer;
986986
let productPathService: IProductPathService;
@@ -992,7 +992,7 @@ suite('Module Installer only', () => {
992992
workspaceService = mock(WorkspaceService);
993993
productService = mock(ProductService);
994994
cmdManager = mock(CommandManager);
995-
experimentsManager = mock(ExperimentsManager);
995+
experimentsService = mock(ExperimentService);
996996
linterManager = mock(LinterManager);
997997
productPathService = mock(LinterProductPathService);
998998
outputChannel = TypeMoq.Mock.ofType<IOutputChannel>();
@@ -1005,9 +1005,9 @@ suite('Module Installer only', () => {
10051005
when(serviceContainer.get<IProductService>(IProductService)).thenReturn(instance(productService));
10061006
when(serviceContainer.get<ICommandManager>(ICommandManager)).thenReturn(instance(cmdManager));
10071007

1008-
const exp = instance(experimentsManager);
1009-
when(serviceContainer.get<IExperimentsManager>(IExperimentsManager)).thenReturn(exp);
1010-
when(experimentsManager.inExperiment(anything())).thenReturn(false);
1008+
const exp = instance(experimentsService);
1009+
when(serviceContainer.get<IExperimentService>(IExperimentService)).thenReturn(exp);
1010+
when(experimentsService.inExperiment(anything())).thenResolve(false);
10111011

10121012
when(serviceContainer.get<ILinterManager>(ILinterManager)).thenReturn(instance(linterManager));
10131013
when(serviceContainer.get<IProductPathService>(IProductPathService, ProductType.Linter)).thenReturn(
@@ -1084,7 +1084,7 @@ suite('Module Installer only', () => {
10841084

10851085
await installer.promptToInstallImplementation(product, resource);
10861086

1087-
verify(experimentsManager.inExperiment(LinterInstallationPromptVariants.noPrompt)).never();
1087+
verify(experimentsService.inExperiment(LinterInstallationPromptVariants.noPrompt)).never();
10881088
verify(
10891089
appShell.showErrorMessage(`Linter ${productName} is not installed.`, 'Install', options[0], options[1])
10901090
).once();
@@ -1102,14 +1102,14 @@ suite('Module Installer only', () => {
11021102
when(productPathService.getExecutableNameFromSettings(Product.pylint, resource)).thenReturn(
11031103
'path/to/something'
11041104
);
1105-
when(experimentsManager.inExperiment(LinterInstallationPromptVariants.flake8First)).thenReturn(true);
1105+
when(experimentsService.inExperiment(LinterInstallationPromptVariants.flake8First)).thenResolve(true);
11061106
installer.isModuleExecutable = false;
11071107

11081108
const product = Product.pylint;
11091109
const options = ['Select Linter', 'Do not show again'];
11101110
const productName = ProductNames.get(product)!;
11111111
await installer.promptToInstallImplementation(product, resource);
1112-
verify(experimentsManager.inExperiment(LinterInstallationPromptVariants.flake8First)).once();
1112+
verify(experimentsService.inExperiment(LinterInstallationPromptVariants.flake8First)).once();
11131113
verify(
11141114
appShell.showInformationMessage(
11151115
Linters.installMessage(),
@@ -1134,11 +1134,11 @@ suite('Module Installer only', () => {
11341134
const product = Product.pylint;
11351135
const options = ['Select Linter', 'Do not show again'];
11361136
const productName = ProductNames.get(product)!;
1137-
when(experimentsManager.inExperiment(LinterInstallationPromptVariants.noPrompt)).thenReturn(true);
1137+
when(experimentsService.inExperiment(LinterInstallationPromptVariants.noPrompt)).thenResolve(true);
11381138

11391139
const response = await installer.promptToInstallImplementation(product, resource);
11401140

1141-
verify(experimentsManager.inExperiment(LinterInstallationPromptVariants.noPrompt)).once();
1141+
verify(experimentsService.inExperiment(LinterInstallationPromptVariants.noPrompt)).once();
11421142
verify(
11431143
appShell.showErrorMessage(`Linter ${productName} is not installed.`, 'Install', options[0], options[1])
11441144
).never();
@@ -1147,11 +1147,11 @@ suite('Module Installer only', () => {
11471147

11481148
test('pylint first Experiment: Linter should install pylint first and install flake8 next', async () => {
11491149
const product = Product.pylint;
1150-
when(experimentsManager.inExperiment(LinterInstallationPromptVariants.pylintFirst)).thenReturn(true);
1150+
when(experimentsService.inExperiment(LinterInstallationPromptVariants.pylintFirst)).thenResolve(true);
11511151

11521152
await installer.promptToInstallImplementation(product, resource);
11531153

1154-
verify(experimentsManager.inExperiment(LinterInstallationPromptVariants.pylintFirst)).once();
1154+
verify(experimentsService.inExperiment(LinterInstallationPromptVariants.pylintFirst)).once();
11551155
verify(
11561156
appShell.showInformationMessage(
11571157
Linters.installMessage(),
@@ -1164,11 +1164,11 @@ suite('Module Installer only', () => {
11641164

11651165
test('flake8 first Experiment: Linter should install flake8 first and install pylint next', async () => {
11661166
const product = Product.pylint;
1167-
when(experimentsManager.inExperiment(LinterInstallationPromptVariants.flake8First)).thenReturn(true);
1167+
when(experimentsService.inExperiment(LinterInstallationPromptVariants.flake8First)).thenResolve(true);
11681168

11691169
await installer.promptToInstallImplementation(product, resource);
11701170

1171-
verify(experimentsManager.inExperiment(LinterInstallationPromptVariants.flake8First)).once();
1171+
verify(experimentsService.inExperiment(LinterInstallationPromptVariants.flake8First)).once();
11721172
verify(
11731173
appShell.showInformationMessage(
11741174
Linters.installMessage(),

0 commit comments

Comments
 (0)