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

chore: wire-only watch implementation #457

Merged
merged 1 commit into from
Apr 4, 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
40 changes: 21 additions & 19 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export class Extension implements RunHooks {
private _runProfile: vscodeTypes.TestRunProfile;
private _debugProfile: vscodeTypes.TestRunProfile;
private _playwrightTestLog: string[] = [];
private _runQueue = Promise.resolve();
private _commandQueue = Promise.resolve();

constructor(vscode: vscodeTypes.VSCode) {
this._vscode = vscode;
Expand Down Expand Up @@ -240,7 +240,7 @@ export class Extension implements RunHooks {
}

private async _rebuildModels(userGesture: boolean): Promise<vscodeTypes.Uri[]> {
this._runQueue = Promise.resolve();
this._commandQueue = Promise.resolve();
this._models.clear();
this._testTree.startedLoading();

Expand Down Expand Up @@ -316,29 +316,20 @@ export class Extension implements RunHooks {
if (this._testRun && !request.continuous)
return;

await this._queueTestRun(request.include, isDebug ? 'debug' : 'run');

if (request.continuous) {
for (const model of this._models.enabledModels())
model.addToWatch(request.include, cancellationToken!);
return;
}

await this._queueTestRun(request.include, isDebug ? 'debug' : 'run');
}

private async _queueTestRun(include: readonly vscodeTypes.TestItem[] | undefined, mode: 'run' | 'debug' | 'watch') {
this._runQueue = this._runQueue.then(async () => {
await this._runTests(include, mode).catch(e => console.error(e));
});
await this._runQueue;
await this._queueCommand(() => this._runTests(include, mode), undefined);
}

private async _queueGlobalHooks(type: 'setup' | 'teardown'): Promise<reporterTypes.FullResult['status']> {
let result: reporterTypes.FullResult['status'] = 'passed';
this._runQueue = this._runQueue.then(async () => {
result = await this._runGlobalHooks(type);
}).catch(e => console.error(e));
await this._runQueue;
return result;
return await this._queueCommand(() => this._runGlobalHooks(type), 'failed');
}

private async _runGlobalHooks(type: 'setup' | 'teardown') {
Expand Down Expand Up @@ -421,8 +412,11 @@ export class Extension implements RunHooks {
}

private async _workspaceChanged(change: WorkspaceChange) {
for (const model of this._models.enabledModels())
await model.workspaceChanged(change);
await this._queueCommand(async () => {
for (const model of this._models.enabledModels())
await model.handleWorkspaceChange(change);
}, undefined);

// Workspace change can be deferred, make sure editors are
// decorated.
await this._updateVisibleEditorItems();
Expand Down Expand Up @@ -576,8 +570,10 @@ export class Extension implements RunHooks {
}

private async _ensureTestsInAllModels(inputFiles: string[]): Promise<void> {
for (const model of this._models.enabledModels())
await model.ensureTests(inputFiles);
await this._queueCommand(async () => {
for (const model of this._models.enabledModels())
await model.ensureTests(inputFiles);
}, undefined);
}

private _updateDiagnostics() {
Expand Down Expand Up @@ -754,6 +750,12 @@ export class Extension implements RunHooks {
if (testModel)
this._traceViewer.open(traceUrl, testModel.config);
}

private _queueCommand<T>(callback: () => Promise<T>, defaultValue: T): Promise<T> {
const result = this._commandQueue.then(callback).catch(e => { console.error(e); return defaultValue; });
this._commandQueue = result.then(() => {});
return result;
}
}

function parseLocationFromStack(stack: string | undefined, testItem?: vscodeTypes.TestItem): reporterTypes.Location | undefined {
Expand Down
3 changes: 3 additions & 0 deletions src/playwrightTestCLI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,9 @@ export class PlaywrightTestCLI {
}
}

async watchFiles(fileNames: string[]) {
}

async findRelatedTestFiles(files: string[]): Promise<ConfigFindRelatedTestFilesReport> {
const configFolder = path.dirname(this._model.config.configFile);
const configFile = path.basename(this._model.config.configFile);
Expand Down
12 changes: 12 additions & 0 deletions src/playwrightTestServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,13 @@ export class PlaywrightTestServer {
}
}

async watchFiles(fileNames: string[]) {
const testServer = await this._testServer();
if (!testServer)
return;
await testServer.watch({ fileNames });
}

async findRelatedTestFiles(files: string[]): Promise<ConfigFindRelatedTestFilesReport> {
const testServer = await this._testServer();
if (!testServer)
Expand Down Expand Up @@ -286,6 +293,7 @@ export class PlaywrightTestServer {
if (!wsEndpoint)
return null;
const testServer = new TestServerConnection(wsEndpoint);
testServer.onTestFilesChanged(params => this._testFilesChanged(params.testFiles));
await testServer.initialize({
serializer: require.resolve('./oopReporter'),
interceptStdio: true,
Expand Down Expand Up @@ -313,6 +321,10 @@ export class PlaywrightTestServer {
});
}

private _testFilesChanged(testFiles: string[]) {
this._model.testFilesChanged(testFiles.map(f => this._vscode.Uri.file(f).fsPath));
}

private _disposeTestServer() {
const testServer = this._testServerPromise;
this._testServerPromise = undefined;
Expand Down
30 changes: 22 additions & 8 deletions src/testModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ export class TestModel {
project.suite.suites = [...files.values()];
}

async workspaceChanged(change: WorkspaceChange) {
async handleWorkspaceChange(change: WorkspaceChange) {
const testDirs = [...new Set([...this._projects.values()].map(p => p.project.testDir))];

const changed = this._mapFilesToSources(testDirs, change.changed);
Expand All @@ -253,16 +253,13 @@ export class TestModel {
this._filesWithListedTests.delete(c);
await this.ensureTests(changedWithListedTests);
}
}

testFilesChanged(testFiles: string[]) {
if (!this._watches.size)
return;
if (!changed.length)
return;

const result = await this._playwrightTest.findRelatedTestFiles([...change.changed]);
if (!result.testFiles.length)
if (!testFiles.length)
return;
const testFiles = result.testFiles.map(f => this._vscode.Uri.file(f).fsPath);

const files: string[] = [];
const items: vscodeTypes.TestItem[] = [];
Expand Down Expand Up @@ -492,7 +489,7 @@ export class TestModel {
return [...result];
}

addToWatch(include: readonly vscodeTypes.TestItem[] | undefined, cancellationToken: vscodeTypes.CancellationToken) {
async addToWatch(include: readonly vscodeTypes.TestItem[] | undefined, cancellationToken: vscodeTypes.CancellationToken) {
const watch: Watch = { include };
this._watches.add(watch);
cancellationToken.onCancellationRequested(() => this._watches.delete(watch));
Expand All @@ -509,6 +506,23 @@ export class TestModel {
}
}
}

const filesToWatch = new Set<string>();
for (const watch of this._watches) {
if (!watch.include) {
for (const project of this._projects.values()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enabledFiles()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll follow up.

for (const fileSuite of project.suite.suites)
filesToWatch.add(fileSuite.location!.file);
}
continue;
}
for (const include of watch.include) {
if (!include.uri)
continue;
filesToWatch.add(include.uri.fsPath);
}
}
await this._playwrightTest.watchFiles([...filesToWatch]);
}

narrowDownLocations(items: vscodeTypes.TestItem[]): { locations: string[] | null, testIds?: string[] } {
Expand Down
7 changes: 7 additions & 0 deletions src/upstream/testServerConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export class TestServerConnection implements TestServerInterface, TestServerInte
this._ws.addEventListener('message', event => {
const message = JSON.parse(String(event.data));
const { id, result, error, method, params } = message;
this._debugLog('RECV>', message);
if (id) {
const callback = this._callbacks.get(id);
if (!callback)
Expand Down Expand Up @@ -82,6 +83,7 @@ export class TestServerConnection implements TestServerInterface, TestServerInte
await this._connectedPromise;
const id = ++this._lastId;
const message = { id, method, params };
this._debugLog('SEND>', message);
this._ws.send(JSON.stringify(message));
return new Promise((resolve, reject) => {
this._callbacks.set(id, { resolve, reject });
Expand Down Expand Up @@ -191,4 +193,9 @@ export class TestServerConnection implements TestServerInterface, TestServerInte
} catch {
}
}

_debugLog(name: string, message: any) {
if (process.env.DEBUG?.includes('pwtest:server'))
console.log(name, message.id,message.method);
}
}
2 changes: 2 additions & 0 deletions tests/mock/vscode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ export class WorkspaceFolder {
}

async changeFile(file: string, content: string) {
// TODO: investigate why watch immediately followed by changeFile doesn't emit the event.
await new Promise(f => setTimeout(f, 1000));
const fsPath = path.join(this.uri.fsPath, file);
await fs.promises.writeFile(fsPath, content);
for (const watcher of this.vscode.fsWatchers) {
Expand Down
Loading