From 56e16bf51a7b620ba40b20c0ed94db2d1265295b Mon Sep 17 00:00:00 2001 From: ambujsahu81 Date: Sun, 1 Jan 2023 00:39:59 +0530 Subject: [PATCH 1/6] Make Run Tests and Debug Tests default profile in sync --- .../testing/browser/testExplorerActions.ts | 2 +- .../testing/common/testProfileService.ts | 26 ++++++++++++++----- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts b/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts index 711e397c707..be4cb0e46c4 100644 --- a/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts +++ b/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts @@ -243,7 +243,7 @@ export class SelectDefaultTestProfiles extends Action2 { }); if (profiles?.length) { - testProfileService.setGroupDefaultProfiles(onlyGroup, profiles); + testProfileService.setGroupDefaultProfiles(profiles); } } } diff --git a/src/vs/workbench/contrib/testing/common/testProfileService.ts b/src/vs/workbench/contrib/testing/common/testProfileService.ts index ecdb9ffcd63..2f9852243fa 100644 --- a/src/vs/workbench/contrib/testing/common/testProfileService.ts +++ b/src/vs/workbench/contrib/testing/common/testProfileService.ts @@ -66,9 +66,9 @@ export interface ITestProfileService { getGroupDefaultProfiles(group: TestRunProfileBitset): ITestRunProfile[]; /** - * Sets the default profiles to be run for a given run group. + * Sets the default profiles to be run for all run group. */ - setGroupDefaultProfiles(group: TestRunProfileBitset, profiles: ITestRunProfile[]): void; + setGroupDefaultProfiles(profiles: ITestRunProfile[]): void; /** * Gets the profiles for a controller, in priority order. @@ -240,12 +240,24 @@ export class TestProfileService implements ITestProfileService { } /** @inheritdoc */ - public setGroupDefaultProfiles(group: TestRunProfileBitset, profiles: ITestRunProfile[]) { - this.preferredDefaults.store({ - ...this.preferredDefaults.get(), - [group]: profiles.map(c => ({ profileId: c.profileId, controllerId: c.controllerId })), - }); + public setGroupDefaultProfiles(selectedProfiles: ITestRunProfile[]) { + // also include extra profiles apart from selected Profiles for different run group + const defaults: ITestRunProfile[] = []; + for (const { profiles } of this.controllerProfiles.values()) { + const extraProfiles = profiles.filter(p => selectedProfiles.find(s => s.label === p.label)); + for (const profile of extraProfiles) { + if (profile) { + defaults.push(profile); + } + } + } + for (const group of testRunProfileBitsetList) { + this.preferredDefaults.store({ + ...this.preferredDefaults.get(), + [group]: defaults.map(c => ({ profileId: c.profileId, controllerId: c.controllerId })), + }); + } this.changeEmitter.fire(); } From 855edbe7ea0eb515630c8764ec1b5a631777fbdd Mon Sep 17 00:00:00 2001 From: ambujsahu81 Date: Tue, 3 Jan 2023 15:45:36 +0530 Subject: [PATCH 2/6] refactor setDefaultProfiles method --- .../testing/browser/testExplorerActions.ts | 2 +- .../testing/common/testProfileService.ts | 18 ++++++++---------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts b/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts index be4cb0e46c4..ca9bba0cbe8 100644 --- a/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts +++ b/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts @@ -243,7 +243,7 @@ export class SelectDefaultTestProfiles extends Action2 { }); if (profiles?.length) { - testProfileService.setGroupDefaultProfiles(profiles); + testProfileService.setDefaultProfiles(profiles); } } } diff --git a/src/vs/workbench/contrib/testing/common/testProfileService.ts b/src/vs/workbench/contrib/testing/common/testProfileService.ts index 2f9852243fa..106b8c5e52c 100644 --- a/src/vs/workbench/contrib/testing/common/testProfileService.ts +++ b/src/vs/workbench/contrib/testing/common/testProfileService.ts @@ -68,7 +68,7 @@ export interface ITestProfileService { /** * Sets the default profiles to be run for all run group. */ - setGroupDefaultProfiles(profiles: ITestRunProfile[]): void; + setDefaultProfiles(profiles: ITestRunProfile[]): void; /** * Gets the profiles for a controller, in priority order. @@ -240,22 +240,20 @@ export class TestProfileService implements ITestProfileService { } /** @inheritdoc */ - public setGroupDefaultProfiles(selectedProfiles: ITestRunProfile[]) { + public setDefaultProfiles(selectedDefaultProfiles: ITestRunProfile[]) { - // also include extra profiles apart from selected Profiles for different run group - const defaults: ITestRunProfile[] = []; + // Include profiles for different run group along with default selected profiles + const defaultProfiles: ITestRunProfile[] = []; for (const { profiles } of this.controllerProfiles.values()) { - const extraProfiles = profiles.filter(p => selectedProfiles.find(s => s.label === p.label)); - for (const profile of extraProfiles) { - if (profile) { - defaults.push(profile); - } + const profilesForAllRunGroup = profiles.filter(p => selectedDefaultProfiles.find(s => s.label === p.label)); + for (const profile of profilesForAllRunGroup) { + defaultProfiles.push(profile); } } for (const group of testRunProfileBitsetList) { this.preferredDefaults.store({ ...this.preferredDefaults.get(), - [group]: defaults.map(c => ({ profileId: c.profileId, controllerId: c.controllerId })), + [group]: defaultProfiles.map(c => ({ profileId: c.profileId, controllerId: c.controllerId })), }); } this.changeEmitter.fire(); From 0cf191a18aebd9c305493f1f81d73cf9036508e7 Mon Sep 17 00:00:00 2001 From: ambujsahu81 Date: Thu, 5 Jan 2023 23:34:26 +0530 Subject: [PATCH 3/6] only make run tests and debug tests default profile in sync --- .../testing/browser/testExplorerActions.ts | 2 +- .../testing/common/testProfileService.ts | 32 ++++++++++--------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts b/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts index ca9bba0cbe8..711e397c707 100644 --- a/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts +++ b/src/vs/workbench/contrib/testing/browser/testExplorerActions.ts @@ -243,7 +243,7 @@ export class SelectDefaultTestProfiles extends Action2 { }); if (profiles?.length) { - testProfileService.setDefaultProfiles(profiles); + testProfileService.setGroupDefaultProfiles(onlyGroup, profiles); } } } diff --git a/src/vs/workbench/contrib/testing/common/testProfileService.ts b/src/vs/workbench/contrib/testing/common/testProfileService.ts index 106b8c5e52c..f7d82cd7908 100644 --- a/src/vs/workbench/contrib/testing/common/testProfileService.ts +++ b/src/vs/workbench/contrib/testing/common/testProfileService.ts @@ -66,9 +66,9 @@ export interface ITestProfileService { getGroupDefaultProfiles(group: TestRunProfileBitset): ITestRunProfile[]; /** - * Sets the default profiles to be run for all run group. + * Sets the default profiles to be run for a given run group. */ - setDefaultProfiles(profiles: ITestRunProfile[]): void; + setGroupDefaultProfiles(group: TestRunProfileBitset, profiles: ITestRunProfile[]): void; /** * Gets the profiles for a controller, in priority order. @@ -240,22 +240,24 @@ export class TestProfileService implements ITestProfileService { } /** @inheritdoc */ - public setDefaultProfiles(selectedDefaultProfiles: ITestRunProfile[]) { + public setGroupDefaultProfiles(group: TestRunProfileBitset, profiles: ITestRunProfile[]) { - // Include profiles for different run group along with default selected profiles - const defaultProfiles: ITestRunProfile[] = []; - for (const { profiles } of this.controllerProfiles.values()) { - const profilesForAllRunGroup = profiles.filter(p => selectedDefaultProfiles.find(s => s.label === p.label)); - for (const profile of profilesForAllRunGroup) { - defaultProfiles.push(profile); + if (group === TestRunProfileBitset.Run || group === TestRunProfileBitset.Debug) { + const runGroup = group === TestRunProfileBitset.Run ? TestRunProfileBitset.Debug : TestRunProfileBitset.Run; + const defaultProfiles = profiles.map(p => this.controllerProfiles.get(p.controllerId)?.profiles.find(c => c.label === p.label && c.group === runGroup)).filter(isDefined); + if (defaultProfiles?.length) { + this.preferredDefaults.store({ + ...this.preferredDefaults.get(), + [runGroup]: defaultProfiles.map(c => ({ profileId: c.profileId, controllerId: c.controllerId })), + }); } } - for (const group of testRunProfileBitsetList) { - this.preferredDefaults.store({ - ...this.preferredDefaults.get(), - [group]: defaultProfiles.map(c => ({ profileId: c.profileId, controllerId: c.controllerId })), - }); - } + + this.preferredDefaults.store({ + ...this.preferredDefaults.get(), + [group]: profiles.map(c => ({ profileId: c.profileId, controllerId: c.controllerId })), + }); + this.changeEmitter.fire(); } From db0eda2acc42d3f0402857778f52bec6fe93f8ca Mon Sep 17 00:00:00 2001 From: ambujsahu81 Date: Fri, 6 Jan 2023 16:34:51 +0530 Subject: [PATCH 4/6] Avoid making changes to defaultsProfiles config of different controllers --- .../contrib/testing/common/testProfileService.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/vs/workbench/contrib/testing/common/testProfileService.ts b/src/vs/workbench/contrib/testing/common/testProfileService.ts index f7d82cd7908..9d0e3addfb1 100644 --- a/src/vs/workbench/contrib/testing/common/testProfileService.ts +++ b/src/vs/workbench/contrib/testing/common/testProfileService.ts @@ -246,6 +246,17 @@ export class TestProfileService implements ITestProfileService { const runGroup = group === TestRunProfileBitset.Run ? TestRunProfileBitset.Debug : TestRunProfileBitset.Run; const defaultProfiles = profiles.map(p => this.controllerProfiles.get(p.controllerId)?.profiles.find(c => c.label === p.label && c.group === runGroup)).filter(isDefined); if (defaultProfiles?.length) { + const preferred = this.preferredDefaults.get(); + if (preferred && preferred[runGroup]) { + const previousDefaultsProfiles = preferred[runGroup]?.map(p => this.controllerProfiles.get(p.controllerId)?.profiles.find(c => c.profileId === p.profileId && c.group === runGroup)).filter(isDefined); + if (previousDefaultsProfiles?.length) { + for (const profile of previousDefaultsProfiles) { + if (!profiles.find(p => p.controllerId === profile.controllerId)) { + defaultProfiles.push(profile); + } + } + } + } this.preferredDefaults.store({ ...this.preferredDefaults.get(), [runGroup]: defaultProfiles.map(c => ({ profileId: c.profileId, controllerId: c.controllerId })), From d2b459ad7c1ca0827f86d3402ba9b3cb9a7dde74 Mon Sep 17 00:00:00 2001 From: ambujsahu81 Date: Fri, 6 Jan 2023 18:03:39 +0530 Subject: [PATCH 5/6] small correction to preserve previousDefault profile --- .../contrib/testing/common/testProfileService.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/vs/workbench/contrib/testing/common/testProfileService.ts b/src/vs/workbench/contrib/testing/common/testProfileService.ts index 9d0e3addfb1..ebde950165c 100644 --- a/src/vs/workbench/contrib/testing/common/testProfileService.ts +++ b/src/vs/workbench/contrib/testing/common/testProfileService.ts @@ -244,14 +244,16 @@ export class TestProfileService implements ITestProfileService { if (group === TestRunProfileBitset.Run || group === TestRunProfileBitset.Debug) { const runGroup = group === TestRunProfileBitset.Run ? TestRunProfileBitset.Debug : TestRunProfileBitset.Run; - const defaultProfiles = profiles.map(p => this.controllerProfiles.get(p.controllerId)?.profiles.find(c => c.label === p.label && c.group === runGroup)).filter(isDefined); - if (defaultProfiles?.length) { + const defaultProfiles: ITestRunProfile[] = []; + const newProfiles = profiles.map(p => this.controllerProfiles.get(p.controllerId)?.profiles.find(c => c.label === p.label && c.group === runGroup)).filter(isDefined); + if (newProfiles?.length) { + newProfiles.forEach(profile => defaultProfiles.push(profile)); const preferred = this.preferredDefaults.get(); if (preferred && preferred[runGroup]) { const previousDefaultsProfiles = preferred[runGroup]?.map(p => this.controllerProfiles.get(p.controllerId)?.profiles.find(c => c.profileId === p.profileId && c.group === runGroup)).filter(isDefined); if (previousDefaultsProfiles?.length) { for (const profile of previousDefaultsProfiles) { - if (!profiles.find(p => p.controllerId === profile.controllerId)) { + if (!newProfiles.find(p => p.controllerId === profile.controllerId)) { defaultProfiles.push(profile); } } From 83b9c9a90ca6600664c54bccdc7d5ae174fc3df9 Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Fri, 6 Jan 2023 10:26:31 -0800 Subject: [PATCH 6/6] add tests --- .../testing/common/testProfileService.ts | 49 ++++---- .../test/common/testProfileService.test.ts | 117 ++++++++++++++++++ 2 files changed, 139 insertions(+), 27 deletions(-) create mode 100644 src/vs/workbench/contrib/testing/test/common/testProfileService.test.ts diff --git a/src/vs/workbench/contrib/testing/common/testProfileService.ts b/src/vs/workbench/contrib/testing/common/testProfileService.ts index ebde950165c..60d5972e130 100644 --- a/src/vs/workbench/contrib/testing/common/testProfileService.ts +++ b/src/vs/workbench/contrib/testing/common/testProfileService.ts @@ -241,36 +241,31 @@ export class TestProfileService implements ITestProfileService { /** @inheritdoc */ public setGroupDefaultProfiles(group: TestRunProfileBitset, profiles: ITestRunProfile[]) { - - if (group === TestRunProfileBitset.Run || group === TestRunProfileBitset.Debug) { - const runGroup = group === TestRunProfileBitset.Run ? TestRunProfileBitset.Debug : TestRunProfileBitset.Run; - const defaultProfiles: ITestRunProfile[] = []; - const newProfiles = profiles.map(p => this.controllerProfiles.get(p.controllerId)?.profiles.find(c => c.label === p.label && c.group === runGroup)).filter(isDefined); - if (newProfiles?.length) { - newProfiles.forEach(profile => defaultProfiles.push(profile)); - const preferred = this.preferredDefaults.get(); - if (preferred && preferred[runGroup]) { - const previousDefaultsProfiles = preferred[runGroup]?.map(p => this.controllerProfiles.get(p.controllerId)?.profiles.find(c => c.profileId === p.profileId && c.group === runGroup)).filter(isDefined); - if (previousDefaultsProfiles?.length) { - for (const profile of previousDefaultsProfiles) { - if (!newProfiles.find(p => p.controllerId === profile.controllerId)) { - defaultProfiles.push(profile); - } - } - } - } - this.preferredDefaults.store({ - ...this.preferredDefaults.get(), - [runGroup]: defaultProfiles.map(c => ({ profileId: c.profileId, controllerId: c.controllerId })), - }); - } - } - - this.preferredDefaults.store({ + const next = { ...this.preferredDefaults.get(), [group]: profiles.map(c => ({ profileId: c.profileId, controllerId: c.controllerId })), - }); + }; + // When switching a run/debug profile, if the controller has a same-named + // profile in the other group, use that instead of anything else that was selected. + if (group === TestRunProfileBitset.Run || group === TestRunProfileBitset.Debug) { + const otherGroup = group === TestRunProfileBitset.Run ? TestRunProfileBitset.Debug : TestRunProfileBitset.Run; + + const previousDefaults = next[otherGroup] || []; + let newDefaults = previousDefaults.slice(); + for (const [ctrlId, { profiles: ctrlProfiles }] of this.controllerProfiles) { + const labels = new Set(profiles.filter(p => p.controllerId === ctrlId).map(p => p.label)); + const nextByLabels = ctrlProfiles.filter(p => labels.has(p.label) && p.group === otherGroup); + if (nextByLabels.length) { + newDefaults = newDefaults.filter(p => p.controllerId !== ctrlId); + newDefaults.push(...nextByLabels.map(p => ({ profileId: p.profileId, controllerId: p.controllerId }))); + } + } + + next[otherGroup] = newDefaults; + } + + this.preferredDefaults.store(next); this.changeEmitter.fire(); } diff --git a/src/vs/workbench/contrib/testing/test/common/testProfileService.test.ts b/src/vs/workbench/contrib/testing/test/common/testProfileService.test.ts new file mode 100644 index 00000000000..f12f301beae --- /dev/null +++ b/src/vs/workbench/contrib/testing/test/common/testProfileService.test.ts @@ -0,0 +1,117 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + + + +import * as assert from 'assert'; +import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKeybindingService'; +import { TestProfileService } from 'vs/workbench/contrib/testing/common/testProfileService'; +import { ITestRunProfile, TestRunProfileBitset } from 'vs/workbench/contrib/testing/common/testTypes'; +import { TestStorageService } from 'vs/workbench/test/common/workbenchTestServices'; + +suite('Workbench - TestProfileService', () => { + let t: TestProfileService; + let idCounter = 0; + setup(() => { + idCounter = 0; + t = new TestProfileService( + new MockContextKeyService(), + new TestStorageService(), + ); + }); + + const addProfile = (profile: Partial) => { + const p: ITestRunProfile = { + controllerId: 'ctrlId', + group: TestRunProfileBitset.Run, + isDefault: true, + label: 'profile', + profileId: idCounter++, + hasConfigurationHandler: false, + tag: null, + ...profile, + }; + + t.addProfile(null as any, p); + return p; + }; + + const expectProfiles = (expected: ITestRunProfile[], actual: string[]) => { + const e = expected.map(e => e.label).sort(); + const a = actual.sort(); + assert.deepStrictEqual(e, a); + }; + + test('getGroupDefaultProfiles', () => { + addProfile({ isDefault: true, group: TestRunProfileBitset.Debug, label: 'a' }); + addProfile({ isDefault: false, group: TestRunProfileBitset.Debug, label: 'b' }); + addProfile({ isDefault: true, group: TestRunProfileBitset.Run, label: 'c' }); + addProfile({ isDefault: true, group: TestRunProfileBitset.Run, label: 'd', controllerId: '2' }); + addProfile({ isDefault: false, group: TestRunProfileBitset.Run, label: 'e', controllerId: '2' }); + expectProfiles(t.getGroupDefaultProfiles(TestRunProfileBitset.Run), ['c', 'd']); + expectProfiles(t.getGroupDefaultProfiles(TestRunProfileBitset.Debug), ['a']); + }); + + suite('setGroupDefaultProfiles', () => { + test('applies simple changes', () => { + const p1 = addProfile({ isDefault: false, group: TestRunProfileBitset.Debug, label: 'a' }); + addProfile({ isDefault: false, group: TestRunProfileBitset.Debug, label: 'b' }); + const p3 = addProfile({ isDefault: false, group: TestRunProfileBitset.Run, label: 'c' }); + addProfile({ isDefault: false, group: TestRunProfileBitset.Run, label: 'd' }); + + t.setGroupDefaultProfiles(TestRunProfileBitset.Run, [p3]); + assert.deepStrictEqual(t.getGroupDefaultProfiles(TestRunProfileBitset.Run), [p3]); + assert.deepStrictEqual(t.getGroupDefaultProfiles(TestRunProfileBitset.Debug), [p1]); + }); + + test('syncs labels if same', () => { + const p1 = addProfile({ isDefault: false, group: TestRunProfileBitset.Debug, label: 'a' }); + const p2 = addProfile({ isDefault: false, group: TestRunProfileBitset.Debug, label: 'b' }); + const p3 = addProfile({ isDefault: false, group: TestRunProfileBitset.Run, label: 'a' }); + const p4 = addProfile({ isDefault: false, group: TestRunProfileBitset.Run, label: 'b' }); + + t.setGroupDefaultProfiles(TestRunProfileBitset.Run, [p3]); + assert.deepStrictEqual(t.getGroupDefaultProfiles(TestRunProfileBitset.Run), [p3]); + assert.deepStrictEqual(t.getGroupDefaultProfiles(TestRunProfileBitset.Debug), [p1]); + + t.setGroupDefaultProfiles(TestRunProfileBitset.Debug, [p2]); + assert.deepStrictEqual(t.getGroupDefaultProfiles(TestRunProfileBitset.Run), [p4]); + assert.deepStrictEqual(t.getGroupDefaultProfiles(TestRunProfileBitset.Debug), [p2]); + }); + + test('does not mess up sync for multiple controllers', () => { + // ctrl a and b both of have their own labels. ctrl c does not and should be unaffected + const p1 = addProfile({ isDefault: false, controllerId: 'a', group: TestRunProfileBitset.Debug, label: 'a' }); + const p2 = addProfile({ isDefault: false, controllerId: 'b', group: TestRunProfileBitset.Debug, label: 'b1' }); + const p3 = addProfile({ isDefault: false, controllerId: 'b', group: TestRunProfileBitset.Debug, label: 'b2' }); + const p4 = addProfile({ isDefault: false, controllerId: 'c', group: TestRunProfileBitset.Debug, label: 'c1' }); + + const p5 = addProfile({ isDefault: false, controllerId: 'a', group: TestRunProfileBitset.Run, label: 'a' }); + const p6 = addProfile({ isDefault: false, controllerId: 'b', group: TestRunProfileBitset.Run, label: 'b1' }); + const p7 = addProfile({ isDefault: false, controllerId: 'b', group: TestRunProfileBitset.Run, label: 'b2' }); + const p8 = addProfile({ isDefault: false, controllerId: 'b', group: TestRunProfileBitset.Run, label: 'b3' }); + + // same profile on both + t.setGroupDefaultProfiles(TestRunProfileBitset.Debug, [p3]); + assert.deepStrictEqual(t.getGroupDefaultProfiles(TestRunProfileBitset.Run), [p7]); + assert.deepStrictEqual(t.getGroupDefaultProfiles(TestRunProfileBitset.Debug), [p3]); + + // different profile, other should be unaffected + t.setGroupDefaultProfiles(TestRunProfileBitset.Run, [p8]); + assert.deepStrictEqual(t.getGroupDefaultProfiles(TestRunProfileBitset.Run), [p8]); + assert.deepStrictEqual(t.getGroupDefaultProfiles(TestRunProfileBitset.Debug), [p3]); + + // multiple changes in one go, with unmatched c + t.setGroupDefaultProfiles(TestRunProfileBitset.Debug, [p1, p2, p4]); + assert.deepStrictEqual(t.getGroupDefaultProfiles(TestRunProfileBitset.Run), [p5, p6]); + assert.deepStrictEqual(t.getGroupDefaultProfiles(TestRunProfileBitset.Debug), [p1, p2, p4]); + + // identity + t.setGroupDefaultProfiles(TestRunProfileBitset.Run, [p5, p8]); + assert.deepStrictEqual(t.getGroupDefaultProfiles(TestRunProfileBitset.Run), [p5, p8]); + assert.deepStrictEqual(t.getGroupDefaultProfiles(TestRunProfileBitset.Debug), [p2, p4, p1]); + }); + }); +});