From e02a704f991a11cfbd478f02a6607c077d67d1ba Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Thu, 13 Feb 2025 15:01:00 -0500 Subject: [PATCH] Resolve test items with server request (#3186) * Add discover tests custom request (#3180) ### Motivation Closes #3171 Add a new custom request to discover tests in a specific document. This request will instantiate all listeners and collect all of the discovered groups and examples as test items for the editor. ### Implementation - Created the new request - Implement a listener dedicated to the test style syntax only (spec will be a separate listener) - Ensured to use ancestor linearization to determine the test framework ### Automated Tests Added tests. * Resolve test items with server request (#3186) ### Motivation Closes #3173 Start firing our new custom request to discover which tests are available in a given file. ### Implementation We fire the request and then recursively add test items to their respective collections. If a document is saved, we check to see if we created a test item for it and then we clear its children (the tests defined inside of the file) and re-trigger the request. I chose to use on save because on change would put a lot of pressure on the server. ### Automated Tests Added tests. --------- Co-authored-by: vinistock <18742907+vinistock@users.noreply.github.com> --- vscode/src/client.ts | 17 ++++ vscode/src/rubyLsp.ts | 37 ++++++- vscode/src/test/suite/testController.test.ts | 100 +++++++++++++++++++ vscode/src/testController.ts | 78 ++++++++++++++- 4 files changed, 227 insertions(+), 5 deletions(-) diff --git a/vscode/src/client.ts b/vscode/src/client.ts index 4d4b5246f..38b8fc669 100644 --- a/vscode/src/client.ts +++ b/vscode/src/client.ts @@ -43,6 +43,17 @@ import { WorkspaceChannel } from "./workspaceChannel"; type EnabledFeatures = Record; +export interface ServerTestItem { + id: string; + label: string; + uri: string; + range: { + start: { line: number; column: number }; + end: { line: number; column: number }; + }; + children: ServerTestItem[]; +} + interface ServerErrorTelemetryEvent { type: "error"; errorMessage: string; @@ -459,6 +470,12 @@ export default class Client extends LanguageClient implements ClientInterface { }); } + async discoverTests(uri: vscode.Uri): Promise { + return this.sendRequest("rubyLsp/discoverTests", { + textDocument: { uri: uri.toString() }, + }); + } + async dispose(timeout?: number): Promise { this.subscriptions.forEach((subscription) => subscription.dispose()); this.subscriptions = []; diff --git a/vscode/src/rubyLsp.ts b/vscode/src/rubyLsp.ts index 890a8e7fe..88266bc2c 100644 --- a/vscode/src/rubyLsp.ts +++ b/vscode/src/rubyLsp.ts @@ -45,6 +45,7 @@ export class RubyLsp { context, this.telemetry, this.currentActiveWorkspace.bind(this), + this.getOrActivateWorkspace.bind(this), ); this.debug = new Debugger(context, this.workspaceResolver.bind(this)); this.rails = new Rails(this.showWorkspacePick.bind(this)); @@ -174,10 +175,22 @@ export class RubyLsp { } } + // Overloaded signatures because when the workspace activation is lazy, it is guaranteed to return `Workspace`, which + // avoids checking for undefined in the caller + private async activateWorkspace( + workspaceFolder: vscode.WorkspaceFolder, + eager: true, + ): Promise; + + private async activateWorkspace( + workspaceFolder: vscode.WorkspaceFolder, + eager: false, + ): Promise; + private async activateWorkspace( workspaceFolder: vscode.WorkspaceFolder, eager: boolean, - ) { + ): Promise { const customBundleGemfile: string = vscode.workspace .getConfiguration("rubyLsp") .get("bundleGemfile")!; @@ -219,6 +232,7 @@ export class RubyLsp { ); await this.showFormatOnSaveModeWarning(workspace); this.workspacesBeingLaunched.delete(workspaceFolder.index); + return workspace; } // Registers all extension commands. Commands can only be registered once, so this happens in the constructor. For @@ -702,6 +716,27 @@ export class RubyLsp { return this.getWorkspace(workspaceFolder.uri); } + private async getOrActivateWorkspace( + workspaceFolder: vscode.WorkspaceFolder, + ): Promise { + const workspace = this.getWorkspace(workspaceFolder.uri); + + if (workspace) { + return workspace; + } + + return vscode.window.withProgress( + { + location: vscode.ProgressLocation.Notification, + title: `Workspace ${workspaceFolder.name} is not activated yet.`, + }, + async (progress) => { + progress.report({ message: "Activating workspace..." }); + return this.activateWorkspace(workspaceFolder, false); + }, + ); + } + private getWorkspace(uri: vscode.Uri): Workspace | undefined { return this.workspaces.get(uri.toString()); } diff --git a/vscode/src/test/suite/testController.test.ts b/vscode/src/test/suite/testController.test.ts index 4c72b67cd..331a43abd 100644 --- a/vscode/src/test/suite/testController.test.ts +++ b/vscode/src/test/suite/testController.test.ts @@ -10,6 +10,7 @@ import sinon from "sinon"; import { TestController } from "../../testController"; import * as common from "../../common"; +import { Workspace } from "../../workspace"; import { FAKE_TELEMETRY } from "./fakeTelemetry"; @@ -31,6 +32,14 @@ suite("TestController", () => { update: (_name: string, _value: any) => Promise.resolve(), }, } as unknown as vscode.ExtensionContext; + const workspace = new Workspace( + context, + workspaceFolder, + FAKE_TELEMETRY, + () => undefined, + new Map(), + true, + ); afterEach(() => { context.subscriptions.forEach((subscription) => subscription.dispose()); @@ -41,6 +50,7 @@ suite("TestController", () => { context, FAKE_TELEMETRY, () => undefined, + () => Promise.resolve(workspace), ); const codeLensItems: CodeLens[] = [ @@ -83,6 +93,7 @@ suite("TestController", () => { context, FAKE_TELEMETRY, () => undefined, + () => Promise.resolve(workspace), ); stub.restore(); @@ -106,11 +117,19 @@ suite("TestController", () => { vscode.Uri.joinPath(workspaceUri, "test").toString(), ); assert.ok(testDir); + assert.deepStrictEqual( + testDir!.tags.map((tag) => tag.id), + ["test_dir", "debug"], + ); const serverTest = testDir!.children.get( vscode.Uri.joinPath(workspaceUri, "test", "server_test.rb").toString(), ); assert.ok(serverTest); + assert.deepStrictEqual( + serverTest!.tags.map((tag) => tag.id), + ["debug"], + ); }); test("makes the workspaces the top level when there's more than one", async () => { @@ -119,6 +138,7 @@ suite("TestController", () => { context, FAKE_TELEMETRY, () => undefined, + () => Promise.resolve(workspace), ); stub.restore(); @@ -171,27 +191,49 @@ suite("TestController", () => { // First workspace const workspaceItem = collection.get(workspaceUri.toString()); assert.ok(workspaceItem); + assert.deepStrictEqual( + workspaceItem!.tags.map((tag) => tag.id), + ["workspace", "debug"], + ); + await controller.testController.resolveHandler!(workspaceItem); const testDir = workspaceItem!.children.get( vscode.Uri.joinPath(workspaceUri, "test").toString(), ); assert.ok(testDir); + assert.deepStrictEqual( + testDir!.tags.map((tag) => tag.id), + ["test_dir", "debug"], + ); const serverTest = testDir!.children.get( vscode.Uri.joinPath(workspaceUri, "test", "server_test.rb").toString(), ); assert.ok(serverTest); + assert.deepStrictEqual( + serverTest!.tags.map((tag) => tag.id), + ["debug"], + ); // Second workspace const secondWorkspaceItem = collection.get(secondWorkspaceUri.toString()); assert.ok(secondWorkspaceItem); + assert.deepStrictEqual( + secondWorkspaceItem!.tags.map((tag) => tag.id), + ["workspace", "debug"], + ); + await controller.testController.resolveHandler!(secondWorkspaceItem); const secondTestDir = secondWorkspaceItem!.children.get( vscode.Uri.joinPath(secondWorkspaceUri, "test").toString(), ); assert.ok(secondTestDir); + assert.deepStrictEqual( + secondTestDir!.tags.map((tag) => tag.id), + ["test_dir", "debug"], + ); const otherTest = secondTestDir!.children.get( vscode.Uri.joinPath( @@ -201,8 +243,66 @@ suite("TestController", () => { ).toString(), ); assert.ok(otherTest); + assert.deepStrictEqual( + otherTest!.tags.map((tag) => tag.id), + ["debug"], + ); + workspacesStub.restore(); relativePathStub.restore(); getWorkspaceStub.restore(); }); + + test("fires discover tests request when resolving a specific test file", async () => { + const stub = sinon.stub(common, "featureEnabled").returns(true); + const controller = new TestController( + context, + FAKE_TELEMETRY, + () => undefined, + () => Promise.resolve(workspace), + ); + stub.restore(); + + const workspacesStub = sinon + .stub(vscode.workspace, "workspaceFolders") + .get(() => [workspaceFolder]); + + const relativePathStub = sinon + .stub(vscode.workspace, "asRelativePath") + .callsFake((uri) => + path.relative(workspacePath, (uri as vscode.Uri).fsPath), + ); + + await controller.testController.resolveHandler!(undefined); + + const collection = controller.testController.items; + + const testDir = collection.get( + vscode.Uri.joinPath(workspaceUri, "test").toString(), + ); + assert.ok(testDir); + assert.deepStrictEqual( + testDir!.tags.map((tag) => tag.id), + ["test_dir", "debug"], + ); + + const serverTest = testDir!.children.get( + vscode.Uri.joinPath(workspaceUri, "test", "server_test.rb").toString(), + ); + assert.ok(serverTest); + assert.deepStrictEqual( + serverTest!.tags.map((tag) => tag.id), + ["debug"], + ); + + const fakeClient = { + discoverTests: sinon.stub().resolves([]), + }; + workspace.lspClient = fakeClient as any; + await controller.testController.resolveHandler!(serverTest); + assert.strictEqual(fakeClient.discoverTests.callCount, 1); + + workspacesStub.restore(); + relativePathStub.restore(); + }); }); diff --git a/vscode/src/testController.ts b/vscode/src/testController.ts index 9a4aad5ef..d8a29fbb9 100644 --- a/vscode/src/testController.ts +++ b/vscode/src/testController.ts @@ -7,6 +7,7 @@ import { CodeLens } from "vscode-languageclient/node"; import { Workspace } from "./workspace"; import { featureEnabled } from "./common"; +import { ServerTestItem } from "./client"; const asyncExec = promisify(exec); @@ -20,6 +21,7 @@ interface CodeLensData { const WORKSPACE_TAG = new vscode.TestTag("workspace"); const TEST_DIR_TAG = new vscode.TestTag("test_dir"); +const TEST_GROUP_TAG = new vscode.TestTag("test_group"); const DEBUG_TAG = new vscode.TestTag("debug"); export class TestController { @@ -36,15 +38,23 @@ export class TestController { .get("testTimeout") as number; private readonly currentWorkspace: () => Workspace | undefined; + private readonly getOrActivateWorkspace: ( + workspaceFolder: vscode.WorkspaceFolder, + ) => Promise; + private readonly fullDiscovery = featureEnabled("fullTestDiscovery"); constructor( context: vscode.ExtensionContext, telemetry: vscode.TelemetryLogger, currentWorkspace: () => Workspace | undefined, + getOrActivateWorkspace: ( + workspaceFolder: vscode.WorkspaceFolder, + ) => Promise, ) { this.telemetry = telemetry; this.currentWorkspace = currentWorkspace; + this.getOrActivateWorkspace = getOrActivateWorkspace; this.testController = vscode.tests.createTestController( "rubyTests", "Ruby Tests", @@ -100,6 +110,19 @@ export class TestController { item, ); }), + vscode.workspace.onDidSaveTextDocument(async (document) => { + const uri = document.uri; + const item = await this.getParentTestItem(uri); + + if (item) { + const testFile = item.children.get(uri.toString()); + + if (testFile) { + testFile.children.replace([]); + await this.resolveHandler(testFile); + } + } + }), ); } @@ -449,9 +472,13 @@ export class TestController { // If the item is a workspace, then we need to gather all test files inside of it if (item.tags.some((tag) => tag === WORKSPACE_TAG)) { await this.gatherWorkspaceTests(workspaceFolder, item); - } else { - // TODO: we are resolving the children of a specific test file. Here we will ask the server to parse the file, - // run all available listeners and then populate all available groups and examples + } else if (!item.tags.some((tag) => tag === TEST_GROUP_TAG)) { + const workspace = await this.getOrActivateWorkspace(workspaceFolder); + const testItems = await workspace.lspClient?.discoverTests(item.uri!); + + if (testItems) { + this.addDiscoveredItems(testItems, item); + } } } else if (workspaceFolders.length === 1) { // If there's only one workspace, there's no point in nesting the tests under the workspace name @@ -497,6 +524,13 @@ export class TestController { // components, so we want to show each individual test directory as a separate item const relativePath = vscode.workspace.asRelativePath(uri, false); const pathParts = relativePath.split(path.sep); + + // Projects may have fixtures that are test files, but not real tests to be executed. We don't want to include + // those + if (pathParts.some((part) => part === "fixtures")) { + continue; + } + const dirPosition = this.testDirectoryPosition(pathParts); const firstLevelName = pathParts.slice(0, dirPosition + 1).join(path.sep); const firstLevelUri = vscode.Uri.joinPath( @@ -582,6 +616,11 @@ export class TestController { // Finds a test item based on its URI taking all possible hierarchies into account private async getTestItem(uri: vscode.Uri) { + const item = await this.getParentTestItem(uri); + return item?.children.get(uri.toString()); + } + + private async getParentTestItem(uri: vscode.Uri) { const workspaceFolders = vscode.workspace.workspaceFolders; if (!workspaceFolders) { return undefined; @@ -608,7 +647,7 @@ export class TestController { item = item?.children.get(secondLevelUri.toString()); } - return item?.children.get(uri.toString()); + return item; } private async directoryLevelUris( @@ -640,4 +679,35 @@ export class TestController { return { firstLevelUri, secondLevelUri: undefined }; } + + private addDiscoveredItems( + testItems: ServerTestItem[], + parent: vscode.TestItem, + ) { + testItems.forEach((item) => { + const testItem = this.testController.createTestItem( + item.id, + item.label, + vscode.Uri.parse(item.uri), + ); + + testItem.canResolveChildren = item.children.length > 0; + const start = item.range.start; + const end = item.range.end; + + testItem.range = new vscode.Range( + new vscode.Position(start.line, start.column), + new vscode.Position(end.line, end.column), + ); + testItem.tags = testItem.canResolveChildren + ? [TEST_GROUP_TAG, DEBUG_TAG] + : [DEBUG_TAG]; + + parent.children.add(testItem); + + if (testItem.canResolveChildren) { + this.addDiscoveredItems(item.children, testItem); + } + }); + } }