Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(state): use workspace state instead of the settings #476

Merged
merged 1 commit into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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