Skip to content

Commit

Permalink
chore: reuse test run request (#566)
Browse files Browse the repository at this point in the history
  • Loading branch information
pavelfeldman authored Dec 12, 2024
1 parent 4fba0d0 commit 2e0df2c
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 57 deletions.
44 changes: 22 additions & 22 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"url": "https://github.com/microsoft/playwright/issues"
},
"engines": {
"vscode": "^1.86.0"
"vscode": "^1.93.0"
},
"categories": [
"Testing"
Expand Down Expand Up @@ -158,7 +158,7 @@
"@types/glob": "^8.0.0",
"@types/node": "^18.11.9",
"@types/stack-utils": "^2.0.1",
"@types/vscode": "1.86.0",
"@types/vscode": "1.93.0",
"@types/which": "^3.0.3",
"@types/ws": "^8.5.3",
"@typescript-eslint/eslint-plugin": "^7.3.1",
Expand Down
32 changes: 16 additions & 16 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -335,20 +335,21 @@ export class Extension implements RunHooks {
if (this._testRun && !request.continuous)
return;

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

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

private async _queueTestRun(include: readonly vscodeTypes.TestItem[] | undefined, mode: 'run' | 'debug') {
await this._queueCommand(() => this._runTests(include, mode), undefined);
private async _queueTestRun(request: vscodeTypes.TestRunRequest, mode: 'run' | 'debug') {
await this._queueCommand(() => this._runTests(request, mode), undefined);
}

private async _queueWatchRun(include: readonly vscodeTypes.TestItem[], type: 'files' | 'items') {
private async _queueWatchRun(request: vscodeTypes.TestRunRequest, type: 'files' | 'items') {
const batch = type === 'files' ? this._watchFilesBatch : this._watchItemsBatch;
const include = request.include || [];
if (batch) {
batch.push(...include); // `narrowDownLocations` dedupes before sending to the testserver, no need to dedupe here
return;
Expand All @@ -369,7 +370,7 @@ export class Extension implements RunHooks {
else
this._watchItemsBatch = undefined;

return this._runTests(items, 'watch');
return this._runTests(request, 'watch');
}, undefined);
}

Expand All @@ -391,16 +392,16 @@ export class Extension implements RunHooks {
}
}

private async _runTests(include: readonly vscodeTypes.TestItem[] | undefined, mode: 'run' | 'debug' | 'watch') {
private async _runTests(request: vscodeTypes.TestRunRequest, mode: 'run' | 'debug' | 'watch') {
this._completedSteps.clear();
this._executionLinesChanged();
const include = request.include;

// Create a test run that potentially includes all the test items.
// This allows running setup tests that are outside of the scope of the
// selected test items.
const rootItems: vscodeTypes.TestItem[] = [];
this._testController.items.forEach(item => rootItems.push(item));
const requestWithDeps = new this._vscode.TestRunRequest(rootItems, [], undefined, mode === 'watch');

// Global errors are attributed to the first test item in the request.
// If the request is global, find the first root test item (folder, file) that has
Expand All @@ -419,7 +420,7 @@ export class Extension implements RunHooks {
}
}

this._testRun = this._testController.createTestRun(requestWithDeps);
this._testRun = this._testController.createTestRun(request);
const enqueuedTests: vscodeTypes.TestItem[] = [];
// Provisionally mark tests (not files and not suits) as enqueued to provide immediate feedback.
const toEnqueue = include?.length ? include : rootItems;
Expand All @@ -430,15 +431,14 @@ export class Extension implements RunHooks {
}
}

const items: vscodeTypes.TestItem[] = include ? [...include] : [];
try {
for (const model of this._models.enabledModels()) {
const result = model.narrowDownLocations(items);
const result = model.narrowDownLocations(request);
if (!result.testIds && !result.locations)
continue;
if (!model.enabledProjects().length)
continue;
await this._runTest(this._testRun, items, testItemForGlobalErrors, new Set(), model, mode, enqueuedTests.length === 1);
await this._runTest(this._testRun, request, testItemForGlobalErrors, new Set(), model, mode, enqueuedTests.length === 1);
}
} finally {
this._activeSteps.clear();
Expand Down Expand Up @@ -467,7 +467,7 @@ export class Extension implements RunHooks {

private async _runTest(
testRun: vscodeTypes.TestRun,
items: vscodeTypes.TestItem[],
request: vscodeTypes.TestRunRequest,
testItemForGlobalErrors: vscodeTypes.TestItem | undefined,
testFailures: Set<vscodeTypes.TestItem>,
model: TestModel,
Expand Down Expand Up @@ -565,11 +565,11 @@ export class Extension implements RunHooks {
};

if (mode === 'debug') {
await model.debugTests(items, testListener, testRun.token);
await model.debugTests(request, testListener, testRun.token);
} else {
// Force trace viewer update to surface check version errors.
await this._models.selectedModel()?.updateTraceViewer(mode === 'run')?.willRunTests();
await model.runTests(items, testListener, testRun.token);
await model.runTests(request, testListener, testRun.token);
}
}

Expand Down Expand Up @@ -604,10 +604,10 @@ export class Extension implements RunHooks {
// Run either locations or test ids to always be compatible with the test server (it can run either or).
if (files.length) {
const fileItems = files.map(f => this._testTree.testItemForFile(f)).filter(Boolean) as vscodeTypes.TestItem[];
await this._queueWatchRun(fileItems, 'files');
await this._queueWatchRun(new this._vscode.TestRunRequest(fileItems), 'files');
}
if (testItems.length)
await this._queueWatchRun(testItems, 'items');
await this._queueWatchRun(new this._vscode.TestRunRequest(testItems), 'items');
}

private async _updateVisibleEditorItems() {
Expand Down
13 changes: 7 additions & 6 deletions src/playwrightTestCLI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ export class PlaywrightTestCLI {
async clearCache() {
}

async runTests(items: vscodeTypes.TestItem[], options: PlaywrightTestRunOptions, reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken): Promise<void> {
const { locations, parametrizedTestTitle } = this._narrowDownLocations(items);
async runTests(request: vscodeTypes.TestRunRequest, options: PlaywrightTestRunOptions, reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken): Promise<void> {
const { locations, parametrizedTestTitle } = this._narrowDownLocations(request);
if (!locations)
return;
const args = [];
Expand Down Expand Up @@ -145,10 +145,10 @@ export class PlaywrightTestCLI {
await reporterServer.wireTestListener(reporter, token);
}

async debugTests(items: vscodeTypes.TestItem[], options: PlaywrightTestRunOptions, reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken): Promise<void> {
async debugTests(request: vscodeTypes.TestRunRequest, options: PlaywrightTestRunOptions, reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken): Promise<void> {
const configFolder = path.dirname(this._model.config.configFile);
const configFile = path.basename(this._model.config.configFile);
const { locations, parametrizedTestTitle } = this._narrowDownLocations(items);
const { locations, parametrizedTestTitle } = this._narrowDownLocations(request);
if (!locations)
return;
const testDirs = this._model.enabledProjects().map(p => p.project.testDir);
Expand Down Expand Up @@ -255,9 +255,10 @@ export class PlaywrightTestCLI {
this._options.playwrightTestLog.push(line);
}

private _narrowDownLocations(items: vscodeTypes.TestItem[]): { locations: string[] | null, parametrizedTestTitle: string | undefined } {
if (!items.length)
private _narrowDownLocations(request: vscodeTypes.TestRunRequest): { locations: string[] | null, parametrizedTestTitle: string | undefined } {
if (!request.include?.length)
return { locations: [], parametrizedTestTitle: undefined };
const items = request.include;

let parametrizedTestTitle: string | undefined;
// When we are given one item, check if it is parametrized (more than 1 item on that line).
Expand Down
8 changes: 4 additions & 4 deletions src/playwrightTestServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,14 +164,14 @@ export class PlaywrightTestServer {
await testServer?.clearCache({});
}

async runTests(items: vscodeTypes.TestItem[], runOptions: PlaywrightTestRunOptions, reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken): Promise<void> {
async runTests(request: vscodeTypes.TestRunRequest, runOptions: PlaywrightTestRunOptions, reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken): Promise<void> {
const testServer = await this._testServer();
if (token?.isCancellationRequested)
return;
if (!testServer)
return;

const { locations, testIds } = this._model.narrowDownLocations(items);
const { locations, testIds } = this._model.narrowDownLocations(request);
if (!locations && !testIds)
return;

Expand Down Expand Up @@ -224,7 +224,7 @@ export class PlaywrightTestServer {
};
}

async debugTests(items: vscodeTypes.TestItem[], runOptions: PlaywrightTestRunOptions, reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken): Promise<void> {
async debugTests(request: vscodeTypes.TestRunRequest, runOptions: PlaywrightTestRunOptions, reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken): Promise<void> {
const addressPromise = new Promise<string>(f => {
const disposable = this._options.onStdOut(output => {
const match = output.match(/Listening on (.*)/);
Expand Down Expand Up @@ -291,7 +291,7 @@ export class PlaywrightTestServer {
if (token?.isCancellationRequested)
return;

const { locations, testIds } = this._model.narrowDownLocations(items);
const { locations, testIds } = this._model.narrowDownLocations(request);
if (!locations && !testIds)
return;

Expand Down
14 changes: 7 additions & 7 deletions src/testModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ export class TestModel extends DisposableBase {
await this._playwrightTest.clearCache();
}

async runTests(items: vscodeTypes.TestItem[], reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken) {
async runTests(request: vscodeTypes.TestRunRequest, reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken) {
if (token?.isCancellationRequested)
return;

Expand Down Expand Up @@ -555,13 +555,13 @@ export class TestModel extends DisposableBase {
try {
if (token?.isCancellationRequested)
return;
await this._playwrightTest.runTests(items, options, reporter, token);
await this._playwrightTest.runTests(request, options, reporter, token);
} finally {
await this._embedder.runHooks.onDidRunTests(false);
}
}

async debugTests(items: vscodeTypes.TestItem[], reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken) {
async debugTests(request: vscodeTypes.TestRunRequest, reporter: reporterTypes.ReporterV2, token: vscodeTypes.CancellationToken) {
if (token?.isCancellationRequested)
return;

Expand All @@ -582,7 +582,7 @@ export class TestModel extends DisposableBase {
try {
if (token?.isCancellationRequested)
return;
await this._playwrightTest.debugTests(items, options, reporter, token);
await this._playwrightTest.debugTests(request, options, reporter, token);
} finally {
await this._embedder.runHooks.onDidRunTests(false);
}
Expand Down Expand Up @@ -636,12 +636,12 @@ export class TestModel extends DisposableBase {
await this._playwrightTest.watchFiles([...filesToWatch]);
}

narrowDownLocations(items: vscodeTypes.TestItem[]): { locations: string[] | null, testIds?: string[] } {
if (!items.length)
narrowDownLocations(request: vscodeTypes.TestRunRequest): { locations: string[] | null, testIds?: string[] } {
if (!request.include?.length)
return { locations: [] };
const locations = new Set<string>();
const testIds: string[] = [];
for (const item of items) {
for (const item of request.include) {
const treeItem = upstreamTreeItem(item);
if (treeItem.kind === 'group' && (treeItem.subKind === 'folder' || treeItem.subKind === 'file')) {
for (const file of this.enabledFiles()) {
Expand Down

0 comments on commit 2e0df2c

Please sign in to comment.