From be406f8105379d6db8780797370e1af0c21abfef Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Wed, 24 Apr 2024 11:50:54 -0400 Subject: [PATCH 1/9] Discover tests via LSP if available If the LSP is available and supports the two requests for listing tests then use those to discover tests in the workspace/document. If these requests are unsupported or if they fail, fall back to the existing methods. This patch uses the existing code to leverage the LSP's workspace/tests method and adds the textDocument/tests for listing tests within documents as they change. These requests produce a collection of TestClass structures, which are hierarchical. These are passed to one of the methods in TestDiscovery.ts which diffs against the existing tree and adds/removes tests. This is similar to how it already worked, but now supports arbitrary levels of nesting in the TestClass structure. This patch also adds preliminary support for listing swift-testing tests through the LSP methods, as well as through `swift test --list-tests`. It does not support finding swift-testing tests through the document symbols. Another small improvement is if listing tests fails via `swift test --list-tests` we'll attempt a `swift build --build-tests` before trying again. If the user was on the 5.10 toolchain and no build had yet been performed they would get an error. Swift-testing tests are filtered out of the list of available tests until code is added to run them. This will be added with #757. - Adds tests for parsing the results of `swift test --list-tests` - Adds tests for parsing tests via document symbols This patch addresses both #761 and #762. --- .../DocumentSymbolTestDiscovery.ts | 81 ++++++ src/TestExplorer/LSPTestDiscovery.ts | 197 ++++++++------- src/TestExplorer/SPMTestDiscovery.ts | 84 +++++++ src/TestExplorer/TestDiscovery.ts | 237 ++++++++---------- src/TestExplorer/TestExplorer.ts | 224 +++++++++-------- src/sourcekit-lsp/LanguageClientManager.ts | 4 +- src/sourcekit-lsp/lspExtensions.ts | 71 +++++- .../DocumentSymbolTestDiscovery.test.ts | 101 ++++++++ .../SPMTestListOutputParser.test.ts | 216 ++++++++++++++++ .../TestOutputParser.test.ts | 2 +- 10 files changed, 876 insertions(+), 341 deletions(-) create mode 100644 src/TestExplorer/DocumentSymbolTestDiscovery.ts create mode 100644 src/TestExplorer/SPMTestDiscovery.ts create mode 100644 test/suite/testexplorer/DocumentSymbolTestDiscovery.test.ts create mode 100644 test/suite/testexplorer/SPMTestListOutputParser.test.ts rename test/suite/{ => testexplorer}/TestOutputParser.test.ts (99%) diff --git a/src/TestExplorer/DocumentSymbolTestDiscovery.ts b/src/TestExplorer/DocumentSymbolTestDiscovery.ts new file mode 100644 index 000000000..83bd91220 --- /dev/null +++ b/src/TestExplorer/DocumentSymbolTestDiscovery.ts @@ -0,0 +1,81 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the VSCode Swift open source project +// +// Copyright (c) 2021-2024 the VSCode Swift project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of VSCode Swift project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +import * as vscode from "vscode"; +import { TestClass } from "./TestDiscovery"; +import { parseTestsFromSwiftTestListOutput } from "./SPMTestDiscovery"; + +export function parseTestsFromDocumentSymbols( + target: string, + symbols: vscode.DocumentSymbol[], + uri: vscode.Uri +): TestClass[] { + // Converts a document into the output of `swift test list`. + // This _only_ looks for XCTests. + const locationLookup = new Map(); + const swiftTestListOutput = symbols + .filter( + symbol => + symbol.kind === vscode.SymbolKind.Class || + symbol.kind === vscode.SymbolKind.Namespace + ) + .flatMap(symbol => { + const functions = symbol.children + .filter(func => func.kind === vscode.SymbolKind.Method) + .filter(func => func.name.match(/^test.*\(\)/)) + .map(func => { + const openBrackets = func.name.indexOf("("); + let funcName = func.name; + if (openBrackets) { + funcName = func.name.slice(0, openBrackets); + } + return { + name: funcName, + location: new vscode.Location(uri, func.range), + }; + }); + + const location = + symbol.kind === vscode.SymbolKind.Class + ? new vscode.Location(uri, symbol.range) + : undefined; + + locationLookup.set(`${target}.${symbol.name}`, location); + + return functions.map(func => { + const testName = `${target}.${symbol.name}/${func.name}`; + locationLookup.set(testName, func.location); + return testName; + }); + }) + .join("\n"); + + const tests = parseTestsFromSwiftTestListOutput(swiftTestListOutput); + + // The locations for each test case/suite were captured when processing the + // symbols. Annotate the processed TestClasses with their locations. + const annotatedTests = annotateTestsWithLocations(tests, locationLookup); + return annotatedTests; +} + +function annotateTestsWithLocations( + tests: TestClass[], + locations: Map +): TestClass[] { + return tests.map(test => ({ + ...test, + location: locations.get(test.id), + children: annotateTestsWithLocations(test.children, locations), + })); +} diff --git a/src/TestExplorer/LSPTestDiscovery.ts b/src/TestExplorer/LSPTestDiscovery.ts index 6c3a635c5..e41f0fc61 100644 --- a/src/TestExplorer/LSPTestDiscovery.ts +++ b/src/TestExplorer/LSPTestDiscovery.ts @@ -2,7 +2,7 @@ // // This source file is part of the VSCode Swift open source project // -// Copyright (c) 2021 the VSCode Swift project authors +// Copyright (c) 2021-2024 the VSCode Swift project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -13,11 +13,16 @@ //===----------------------------------------------------------------------===// import * as vscode from "vscode"; -import * as langclient from "vscode-languageclient/node"; import * as TestDiscovery from "./TestDiscovery"; -import { workspaceTestsRequest } from "../sourcekit-lsp/lspExtensions"; +import { + LSPTestItem, + textDocumentTestsRequest, + workspaceTestsRequest, +} from "../sourcekit-lsp/lspExtensions"; import { isPathInsidePath } from "../utilities/utilities"; import { LanguageClientManager } from "../sourcekit-lsp/LanguageClientManager"; +import { LanguageClient } from "vscode-languageclient/node"; +import { SwiftPackage, TargetType } from "../SwiftPackage"; /** * Used to augment test discovery via `swift test --list-tests`. @@ -29,104 +34,112 @@ import { LanguageClientManager } from "../sourcekit-lsp/LanguageClientManager"; */ export class LSPTestDiscovery { constructor(private languageClient: LanguageClientManager) {} + /** - * Used to augment test discovery via `swift test --list-tests`. - * - * Parses result of any document symbol request to add/remove tests from - * the current file. + * Return a list of tests in the supplied document. + * @param document A document to query */ - getTests(symbols: vscode.DocumentSymbol[], uri: vscode.Uri): TestDiscovery.TestClass[] { - return symbols - .filter( - symbol => - symbol.kind === vscode.SymbolKind.Class || - symbol.kind === vscode.SymbolKind.Namespace - ) - .map(symbol => { - const functions = symbol.children - .filter(func => func.kind === vscode.SymbolKind.Method) - .filter(func => func.name.match(/^test.*\(\)/)) - .map(func => { - const openBrackets = func.name.indexOf("("); - let funcName = func.name; - if (openBrackets) { - funcName = func.name.slice(0, openBrackets); - } - return { - name: funcName, - location: new vscode.Location(uri, func.range), - }; - }); - const location = - symbol.kind === vscode.SymbolKind.Class - ? new vscode.Location(uri, symbol.range) - : undefined; - // As we cannot recognise if a symbol is a new XCTestCase class we have - // to treat everything as an extension of existing classes - return { - name: symbol.name, - location: location, - extension: true, //symbol.kind === vscode.SymbolKind.Namespace - functions: functions, - }; - }) - .reduce((result, current) => { - const index = result.findIndex(item => item.name === current.name); - if (index !== -1) { - result[index].functions = [...result[index].functions, ...current.functions]; - result[index].location = result[index].location ?? current.location; - result[index].extension = result[index].extension || current.extension; - return result; - } else { - return [...result, current]; - } - }, new Array()); + async getDocumentTests( + swiftPackage: SwiftPackage, + document: vscode.Uri + ): Promise { + return await this.languageClient.useLanguageClient(async (client, token) => { + const workspaceTestCaps = + client.initializeResult?.capabilities.experimental[textDocumentTestsRequest.method]; + + // Only use the lsp for this request if it supports the + // textDocument/tests method, and is at least version 2. + if (workspaceTestCaps?.version >= 2) { + const testsInDocument = await client.sendRequest( + textDocumentTestsRequest, + { textDocument: { uri: document.toString() } }, + token + ); + return this.transform(client, swiftPackage, testsInDocument); + } else { + throw new Error("workspace/tests requests not supported"); + } + }); } /** * Return list of workspace tests * @param workspaceRoot Root of current workspace folder */ - async getWorkspaceTests(workspaceRoot: vscode.Uri): Promise { + async getWorkspaceTests( + swiftPackage: SwiftPackage, + workspaceRoot: vscode.Uri + ): Promise { return await this.languageClient.useLanguageClient(async (client, token) => { - const tests = await client.sendRequest(workspaceTestsRequest, {}, token); - const testsInWorkspace = tests.filter(item => - isPathInsidePath( - client.protocol2CodeConverter.asLocation(item.location).uri.fsPath, - workspaceRoot.fsPath - ) - ); - const classes = testsInWorkspace - .filter(item => { - return ( - item.kind === langclient.SymbolKind.Class && - isPathInsidePath( - client.protocol2CodeConverter.asLocation(item.location).uri.fsPath, - workspaceRoot.fsPath - ) - ); - }) - .map(item => { - const functions = testsInWorkspace - .filter(func => func.containerName === item.name) - .map(func => { - const openBrackets = func.name.indexOf("("); - let funcName = func.name; - if (openBrackets) { - funcName = func.name.slice(0, openBrackets); - } - return { - name: funcName, - location: client.protocol2CodeConverter.asLocation(func.location), - }; - }); - return { - name: item.name, - location: client.protocol2CodeConverter.asLocation(item.location), - functions: functions, - }; - }); - return classes; + const workspaceTestCaps = + client.initializeResult?.capabilities.experimental[workspaceTestsRequest.method]; + + // Only use the lsp for this request if it supports the + // workspace/tests method, and is at least version 2. + if (workspaceTestCaps?.version >= 2) { + const tests = await client.sendRequest(workspaceTestsRequest, {}, token); + const testsInWorkspace = tests.filter(item => + isPathInsidePath( + client.protocol2CodeConverter.asLocation(item.location).uri.fsPath, + workspaceRoot.fsPath + ) + ); + + return this.transform(client, swiftPackage, testsInWorkspace); + } else { + throw new Error("workspace/tests requests not supported"); + } }); } + + /** + * Convert from a collection of LSP TestItems to a collection of + * TestDiscovery.TestClasses, updating the format of the location. + */ + private transform( + client: LanguageClient, + swiftPackage: SwiftPackage, + input: LSPTestItem[] + ): TestDiscovery.TestClass[] { + return input.map(item => { + const location = client.protocol2CodeConverter.asLocation(item.location); + const id = this.transformId(item, location, swiftPackage); + return { + ...item, + id: id, + location: location, + children: this.transform(client, swiftPackage, item.children), + }; + }); + } + + /** + * If the test is an XCTest, transform the ID provided by the LSP from a + * swift-testing style ID to one that XCTest can use. This allows the ID to + * be used to specify to the test runner (xctest or swift-testing) which tests to run. + */ + private transformId( + item: LSPTestItem, + location: vscode.Location, + swiftPackage: SwiftPackage + ): string { + // XCTest: Target.TestClass/testCase + // swift-testing: TestClass/testCase() + // TestClassOrStruct/NestedTestSuite/testCase() + + let id: string = item.id; + if (item.style === "XCTest") { + const target = swiftPackage + .getTargets(TargetType.test) + .find(target => swiftPackage.getTarget(location.uri.fsPath) === target); + + id = ""; + if (target) { + id += `${target?.name}.`; + } + return id + item.id.replace(/\(\)$/, ""); + } else { + return item.id; + } + } } diff --git a/src/TestExplorer/SPMTestDiscovery.ts b/src/TestExplorer/SPMTestDiscovery.ts new file mode 100644 index 000000000..8d32f01d1 --- /dev/null +++ b/src/TestExplorer/SPMTestDiscovery.ts @@ -0,0 +1,84 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the VSCode Swift open source project +// +// Copyright (c) 2021-2024 the VSCode Swift project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of VSCode Swift project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +import { TestStyle } from "../sourcekit-lsp/lspExtensions"; +import { TestClass } from "./TestDiscovery"; + +/* + * Build an array of TestClasses from test list output by `swift test list` + */ +export function parseTestsFromSwiftTestListOutput(input: string): TestClass[] { + const tests = new Array(); + const lines = input.match(/[^\r\n]+/g); + if (!lines) { + return tests; + } + + for (const line of lines) { + let targetName: string | undefined; + let testName: string | undefined; + let style: TestStyle = "XCTest"; + + // Regex "./" + const xcTestGroup = /^([\w\d_]*)\.([\w\d_]*)\/(.*)$/.exec(line); + if (xcTestGroup) { + targetName = xcTestGroup[1]; + testName = `${xcTestGroup[2]}/${xcTestGroup[3]}`; + style = "XCTest"; + } + + // Regex "." + const swiftTestGroup = /^([\w\d_]*)\.(.*\(.*\))$/.exec(line); + if (swiftTestGroup) { + targetName = swiftTestGroup[1]; + testName = swiftTestGroup[2]; + style = "swift-testing"; + } + + if (!testName || !targetName) { + continue; + } + + const components = [targetName, ...testName.split("/")]; + let separator = "."; + // Walk the components of the fully qualified name, adding any missing nodes in the tree + // as we encounter them, and adding to the children of existing nodes. + components.reduce( + ({ tests, currentId }, component) => { + const id = currentId ? `${currentId}${separator}${component}` : component; + if (currentId) { + separator = "/"; // separator starts as . after the tartget name, then switches to / for suites. + } + + const testStyle: TestStyle = id === targetName ? "test-target" : style; + let target = tests.find(item => item.id === id); + if (!target) { + target = { + id, + label: component, + location: undefined, + style, + children: [], + disabled: false, + tags: [{ id: testStyle }], + }; + tests.push(target); + } + return { tests: target.children, currentId: id }; + }, + { tests, currentId: undefined as undefined | string } + ); + } + return tests; +} diff --git a/src/TestExplorer/TestDiscovery.ts b/src/TestExplorer/TestDiscovery.ts index 29cba244c..06c01004a 100644 --- a/src/TestExplorer/TestDiscovery.ts +++ b/src/TestExplorer/TestDiscovery.ts @@ -2,7 +2,7 @@ // // This source file is part of the VSCode Swift open source project // -// Copyright (c) 2024 the VSCode Swift project authors +// Copyright (c) 2021-2024 the VSCode Swift project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -15,26 +15,12 @@ import * as vscode from "vscode"; import { FolderContext } from "../FolderContext"; import { TargetType } from "../SwiftPackage"; - -/** Test function definition */ -export interface TestFunction { - name: string; - location?: vscode.Location; -} +import { LSPTestItem } from "../sourcekit-lsp/lspExtensions"; /** Test class definition */ -export interface TestClass { - name: string; - location?: vscode.Location; - extension?: boolean; - functions: TestFunction[]; -} - -/** Test target definition */ -export interface TestTarget { - name: string; - folder?: vscode.Uri; - classes: TestClass[]; +export interface TestClass extends Omit, "children"> { + location: vscode.Location | undefined; + children: TestClass[]; } /** @@ -45,22 +31,26 @@ export interface TestTarget { * @param folderContext Folder test classes came * @param testClasses Array of test classes */ -export function updateTestsFromClasses(folderContext: FolderContext, testClasses: TestClass[]) { +export function updateTestsFromClasses(folderContext: FolderContext, testItems: TestClass[]) { const testExplorer = folderContext.testExplorer; if (!testExplorer) { return; } const targets = folderContext.swiftPackage.getTargets(TargetType.test).map(target => { - const classes = testClasses.filter( - testClass => - testClass.location && - folderContext.swiftPackage.getTarget(testClass.location.uri.fsPath) === target + const filteredItems = testItems.filter( + testItem => + testItem.location && + folderContext.swiftPackage.getTarget(testItem.location.uri.fsPath) === target ); return { - name: target.name, - folder: vscode.Uri.file(target.path), - classes: classes, - }; + id: target.name, + label: target.name, + children: filteredItems, + location: undefined, + disabled: false, + style: "test-target", + tags: [], + } as TestClass; }); updateTests(testExplorer.controller, targets); } @@ -68,130 +58,101 @@ export function updateTestsFromClasses(folderContext: FolderContext, testClasses /** * Update Test Controller TestItems based off array of TestTargets * @param testController Test controller - * @param testTargets Array of TestTargets + * @param testItems Array of TestClasses * @param filterFile Filter test deletion just for tests in the one file */ export function updateTests( testController: vscode.TestController, - testTargets: TestTarget[], + testItems: TestClass[], filterFile?: vscode.Uri ) { - // remove TestItems that aren't in testTarget list - testController.items.forEach(targetItem => { - const testTarget = testTargets.find(item => item.name === targetItem.id); - if (testTarget) { - const targetId = targetItem.id; - targetItem.children.forEach(classItem => { - const testClass = testTarget.classes.find( - item => `${targetId}.${item.name}` === classItem.id - ); - if (testClass) { - const classId = classItem.id; - classItem.children.forEach(functionItem => { - // if we are filtering based on targets being one file and this - // function isn't in the file then ignore - if (filterFile && functionItem.uri?.fsPath !== filterFile.fsPath) { - return; - } - const testFunction = testClass.functions.find( - item => `${classId}/${item.name}` === functionItem.id - ); - if (!testFunction) { - classItem.children.delete(functionItem.id); - } - }); - if (classItem.children.size === 0) { - targetItem.children.delete(classItem.id); - } - } else { - if (!filterFile) { - targetItem.children.delete(classItem.id); - } else if (classItem.uri?.fsPath === filterFile.fsPath) { - // If filtering on a file and a class is in that file and all its - // functions are in that file then delete the class - let allInFilteredFile = true; - classItem.children.forEach(func => { - if (func.uri?.fsPath !== filterFile.fsPath) { - allInFilteredFile = false; - } - }); - if (allInFilteredFile) { - targetItem.children.delete(classItem.id); - } - } - } - }); - if (targetItem.children.size === 0) { - testController.items.delete(targetItem.id); - } - } else if (!filterFile) { - testController.items.delete(targetItem.id); + const incomingTestsLookup = createIncomingTestLookup(testItems); + function removeOldTests(testItem: vscode.TestItem) { + testItem.children.forEach(child => removeOldTests(child)); + + // If the existing item isn't in the map + if ( + !incomingTestsLookup.get(testItem.id) && + (!filterFile || testItem.uri?.fsPath === filterFile.fsPath) + ) { + // TODO: Remove + console.log(`Removing test: ${testItem.id}`); + (testItem.parent ? testItem.parent.children : testController.items).delete(testItem.id); } + } + + // Skip removing tests if the test explorer is empty + if (testController.items.size !== 0) { + testController.items.forEach(removeOldTests); + } + + // Add/update the top level test items. upsertTestItem will decend the tree of children adding them as well. + testItems.forEach(testItem => { + upsertTestItem(testController, testItem); }); - // Add in new items, update items already in place - testTargets.forEach(testTarget => { - const targetItem = - testController.items.get(testTarget.name) ?? - createTopLevelTestItem(testController, testTarget.name, testTarget.folder); - testTarget.classes.forEach(testClass => { - const classItem = updateChildTestItem( - testController, - targetItem, - testClass.name, - ".", - testClass.extension !== true, - testClass.location - ); - if (classItem) { - testClass.functions.forEach(testFunction => { - updateChildTestItem( - testController, - classItem, - testFunction.name, - "/", - true, - testFunction.location - ); - }); - } - }); + testController.items.forEach(item => { + testController.items.add(item); }); } -/** Create top level test item and add it to the test controller */ -function createTopLevelTestItem( - testController: vscode.TestController, - name: string, - uri?: vscode.Uri -): vscode.TestItem { - const testItem = testController.createTestItem(name, name, uri); - testController.items.add(testItem); - return testItem; +/** + * Create a lookup of the incoming tests we can compare to the existing list of tests + * to produce a list of tests that are no longer present. If a filterFile is specified we + * scope this work to just the tests inside that file. + */ +function createIncomingTestLookup( + collection: TestClass[], + filterFile?: vscode.Uri +): Map { + const dictionary = new Map(); + function traverse(testItem: TestClass) { + // If we are filtering based on tests being one file and this + // function isn't in the file then ignore + if (!filterFile || testItem.location?.uri.fsPath === filterFile.fsPath) { + dictionary.set(testItem.id, testItem); + testItem.children.forEach(item => traverse(item)); + } + } + collection.forEach(item => traverse(item)); + return dictionary; } -/** Update a child test item or if it doesn't exist create a new test item */ -function updateChildTestItem( +/** + * Updates the existing vscode.TestItem if it exists with the same ID as the VSCodeLSPTestItem, + * otherwise creates an add a new one. The location on the returned vscode.TestItem is always updated. + */ +function upsertTestItem( testController: vscode.TestController, - parent: vscode.TestItem, - name: string, - separator: string, - addNewItem: boolean, - location?: vscode.Location -): vscode.TestItem | undefined { - const id = `${parent.id}${separator}${name}`; - const testItem = parent.children.get(id); - if (testItem) { - if (testItem.uri?.fsPath === location?.uri.fsPath || location === undefined) { - testItem.range = location?.range; - return testItem; - } - parent.children.delete(testItem.id); - } else if (addNewItem === false) { - return undefined; + testItem: TestClass, + parent?: vscode.TestItem +) { + // This is a temporary gate on adding swift-testing tests until there is code to + // run them. + if (testItem.style === "swift-testing") { + return; } - const newTestItem = testController.createTestItem(id, name, location?.uri); - newTestItem.range = location?.range; - parent.children.add(newTestItem); - return newTestItem; + + const collection = parent === undefined ? testController.items : parent.children; + + const newItem = + collection.get(testItem.id) ?? + testController.createTestItem(testItem.id, testItem.label, testItem.location?.uri); + + // The leading `.` is part of the tag name when parsed by sourcekit-lsp. Strip + // it off for better UX when filtering by tag. + const tags = testItem.tags.map(tag => ({ ...tag, id: tag.id.replace(/^\./, "") })); + + // Manually add the test style as a tag so we can filter by test type. + newItem.tags = [{ id: testItem.style }, ...tags]; + newItem.label = testItem.label; + newItem.range = testItem.location?.range; + + // Performs an upsert based on whether a test item exists in the collection with the same id. + // If no parent is provided operate on the testController's root items. + collection.add(newItem); + + testItem.children.forEach(child => { + upsertTestItem(testController, child, newItem); + }); } diff --git a/src/TestExplorer/TestExplorer.ts b/src/TestExplorer/TestExplorer.ts index 3ed6da08b..db652eef2 100644 --- a/src/TestExplorer/TestExplorer.ts +++ b/src/TestExplorer/TestExplorer.ts @@ -2,7 +2,7 @@ // // This source file is part of the VSCode Swift open source project // -// Copyright (c) 2021-2022 the VSCode Swift project authors +// Copyright (c) 2021-2024 the VSCode Swift project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -24,6 +24,8 @@ import { buildOptions, getBuildAllTask } from "../SwiftTaskProvider"; import { SwiftExecOperation, TaskOperation } from "../TaskQueue"; import * as TestDiscovery from "./TestDiscovery"; import { TargetType } from "../SwiftPackage"; +import { parseTestsFromSwiftTestListOutput } from "./SPMTestDiscovery"; +import { parseTestsFromDocumentSymbols } from "./DocumentSymbolTestDiscovery"; /** Build test explorer UI */ export class TestExplorer { @@ -147,18 +149,27 @@ export class TestExplorer { if (isPathInsidePath(uri.fsPath, folder.folder.fsPath)) { const target = folder.swiftPackage.getTarget(uri.fsPath); if (target && target.type === "test") { - const tests = testExplorer.lspTestDiscovery.getTests(symbols, uri); - TestDiscovery.updateTests( - testExplorer.controller, - [ - { - name: target.name, - folder: vscode.Uri.file(target.path), - classes: tests, - }, - ], - uri - ); + testExplorer.lspTestDiscovery + .getDocumentTests(folder.swiftPackage, uri) + .then( + tests => + [ + { + id: target.name, + label: target.name, + children: tests, + location: undefined, + disabled: false, + style: "test-target", + tags: [], + }, + ] as TestDiscovery.TestClass[] + ) + // Fallback to parsing document symbols for XCTests only + .catch(() => parseTestsFromDocumentSymbols(target.name, symbols, uri)) + .then(tests => { + TestDiscovery.updateTests(testExplorer.controller, tests, uri); + }); } } } @@ -168,16 +179,17 @@ export class TestExplorer { * Discover tests */ async discoverTestsInWorkspace() { - //const toolchain = this.folderContext.workspaceContext.toolchain; - //if (toolchain.swiftVersion.isLessThan(new Version(5, 11, 0))) { - await this.discoverTestsInWorkspaceSPM(); - /*} else { - try { - await this.discoverTestsInWorkspaceLSP(); - } catch { - await this.discoverTestsInWorkspaceSPM(); - } - }*/ + try { + // If the LSP cannot produce a list of tests it throws and + // we fall back to discovering tests with SPM. + await this.discoverTestsInWorkspaceLSP(); + } catch { + this.folderContext.workspaceContext.outputChannel.logDiagnostic( + "workspace/tests LSP request not supported, falling back to SPM to discover tests.", + "Test Discovery" + ); + await this.discoverTestsInWorkspaceSPM(); + } } /** @@ -185,97 +197,96 @@ export class TestExplorer { * Uses `swift test --list-tests` to get the list of tests */ async discoverTestsInWorkspaceSPM() { - try { - const toolchain = this.folderContext.workspaceContext.toolchain; - // get build options before build is run so we can be sure they aren't changed - // mid-build - const testBuildOptions = buildOptions(toolchain); - // normally we wouldn't run the build here, but you can hang swiftPM on macOS - // if you try and list tests while skipping the build if you are using a different - // sanitizer settings - if (process.platform === "darwin" && configuration.sanitizer !== "off") { - const task = await getBuildAllTask(this.folderContext); - task.definition.dontTriggerTestDiscovery = true; - const exitCode = await this.folderContext.taskQueue.queueOperation( - new TaskOperation(task) - ); - if (exitCode === undefined || exitCode !== 0) { - this.setErrorTestItem("Build the project to enable test discovery."); - return; + async function runDiscover(explorer: TestExplorer, firstTry: boolean) { + try { + const toolchain = explorer.folderContext.workspaceContext.toolchain; + // get build options before build is run so we can be sure they aren't changed + // mid-build + const testBuildOptions = buildOptions(toolchain); + // normally we wouldn't run the build here, but you can hang swiftPM on macOS + // if you try and list tests while skipping the build if you are using a different + // sanitizer settings + if (process.platform === "darwin" && configuration.sanitizer !== "off") { + const task = await getBuildAllTask(explorer.folderContext); + task.definition.dontTriggerTestDiscovery = true; + const exitCode = await explorer.folderContext.taskQueue.queueOperation( + new TaskOperation(task) + ); + if (exitCode === undefined || exitCode !== 0) { + explorer.setErrorTestItem("Build the project to enable test discovery."); + return; + } } - } - // get list of tests from `swift test --list-tests` - let listTestArguments: string[]; - if (toolchain.swiftVersion.isGreaterThanOrEqual(new Version(5, 8, 0))) { - listTestArguments = ["test", "list", "--skip-build"]; - } else { - listTestArguments = ["test", "--list-tests", "--skip-build"]; - } - listTestArguments = [...listTestArguments, ...testBuildOptions]; - const listTestsOperation = new SwiftExecOperation( - listTestArguments, - this.folderContext, - "Listing Tests", - { showStatusItem: true, checkAlreadyRunning: false, log: "Listing tests" }, - stdout => { - // if we got to this point we can get rid of any error test item - this.deleteErrorTestItem(); + // get list of tests from `swift test --list-tests` + let listTestArguments: string[]; + if (toolchain.swiftVersion.isGreaterThanOrEqual(new Version(5, 8, 0))) { + listTestArguments = ["test", "list", "--skip-build"]; + } else { + listTestArguments = ["test", "--list-tests", "--skip-build"]; + } + listTestArguments = [...listTestArguments, ...testBuildOptions]; + const listTestsOperation = new SwiftExecOperation( + listTestArguments, + explorer.folderContext, + "Listing Tests", + { showStatusItem: true, checkAlreadyRunning: false, log: "Listing tests" }, + stdout => { + // if we got to this point we can get rid of any error test item + explorer.deleteErrorTestItem(); - const lines = stdout.match(/[^\r\n]+/g); - if (!lines) { - return; + const tests = parseTestsFromSwiftTestListOutput(stdout); + TestDiscovery.updateTests(explorer.controller, tests); } + ); + await explorer.folderContext.taskQueue.queueOperation(listTestsOperation); + } catch (error) { + // If a test list fails its possible the tests have not been built. + // Build them and try again, and if we still fail then notify the user. + if (firstTry) { + await new Promise(resolve => { + const swiftBuildOperation = new SwiftExecOperation( + ["build", "--build-tests"], + explorer.folderContext, + "Building", + { + showStatusItem: true, + checkAlreadyRunning: true, + log: "Performing initial build", + }, + resolve + ); + explorer.folderContext.taskQueue.queueOperation(swiftBuildOperation); + }); - // Build target array from test list output by `swift test list` - const targets = new Array(); - for (const line of lines) { - // Regex "./" - const groups = /^([\w\d_]*)\.([\w\d_]*)\/(.*)$/.exec(line); - if (!groups) { - continue; - } - const targetName = groups[1]; - const className = groups[2]; - const funcName = groups[3]; - let target = targets.find(item => item.name === targetName); - if (!target) { - target = { name: targetName, folder: undefined, classes: [] }; - targets.push(target); - } - let testClass = target.classes.find(item => item.name === className); - if (!testClass) { - testClass = { name: className, location: undefined, functions: [] }; - target.classes.push(testClass); - } - const testFunc = { name: funcName, location: undefined }; - testClass.functions.push(testFunc); + // Retry test discovery after performing a build. + await runDiscover(explorer, false); + } else { + const errorDescription = getErrorDescription(error); + if ( + (process.platform === "darwin" && + errorDescription.match(/error: unableToLoadBundle/)) || + (process.platform === "win32" && + errorDescription.match(/The file doesn’t exist./)) || + (!["darwin", "win32"].includes(process.platform) && + errorDescription.match(/No such file or directory/)) + ) { + explorer.setErrorTestItem("Build the project to enable test discovery."); + } else if (errorDescription.startsWith("error: no tests found")) { + explorer.setErrorTestItem( + "Add a test target to your Package.", + "No Tests Found." + ); + } else { + explorer.setErrorTestItem(errorDescription); } - // Update tests from target array - TestDiscovery.updateTests(this.controller, targets); + explorer.folderContext.workspaceContext.outputChannel.log( + `Test Discovery Failed: ${errorDescription}`, + explorer.folderContext.name + ); } - ); - await this.folderContext.taskQueue.queueOperation(listTestsOperation); - } catch (error) { - const errorDescription = getErrorDescription(error); - if ( - (process.platform === "darwin" && - errorDescription.match(/error: unableToLoadBundle/)) || - (process.platform === "win32" && - errorDescription.match(/The file doesn’t exist./)) || - (!["darwin", "win32"].includes(process.platform) && - errorDescription.match(/No such file or directory/)) - ) { - this.setErrorTestItem("Build the project to enable test discovery."); - } else if (errorDescription.startsWith("error: no tests found")) { - this.setErrorTestItem("Add a test target to your Package.", "No Tests Found."); - } else { - this.setErrorTestItem(errorDescription); } - this.folderContext.workspaceContext.outputChannel.log( - `Test Discovery Failed: ${errorDescription}`, - this.folderContext.name - ); } + await runDiscover(this, true); } /** @@ -283,6 +294,7 @@ export class TestExplorer { */ async discoverTestsInWorkspaceLSP() { const tests = await this.lspTestDiscovery.getWorkspaceTests( + this.folderContext.swiftPackage, this.folderContext.workspaceFolder.uri ); TestDiscovery.updateTestsFromClasses(this.folderContext, tests); diff --git a/src/sourcekit-lsp/LanguageClientManager.ts b/src/sourcekit-lsp/LanguageClientManager.ts index 18935b9dc..e0ac81b15 100644 --- a/src/sourcekit-lsp/LanguageClientManager.ts +++ b/src/sourcekit-lsp/LanguageClientManager.ts @@ -2,7 +2,7 @@ // // This source file is part of the VSCode Swift open source project // -// Copyright (c) 2021-2023 the VSCode Swift project authors +// Copyright (c) 2021-2024 the VSCode Swift project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -95,6 +95,7 @@ export class LanguageClientManager { * null means in the process of restarting */ private languageClient: langclient.LanguageClient | null | undefined; + private initializeResult?: langclient.InitializeResult; private cancellationToken?: vscode.CancellationTokenSource; private legacyInlayHints?: vscode.Disposable; private restartedPromise?: Promise; @@ -509,6 +510,7 @@ export class LanguageClientManager { if (this.workspaceContext.swiftVersion.isLessThan(new Version(5, 7, 0))) { this.legacyInlayHints = activateLegacyInlayHints(client); } + this.initializeResult = client.initializeResult; }) .catch(reason => { this.workspaceContext.outputChannel.log(`${reason}`); diff --git a/src/sourcekit-lsp/lspExtensions.ts b/src/sourcekit-lsp/lspExtensions.ts index 01ecd5281..0724dda8e 100644 --- a/src/sourcekit-lsp/lspExtensions.ts +++ b/src/sourcekit-lsp/lspExtensions.ts @@ -2,7 +2,7 @@ // // This source file is part of the VSCode Swift open source project // -// Copyright (c) 2021 the VSCode Swift project authors +// Copyright (c) 2021-2024 the VSCode Swift project authors // Licensed under Apache License v2.0 // // See LICENSE.txt for license information @@ -12,6 +12,7 @@ // //===----------------------------------------------------------------------===// +import * as ls from "vscode-languageserver-protocol"; import * as langclient from "vscode-languageclient/node"; // Definitions for non-standard requests used by sourcekit-lsp @@ -62,11 +63,75 @@ export const legacyInlayHintsRequest = new langclient.RequestType< unknown >("sourcekit-lsp/inlayHints"); +// Test styles where test-target represents a test target that contains tests +export type TestStyle = "XCTest" | "swift-testing" | "test-target"; + // Listing tests -//export interface WorkspaceTestsParams {} +export interface LSPTestItem { + /** + * This identifier uniquely identifies the test case or test suite. It can be used to run an individual test (suite). + */ + id: string; + + /** + * Display name describing the test. + */ + label: string; + + /** + * Optional description that appears next to the label. + */ + description?: string; + + /** + * A string that should be used when comparing this item with other items. + * + * When `undefined` the `label` is used. + */ + sortText?: string; + + /** + * Whether the test is disabled. + */ + disabled: boolean; + + /** + * The type of test, eg. the testing framework that was used to declare the test. + */ + style: TestStyle; + + /** + * The location of the test item in the source code. + */ + location: ls.Location; + + /** + * The children of this test item. + * + * For a test suite, this may contain the individual test cases or nested suites. + */ + children: [LSPTestItem]; + + /** + * Tags associated with this test item. + */ + tags: { id: string }[]; +} export const workspaceTestsRequest = new langclient.RequestType< Record, - langclient.SymbolInformation[], + LSPTestItem[], unknown >("workspace/tests"); + +interface DocumentTestsParams { + textDocument: { + uri: ls.URI; + }; +} + +export const textDocumentTestsRequest = new langclient.RequestType< + DocumentTestsParams, + LSPTestItem[], + unknown +>("textDocument/tests"); diff --git a/test/suite/testexplorer/DocumentSymbolTestDiscovery.test.ts b/test/suite/testexplorer/DocumentSymbolTestDiscovery.test.ts new file mode 100644 index 000000000..be0ae1600 --- /dev/null +++ b/test/suite/testexplorer/DocumentSymbolTestDiscovery.test.ts @@ -0,0 +1,101 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the VSCode Swift open source project +// +// Copyright (c) 2021-2024 the VSCode Swift project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of VSCode Swift project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +import * as assert from "assert"; +import * as vscode from "vscode"; +import { parseTestsFromDocumentSymbols } from "../../../src/TestExplorer/DocumentSymbolTestDiscovery"; +import { TestClass } from "../../../src/TestExplorer/TestDiscovery"; + +suite("DocumentSymbolTestDiscovery Suite", () => { + const mockRange = new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 0)); + const mockUri = vscode.Uri.file("file:///var/foo"); + const basicXCTest: TestClass = { + id: "", + label: "", + disabled: false, + style: "XCTest", + location: { + range: mockRange, + uri: mockUri, + }, + children: [], + tags: [{ id: "XCTest" }], + }; + + test("Parse empty document symbols", async () => { + const tests = parseTestsFromDocumentSymbols("TestTarget", [], mockUri); + assert.deepEqual(tests, []); + }); + + test("Parse empty test suite", async () => { + const symbols = [ + new vscode.DocumentSymbol( + "MyXCTestCase", + "", + vscode.SymbolKind.Class, + mockRange, + mockRange + ), + ]; + + const tests = parseTestsFromDocumentSymbols("TestTarget", symbols, mockUri); + assert.deepEqual(tests, []); + }); + + test("Parse suite with one test", async () => { + const testClass = new vscode.DocumentSymbol( + "MyXCTestCase", + "", + vscode.SymbolKind.Class, + mockRange, + mockRange + ); + testClass.children = [ + new vscode.DocumentSymbol( + "testFoo()", + "", + vscode.SymbolKind.Method, + mockRange, + mockRange + ), + ]; + + const tests = parseTestsFromDocumentSymbols("TestTarget", [testClass], mockUri); + assert.deepEqual(tests, [ + { + ...basicXCTest, + id: "TestTarget", + label: "TestTarget", + location: undefined, + tags: [{ id: "test-target" }], + children: [ + { + ...basicXCTest, + id: "TestTarget.MyXCTestCase", + label: "MyXCTestCase", + tags: [{ id: "XCTest" }], + children: [ + { + ...basicXCTest, + id: "TestTarget.MyXCTestCase/testFoo", + label: "testFoo", + tags: [{ id: "XCTest" }], + }, + ], + }, + ], + }, + ]); + }); +}); diff --git a/test/suite/testexplorer/SPMTestListOutputParser.test.ts b/test/suite/testexplorer/SPMTestListOutputParser.test.ts new file mode 100644 index 000000000..de4d62262 --- /dev/null +++ b/test/suite/testexplorer/SPMTestListOutputParser.test.ts @@ -0,0 +1,216 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the VSCode Swift open source project +// +// Copyright (c) 2021-2024 the VSCode Swift project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of VSCode Swift project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +import * as assert from "assert"; +import { parseTestsFromSwiftTestListOutput } from "../../../src/TestExplorer/SPMTestDiscovery"; +import { TestClass } from "../../../src/TestExplorer/TestDiscovery"; + +suite("SPMTestListOutputParser Suite", () => { + const basicXCTest: TestClass = { + id: "", + label: "", + disabled: false, + style: "XCTest", + location: undefined, + children: [], + tags: [{ id: "XCTest" }], + }; + + const basicSwiftTestingTest: TestClass = { + ...basicXCTest, + style: "swift-testing", + tags: [{ id: "swift-testing" }], + }; + + test("Parse single XCTest", async () => { + const tests = parseTestsFromSwiftTestListOutput("TestTarget.XCTestSuite/testXCTest"); + assert.deepEqual(tests, [ + { + ...basicXCTest, + id: "TestTarget", + label: "TestTarget", + tags: [{ id: "test-target" }], + children: [ + { + ...basicXCTest, + id: "TestTarget.XCTestSuite", + label: "XCTestSuite", + children: [ + { + ...basicXCTest, + id: "TestTarget.XCTestSuite/testXCTest", + label: "testXCTest", + }, + ], + }, + ], + }, + ]); + }); + + test("Parse multiple XCTests", async () => { + const tests = parseTestsFromSwiftTestListOutput(` +TestTarget.XCTestSuite/testXCTest +TestTarget.XCTestSuite/testAnotherXCTest +`); + assert.deepEqual(tests, [ + { + ...basicXCTest, + id: "TestTarget", + label: "TestTarget", + tags: [{ id: "test-target" }], + children: [ + { + ...basicXCTest, + id: "TestTarget.XCTestSuite", + label: "XCTestSuite", + children: [ + { + ...basicXCTest, + id: "TestTarget.XCTestSuite/testXCTest", + label: "testXCTest", + }, + { + ...basicXCTest, + id: "TestTarget.XCTestSuite/testAnotherXCTest", + label: "testAnotherXCTest", + }, + ], + }, + ], + }, + ]); + }); + + test("Parse one of each style", async () => { + const tests = parseTestsFromSwiftTestListOutput(` +TestTarget.XCTestSuite/testXCTest +TestTarget.testSwiftTest() +`); + assert.deepEqual(tests, [ + { + ...basicXCTest, + id: "TestTarget", + label: "TestTarget", + tags: [{ id: "test-target" }], + children: [ + { + ...basicXCTest, + id: "TestTarget.XCTestSuite", + label: "XCTestSuite", + children: [ + { + ...basicXCTest, + id: "TestTarget.XCTestSuite/testXCTest", + label: "testXCTest", + }, + ], + }, + { + ...basicSwiftTestingTest, + id: "TestTarget.testSwiftTest()", + label: "testSwiftTest()", + }, + ], + }, + ]); + }); + + test("Parse single top level swift testing test", async () => { + const tests = parseTestsFromSwiftTestListOutput("TestTarget.testSwiftTest()"); + assert.deepEqual(tests, [ + { + ...basicSwiftTestingTest, + id: "TestTarget", + label: "TestTarget", + tags: [{ id: "test-target" }], + children: [ + { + ...basicSwiftTestingTest, + id: "TestTarget.testSwiftTest()", + label: "testSwiftTest()", + }, + ], + }, + ]); + }); + + test("Parse multiple top level swift testing tests", async () => { + const tests = parseTestsFromSwiftTestListOutput(` +TestTarget.testSwiftTest() +TestTarget.testAnotherSwiftTest() +`); + assert.deepEqual(tests, [ + { + ...basicSwiftTestingTest, + id: "TestTarget", + label: "TestTarget", + tags: [{ id: "test-target" }], + children: [ + { + ...basicSwiftTestingTest, + id: "TestTarget.testSwiftTest()", + label: "testSwiftTest()", + }, + { + ...basicSwiftTestingTest, + id: "TestTarget.testAnotherSwiftTest()", + label: "testAnotherSwiftTest()", + }, + ], + }, + ]); + }); + + test("Parse nested swift testing tests", async () => { + const tests = parseTestsFromSwiftTestListOutput(` +TestTarget.RootSuite/NestedSuite/nestedTestInASuite() +TestTarget.RootSuite/aTestInASuite() +`); + assert.deepEqual(tests, [ + { + ...basicSwiftTestingTest, + id: "TestTarget", + label: "TestTarget", + tags: [{ id: "test-target" }], + children: [ + { + ...basicSwiftTestingTest, + id: "TestTarget.RootSuite", + label: "RootSuite", + children: [ + { + ...basicSwiftTestingTest, + id: "TestTarget.RootSuite/NestedSuite", + label: "NestedSuite", + children: [ + { + ...basicSwiftTestingTest, + id: "TestTarget.RootSuite/NestedSuite/nestedTestInASuite()", + label: "nestedTestInASuite()", + }, + ], + }, + { + ...basicSwiftTestingTest, + id: "TestTarget.RootSuite/aTestInASuite()", + label: "aTestInASuite()", + }, + ], + }, + ], + }, + ]); + }); +}); diff --git a/test/suite/TestOutputParser.test.ts b/test/suite/testexplorer/TestOutputParser.test.ts similarity index 99% rename from test/suite/TestOutputParser.test.ts rename to test/suite/testexplorer/TestOutputParser.test.ts index 90b1ee433..0d3f5bee2 100644 --- a/test/suite/TestOutputParser.test.ts +++ b/test/suite/testexplorer/TestOutputParser.test.ts @@ -18,7 +18,7 @@ import { iTestRunState, nonDarwinTestRegex, TestOutputParser, -} from "../../src/TestExplorer/TestOutputParser"; +} from "../../../src/TestExplorer/TestOutputParser"; /** TestStatus */ export enum TestStatus { From 1888d4bcaff87c79148dc878fc36dd9755d218c6 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Wed, 24 Apr 2024 13:35:16 -0400 Subject: [PATCH 2/9] Update comments --- src/TestExplorer/LSPTestDiscovery.ts | 8 ++++---- src/TestExplorer/TestDiscovery.ts | 9 ++++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/TestExplorer/LSPTestDiscovery.ts b/src/TestExplorer/LSPTestDiscovery.ts index e41f0fc61..d605e0aee 100644 --- a/src/TestExplorer/LSPTestDiscovery.ts +++ b/src/TestExplorer/LSPTestDiscovery.ts @@ -57,7 +57,7 @@ export class LSPTestDiscovery { ); return this.transform(client, swiftPackage, testsInDocument); } else { - throw new Error("workspace/tests requests not supported"); + throw new Error("textDocument/tests requests not supported"); } }); } @@ -93,8 +93,8 @@ export class LSPTestDiscovery { } /** - * Convert from a collection of LSP TestItems to a collection of - * TestDiscovery.TestClasses, updating the format of the location. + * Convert from `LSPTestItem[]` to `TestDiscovery.TestClass[]`, + * updating the format of the location. */ private transform( client: LanguageClient, @@ -116,7 +116,7 @@ export class LSPTestDiscovery { /** * If the test is an XCTest, transform the ID provided by the LSP from a * swift-testing style ID to one that XCTest can use. This allows the ID to - * be used to specify to the test runner (xctest or swift-testing) which tests to run. + * be used to tell to the test runner (xctest or swift-testing) which tests to run. */ private transformId( item: LSPTestItem, diff --git a/src/TestExplorer/TestDiscovery.ts b/src/TestExplorer/TestDiscovery.ts index 06c01004a..a79d40cbf 100644 --- a/src/TestExplorer/TestDiscovery.ts +++ b/src/TestExplorer/TestDiscovery.ts @@ -75,9 +75,8 @@ export function updateTests( !incomingTestsLookup.get(testItem.id) && (!filterFile || testItem.uri?.fsPath === filterFile.fsPath) ) { - // TODO: Remove - console.log(`Removing test: ${testItem.id}`); - (testItem.parent ? testItem.parent.children : testController.items).delete(testItem.id); + const collection = testItem.parent ? testItem.parent.children : testController.items; + collection.delete(testItem.id); } } @@ -119,7 +118,7 @@ function createIncomingTestLookup( } /** - * Updates the existing vscode.TestItem if it exists with the same ID as the VSCodeLSPTestItem, + * Updates the existing `vscode.TestItem` if it exists with the same ID as the `TestClass`, * otherwise creates an add a new one. The location on the returned vscode.TestItem is always updated. */ function upsertTestItem( @@ -128,7 +127,7 @@ function upsertTestItem( parent?: vscode.TestItem ) { // This is a temporary gate on adding swift-testing tests until there is code to - // run them. + // run them. See https://github.com/swift-server/vscode-swift/issues/757 if (testItem.style === "swift-testing") { return; } From 6ac7c0b40adefc33de71713f9fd0b441fc02b4cf Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Wed, 24 Apr 2024 13:53:54 -0400 Subject: [PATCH 3/9] Member accessed tags no longer have a leading . --- src/TestExplorer/TestDiscovery.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/TestExplorer/TestDiscovery.ts b/src/TestExplorer/TestDiscovery.ts index a79d40cbf..f1fac8336 100644 --- a/src/TestExplorer/TestDiscovery.ts +++ b/src/TestExplorer/TestDiscovery.ts @@ -138,12 +138,8 @@ function upsertTestItem( collection.get(testItem.id) ?? testController.createTestItem(testItem.id, testItem.label, testItem.location?.uri); - // The leading `.` is part of the tag name when parsed by sourcekit-lsp. Strip - // it off for better UX when filtering by tag. - const tags = testItem.tags.map(tag => ({ ...tag, id: tag.id.replace(/^\./, "") })); - // Manually add the test style as a tag so we can filter by test type. - newItem.tags = [{ id: testItem.style }, ...tags]; + newItem.tags = [{ id: testItem.style }, ...testItem.tags]; newItem.label = testItem.label; newItem.range = testItem.location?.range; From 5f1f42b50e0eebe1558f42f148d9cfe926cb4b5d Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Wed, 24 Apr 2024 15:34:34 -0400 Subject: [PATCH 4/9] Properly check if experimental lsp caps exist --- src/TestExplorer/LSPTestDiscovery.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/TestExplorer/LSPTestDiscovery.ts b/src/TestExplorer/LSPTestDiscovery.ts index d605e0aee..51a9b1c46 100644 --- a/src/TestExplorer/LSPTestDiscovery.ts +++ b/src/TestExplorer/LSPTestDiscovery.ts @@ -44,12 +44,15 @@ export class LSPTestDiscovery { document: vscode.Uri ): Promise { return await this.languageClient.useLanguageClient(async (client, token) => { - const workspaceTestCaps = - client.initializeResult?.capabilities.experimental[textDocumentTestsRequest.method]; + const workspaceTestCaps = client.initializeResult?.capabilities.experimental; + if (!workspaceTestCaps) { + throw new Error("textDocument/tests requests not supported"); + } + const textDocumentCaps = workspaceTestCaps[textDocumentTestsRequest.method]; // Only use the lsp for this request if it supports the // textDocument/tests method, and is at least version 2. - if (workspaceTestCaps?.version >= 2) { + if (textDocumentCaps?.version >= 2) { const testsInDocument = await client.sendRequest( textDocumentTestsRequest, { textDocument: { uri: document.toString() } }, @@ -71,12 +74,15 @@ export class LSPTestDiscovery { workspaceRoot: vscode.Uri ): Promise { return await this.languageClient.useLanguageClient(async (client, token) => { - const workspaceTestCaps = - client.initializeResult?.capabilities.experimental[workspaceTestsRequest.method]; + const workspaceTestCaps = client.initializeResult?.capabilities.experimental; + if (!workspaceTestCaps) { + throw new Error("textDocument/tests requests not supported"); + } + const workspaceTestsCaps = workspaceTestCaps[workspaceTestsRequest.method]; // Only use the lsp for this request if it supports the // workspace/tests method, and is at least version 2. - if (workspaceTestCaps?.version >= 2) { + if (workspaceTestsCaps?.version >= 2) { const tests = await client.sendRequest(workspaceTestsRequest, {}, token); const testsInWorkspace = tests.filter(item => isPathInsidePath( From f216f32f903b4120204b457fa9160264288d9af1 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Thu, 25 Apr 2024 17:04:18 -0400 Subject: [PATCH 5/9] Append target to swift-testing test ids --- src/TestExplorer/LSPTestDiscovery.ts | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/TestExplorer/LSPTestDiscovery.ts b/src/TestExplorer/LSPTestDiscovery.ts index 51a9b1c46..f1bacf4dc 100644 --- a/src/TestExplorer/LSPTestDiscovery.ts +++ b/src/TestExplorer/LSPTestDiscovery.ts @@ -132,20 +132,15 @@ export class LSPTestDiscovery { // XCTest: Target.TestClass/testCase // swift-testing: TestClass/testCase() // TestClassOrStruct/NestedTestSuite/testCase() + const target = swiftPackage + .getTargets(TargetType.test) + .find(target => swiftPackage.getTarget(location.uri.fsPath) === target); - let id: string = item.id; + const id = target !== undefined ? `${target.name}.${item.id}` : item.id; if (item.style === "XCTest") { - const target = swiftPackage - .getTargets(TargetType.test) - .find(target => swiftPackage.getTarget(location.uri.fsPath) === target); - - id = ""; - if (target) { - id += `${target?.name}.`; - } - return id + item.id.replace(/\(\)$/, ""); + return id.replace(/\(\)$/, ""); } else { - return item.id; + return id; } } } From c3bf0ebea5a0313eea8ba74297288ad17aef6f20 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Thu, 25 Apr 2024 17:17:06 -0400 Subject: [PATCH 6/9] Refactor out common LSP capability check --- src/TestExplorer/LSPTestDiscovery.ts | 37 ++++++++++++++++------------ 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/TestExplorer/LSPTestDiscovery.ts b/src/TestExplorer/LSPTestDiscovery.ts index f1bacf4dc..55ac4f927 100644 --- a/src/TestExplorer/LSPTestDiscovery.ts +++ b/src/TestExplorer/LSPTestDiscovery.ts @@ -44,15 +44,9 @@ export class LSPTestDiscovery { document: vscode.Uri ): Promise { return await this.languageClient.useLanguageClient(async (client, token) => { - const workspaceTestCaps = client.initializeResult?.capabilities.experimental; - if (!workspaceTestCaps) { - throw new Error("textDocument/tests requests not supported"); - } - const textDocumentCaps = workspaceTestCaps[textDocumentTestsRequest.method]; - // Only use the lsp for this request if it supports the // textDocument/tests method, and is at least version 2. - if (textDocumentCaps?.version >= 2) { + if (this.checkExperimentalCapability(client, textDocumentTestsRequest.method, 2)) { const testsInDocument = await client.sendRequest( textDocumentTestsRequest, { textDocument: { uri: document.toString() } }, @@ -60,7 +54,7 @@ export class LSPTestDiscovery { ); return this.transform(client, swiftPackage, testsInDocument); } else { - throw new Error("textDocument/tests requests not supported"); + throw new Error(`${textDocumentTestsRequest.method} requests not supported`); } }); } @@ -74,15 +68,9 @@ export class LSPTestDiscovery { workspaceRoot: vscode.Uri ): Promise { return await this.languageClient.useLanguageClient(async (client, token) => { - const workspaceTestCaps = client.initializeResult?.capabilities.experimental; - if (!workspaceTestCaps) { - throw new Error("textDocument/tests requests not supported"); - } - const workspaceTestsCaps = workspaceTestCaps[workspaceTestsRequest.method]; - // Only use the lsp for this request if it supports the // workspace/tests method, and is at least version 2. - if (workspaceTestsCaps?.version >= 2) { + if (this.checkExperimentalCapability(client, workspaceTestsRequest.method, 2)) { const tests = await client.sendRequest(workspaceTestsRequest, {}, token); const testsInWorkspace = tests.filter(item => isPathInsidePath( @@ -93,11 +81,28 @@ export class LSPTestDiscovery { return this.transform(client, swiftPackage, testsInWorkspace); } else { - throw new Error("workspace/tests requests not supported"); + throw new Error(`${workspaceTestsRequest.method} requests not supported`); } }); } + /** + * Returns `true` if the LSP supports the supplied `method` at or + * above the supplied `minVersion`. + */ + private checkExperimentalCapability( + client: LanguageClient, + method: string, + minVersion: number + ) { + const experimentalCapability = client.initializeResult?.capabilities.experimental; + if (!experimentalCapability) { + throw new Error(`${method} requests not supported`); + } + const targetCapability = experimentalCapability[method]; + return (targetCapability?.version ?? -1) >= minVersion; + } + /** * Convert from `LSPTestItem[]` to `TestDiscovery.TestClass[]`, * updating the format of the location. From eded282f162a85509f7d0c524cdc6425d1ce8c0d Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Mon, 29 Apr 2024 11:03:33 -0400 Subject: [PATCH 7/9] Rename transform to transformToTestClass --- src/TestExplorer/LSPTestDiscovery.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/TestExplorer/LSPTestDiscovery.ts b/src/TestExplorer/LSPTestDiscovery.ts index 55ac4f927..8224a5a1e 100644 --- a/src/TestExplorer/LSPTestDiscovery.ts +++ b/src/TestExplorer/LSPTestDiscovery.ts @@ -52,7 +52,7 @@ export class LSPTestDiscovery { { textDocument: { uri: document.toString() } }, token ); - return this.transform(client, swiftPackage, testsInDocument); + return this.transformToTestClass(client, swiftPackage, testsInDocument); } else { throw new Error(`${textDocumentTestsRequest.method} requests not supported`); } @@ -79,7 +79,7 @@ export class LSPTestDiscovery { ) ); - return this.transform(client, swiftPackage, testsInWorkspace); + return this.transformToTestClass(client, swiftPackage, testsInWorkspace); } else { throw new Error(`${workspaceTestsRequest.method} requests not supported`); } @@ -107,7 +107,7 @@ export class LSPTestDiscovery { * Convert from `LSPTestItem[]` to `TestDiscovery.TestClass[]`, * updating the format of the location. */ - private transform( + private transformToTestClass( client: LanguageClient, swiftPackage: SwiftPackage, input: LSPTestItem[] @@ -119,7 +119,7 @@ export class LSPTestDiscovery { ...item, id: id, location: location, - children: this.transform(client, swiftPackage, item.children), + children: this.transformToTestClass(client, swiftPackage, item.children), }; }); } From d587d8139e943ec3041f3b9f7dda7a2676aa24ee Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Mon, 29 Apr 2024 11:14:29 -0400 Subject: [PATCH 8/9] No need to filter workspace tests to the workspace --- src/TestExplorer/LSPTestDiscovery.ts | 15 ++------------- src/TestExplorer/TestDiscovery.ts | 2 +- src/TestExplorer/TestExplorer.ts | 3 +-- 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/src/TestExplorer/LSPTestDiscovery.ts b/src/TestExplorer/LSPTestDiscovery.ts index 8224a5a1e..9d137eeee 100644 --- a/src/TestExplorer/LSPTestDiscovery.ts +++ b/src/TestExplorer/LSPTestDiscovery.ts @@ -19,7 +19,6 @@ import { textDocumentTestsRequest, workspaceTestsRequest, } from "../sourcekit-lsp/lspExtensions"; -import { isPathInsidePath } from "../utilities/utilities"; import { LanguageClientManager } from "../sourcekit-lsp/LanguageClientManager"; import { LanguageClient } from "vscode-languageclient/node"; import { SwiftPackage, TargetType } from "../SwiftPackage"; @@ -63,23 +62,13 @@ export class LSPTestDiscovery { * Return list of workspace tests * @param workspaceRoot Root of current workspace folder */ - async getWorkspaceTests( - swiftPackage: SwiftPackage, - workspaceRoot: vscode.Uri - ): Promise { + async getWorkspaceTests(swiftPackage: SwiftPackage): Promise { return await this.languageClient.useLanguageClient(async (client, token) => { // Only use the lsp for this request if it supports the // workspace/tests method, and is at least version 2. if (this.checkExperimentalCapability(client, workspaceTestsRequest.method, 2)) { const tests = await client.sendRequest(workspaceTestsRequest, {}, token); - const testsInWorkspace = tests.filter(item => - isPathInsidePath( - client.protocol2CodeConverter.asLocation(item.location).uri.fsPath, - workspaceRoot.fsPath - ) - ); - - return this.transformToTestClass(client, swiftPackage, testsInWorkspace); + return this.transformToTestClass(client, swiftPackage, tests); } else { throw new Error(`${workspaceTestsRequest.method} requests not supported`); } diff --git a/src/TestExplorer/TestDiscovery.ts b/src/TestExplorer/TestDiscovery.ts index f1fac8336..b684330d1 100644 --- a/src/TestExplorer/TestDiscovery.ts +++ b/src/TestExplorer/TestDiscovery.ts @@ -85,7 +85,7 @@ export function updateTests( testController.items.forEach(removeOldTests); } - // Add/update the top level test items. upsertTestItem will decend the tree of children adding them as well. + // Add/update the top level test items. upsertTestItem will descend the tree of children adding them as well. testItems.forEach(testItem => { upsertTestItem(testController, testItem); }); diff --git a/src/TestExplorer/TestExplorer.ts b/src/TestExplorer/TestExplorer.ts index db652eef2..df9b8c75f 100644 --- a/src/TestExplorer/TestExplorer.ts +++ b/src/TestExplorer/TestExplorer.ts @@ -294,8 +294,7 @@ export class TestExplorer { */ async discoverTestsInWorkspaceLSP() { const tests = await this.lspTestDiscovery.getWorkspaceTests( - this.folderContext.swiftPackage, - this.folderContext.workspaceFolder.uri + this.folderContext.swiftPackage ); TestDiscovery.updateTestsFromClasses(this.folderContext, tests); } From 9b9b50531a114c0a73d9a7c5ef8741a2d9ea2167 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Mon, 29 Apr 2024 11:22:00 -0400 Subject: [PATCH 9/9] Cache caps checks --- src/TestExplorer/LSPTestDiscovery.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/TestExplorer/LSPTestDiscovery.ts b/src/TestExplorer/LSPTestDiscovery.ts index 9d137eeee..62535e08a 100644 --- a/src/TestExplorer/LSPTestDiscovery.ts +++ b/src/TestExplorer/LSPTestDiscovery.ts @@ -32,6 +32,8 @@ import { SwiftPackage, TargetType } from "../SwiftPackage"; * these results. */ export class LSPTestDiscovery { + private capCache = new Map(); + constructor(private languageClient: LanguageClientManager) {} /** @@ -84,12 +86,20 @@ export class LSPTestDiscovery { method: string, minVersion: number ) { + const capKey = `${method}:${minVersion}`; + const cachedCap = this.capCache.get(capKey); + if (cachedCap !== undefined) { + return cachedCap; + } + const experimentalCapability = client.initializeResult?.capabilities.experimental; if (!experimentalCapability) { throw new Error(`${method} requests not supported`); } const targetCapability = experimentalCapability[method]; - return (targetCapability?.version ?? -1) >= minVersion; + const canUse = (targetCapability?.version ?? -1) >= minVersion; + this.capCache.set(capKey, canUse); + return canUse; } /**