Skip to content

Commit

Permalink
fix(state): use workspace state instead of the settings
Browse files Browse the repository at this point in the history
  • Loading branch information
pavelfeldman committed May 14, 2024
1 parent bf17181 commit 104d278
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 35 deletions.
2 changes: 1 addition & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"runtimeExecutable": "${execPath}",
"args": [
"--extensionDevelopmentPath=${workspaceFolder}",
"${workspaceFolder}/../playwright/packages/html-reporter"
"${workspaceFolder}/../playwright/examples/todomvc"
],
"outFiles": [
"${workspaceFolder}/out/**/*.js"
Expand Down
4 changes: 0 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,6 @@
"type": "boolean",
"default": false,
"description": "%configuration.playwright.showTrace%"
},
"playwright.workspaceSettings": {
"type": "object",
"default": {}
}
}
},
Expand Down
14 changes: 8 additions & 6 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,12 @@ type StepInfo = {

export async function activate(context: vscodeTypes.ExtensionContext) {
// Do not await, quickly run the extension, schedule work.
new Extension(require('vscode')).activate(context);
new Extension(require('vscode'), context).activate();
}

export class Extension implements RunHooks {
private _vscode: vscodeTypes.VSCode;
private _context: vscodeTypes.ExtensionContext;
private _disposables: vscodeTypes.Disposable[] = [];

// Global test item map.
Expand Down Expand Up @@ -77,8 +78,9 @@ export class Extension implements RunHooks {
private _commandQueue = Promise.resolve();
overridePlaywrightVersion: number | null = null;

constructor(vscode: vscodeTypes.VSCode) {
constructor(vscode: vscodeTypes.VSCode, context: vscodeTypes.ExtensionContext) {
this._vscode = vscode;
this._context = context;
this._isUnderTest = !!(this._vscode as any).isUnderTest;
this._activeStepDecorationType = this._vscode.window.createTextEditorDecorationType({
isWholeLine: true,
Expand All @@ -98,7 +100,7 @@ export class Extension implements RunHooks {
});

this._settingsModel = new SettingsModel(vscode);
this._models = new TestModelCollection(vscode, this._settingsModel);
this._models = new TestModelCollection(vscode, context);
this._reusedBrowser = new ReusedBrowser(this._vscode, this._settingsModel, this._envProvider.bind(this));
this._traceViewer = new TraceViewer(this._vscode, this._settingsModel, this._envProvider.bind(this));
this._testController = vscode.tests.createTestController('playwright', 'Playwright');
Expand Down Expand Up @@ -135,9 +137,9 @@ export class Extension implements RunHooks {
d?.dispose?.();
}

async activate(context: vscodeTypes.ExtensionContext) {
async activate() {
const vscode = this._vscode;
this._settingsView = new SettingsView(vscode, this._settingsModel, this._models, this._reusedBrowser, context.extensionUri);
this._settingsView = new SettingsView(vscode, this._settingsModel, this._models, this._reusedBrowser, this._context.extensionUri);
const messageNoPlaywrightTestsFound = this._vscode.l10n.t('No Playwright tests found.');
this._disposables = [
this._debugHighlight,
Expand Down Expand Up @@ -248,7 +250,7 @@ export class Extension implements RunHooks {
fileSystemWatchers.map(w => w.onDidChange(rebuildModelForConfig));
fileSystemWatchers.map(w => w.onDidCreate(rebuildModelForConfig));
fileSystemWatchers.map(w => w.onDidDelete(rebuildModelForConfig));
context.subscriptions.push(this);
this._context.subscriptions.push(this);
}

private async _rebuildModels(userGesture: boolean): Promise<vscodeTypes.Uri[]> {
Expand Down
16 changes: 3 additions & 13 deletions src/settingsModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ export class SettingsModel extends DisposableBase {
private _onChange: vscodeTypes.EventEmitter<void>;
showBrowser: Setting<boolean>;
showTrace: Setting<boolean>;
workspaceSettings: Setting<WorkspaceSettings>;

constructor(vscode: vscodeTypes.VSCode) {
super();
Expand All @@ -50,7 +49,6 @@ export class SettingsModel extends DisposableBase {

this.showBrowser = this._createSetting('reuseBrowser');
this.showTrace = this._createSetting('showTrace');
this.workspaceSettings = this._createSetting('workspaceSettings', true);

this.showBrowser.onChange(enabled => {
if (enabled && this.showTrace.get())
Expand All @@ -62,8 +60,8 @@ export class SettingsModel extends DisposableBase {
});
}

private _createSetting<T>(settingName: string, perWorkspace = false): Setting<T> {
const setting = new Setting<T>(this._vscode, settingName, perWorkspace);
private _createSetting<T>(settingName: string): Setting<T> {
const setting = new Setting<T>(this._vscode, settingName);
this._disposables.push(setting);
this._disposables.push(setting.onChange(() => this._onChange.fire()));
this._settings.set(settingName, setting);
Expand All @@ -83,13 +81,11 @@ export class Setting<T> extends DisposableBase {
readonly onChange: vscodeTypes.Event<T>;
private _onChange: vscodeTypes.EventEmitter<T>;
private _vscode: vscodeTypes.VSCode;
private _perWorkspace: boolean;

constructor(vscode: vscodeTypes.VSCode, settingName: string, perWorkspace: boolean) {
constructor(vscode: vscodeTypes.VSCode, settingName: string) {
super();
this._vscode = vscode;
this.settingName = settingName;
this._perWorkspace = perWorkspace;
this._onChange = new vscode.EventEmitter<T>();
this.onChange = this._onChange.event;

Expand All @@ -112,12 +108,6 @@ export class Setting<T> extends DisposableBase {

async set(value: T) {
const configuration = this._vscode.workspace.getConfiguration('playwright');
if (this._perWorkspace) {
// These are not dispatched :shrug:
configuration.update(this.settingName, value, false);
this._onChange.fire(value);
return;
}
const existsInWorkspace = configuration.inspect(this.settingName)?.workspaceValue !== undefined;
if (existsInWorkspace)
configuration.update(this.settingName, value, false);
Expand Down
14 changes: 8 additions & 6 deletions src/testModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -588,11 +588,11 @@ export class TestModelCollection extends DisposableBase {
private _selectedConfigFile: string | undefined;
private _didUpdate: vscodeTypes.EventEmitter<void>;
readonly onUpdated: vscodeTypes.Event<void>;
private _settingsModel: SettingsModel;
private _context: vscodeTypes.ExtensionContext;

constructor(vscode: vscodeTypes.VSCode, settingsModel: SettingsModel) {
constructor(vscode: vscodeTypes.VSCode, context: vscodeTypes.ExtensionContext) {
super();
this._settingsModel = settingsModel;
this._context = context;
this._didUpdate = new vscode.EventEmitter();
this.onUpdated = this._didUpdate.event;
}
Expand Down Expand Up @@ -635,7 +635,7 @@ export class TestModelCollection extends DisposableBase {

async addModel(model: TestModel) {
this._models.push(model);
const workspaceSettings = this._settingsModel.workspaceSettings.get();
const workspaceSettings = this._context.workspaceState.get(workspaceStateKey) as WorkspaceSettings || {};
const configSettings = (workspaceSettings.configs || []).find(c => c.relativeConfigFile === path.relative(model.config.workspaceFolder, model.config.configFile));
model.isEnabled = configSettings?.enabled || (this._models.length === 1 && !configSettings);
await this._loadModelIfNeeded(model);
Expand All @@ -652,7 +652,7 @@ export class TestModelCollection extends DisposableBase {
if (!model.isEnabled)
return;
await model._listFiles();
const workspaceSettings = this._settingsModel.workspaceSettings.get();
const workspaceSettings = this._context.workspaceState.get(workspaceStateKey) as WorkspaceSettings || {};
const configSettings = (workspaceSettings.configs || []).find(c => c.relativeConfigFile === path.relative(model.config.workspaceFolder, model.config.configFile));
if (configSettings) {
let firstProject = true;
Expand Down Expand Up @@ -727,7 +727,7 @@ export class TestModelCollection extends DisposableBase {
projects: model.projects().map(p => ({ name: p.name, enabled: p.isEnabled })),
});
}
this._settingsModel.workspaceSettings.set(workspaceSettings);
this._context.workspaceState.update(workspaceStateKey, workspaceSettings);
}
}

Expand All @@ -738,6 +738,8 @@ export function projectFiles(project: TestProject): Map<string, reporterTypes.Su
return files;
}

const workspaceStateKey = 'pw.workspace-settings';

const listFilesFlag = Symbol('listFilesFlag');

function isAncestorOf(root: vscodeTypes.TestItem, descendent: vscodeTypes.TestItem) {
Expand Down
12 changes: 8 additions & 4 deletions tests/mock/vscode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -836,7 +836,7 @@ export class VSCode {
readonly fsWatchers = new Set<FileSystemWatcher>();
readonly warnings: string[] = [];
readonly errors: string[] = [];
readonly context: { subscriptions: any[]; extensionUri: Uri; };
readonly context: { subscriptions: any[]; extensionUri: Uri; workspaceState: any };
readonly extensions: any[] = [];
private _webviewProviders = new Map<string, any>();
private _browser: Browser;
Expand All @@ -850,7 +850,12 @@ export class VSCode {

constructor(readonly versionNumber: number, baseDir: string, browser: Browser) {
this.version = String(versionNumber);
this.context = { subscriptions: [], extensionUri: Uri.file(baseDir) };
const workspaceStateStorage = new Map();
const workspaceState = {
get: (key: string) => workspaceStateStorage.get(key),
update: (key: string, value: any) => workspaceStateStorage.set(key, value)
};
this.context = { subscriptions: [], extensionUri: Uri.file(baseDir), workspaceState };
this._browser = browser;
(globalThis as any).__logForTest = message => this.connectionLog.push(message);
const commands = new Map<string, () => Promise<void>>();
Expand Down Expand Up @@ -989,7 +994,6 @@ export class VSCode {
'playwright.env': {},
'playwright.reuseBrowser': false,
'playwright.showTrace': false,
'playwright.workspaceSettings': {},
};
this.workspace.getConfiguration = scope => {
return {
Expand All @@ -1011,7 +1015,7 @@ export class VSCode {

async activate() {
for (const extension of this.extensions)
await extension.activate(this.context);
await extension.activate();

for (const [name, provider] of this._webviewProviders) {
const webview: any = {};
Expand Down
2 changes: 1 addition & 1 deletion tests/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ export const test = baseTest.extend<TestFixtures, WorkerOptions>({
if (showBrowser)
configuration.update('reuseBrowser', true);

const extension = new Extension(vscode);
const extension = new Extension(vscode, vscode.context);
if (overridePlaywrightVersion)
extension.overridePlaywrightVersion = overridePlaywrightVersion;

Expand Down

0 comments on commit 104d278

Please sign in to comment.