From e86e3808475ef1eb704e02df13218bb972b8b8a4 Mon Sep 17 00:00:00 2001 From: Steven Kabbes Date: Thu, 27 Mar 2025 15:40:21 -0700 Subject: [PATCH 1/7] implement test specifier configuration --- package.json | 46 +++++++ src/controller.ts | 33 +++-- src/extension.ts | 4 + src/parsing.ts | 119 +++++++++++++++--- src/test-function-specifier-config.ts | 20 +++ src/utils.ts | 11 ++ .../workspace/inRootFolderWrapped.test.mjs | 6 + .../otherFolder/otherFolderWrapped.test.mjs | 6 + .../workspace/test/inTestFolderWrapped.mjs | 6 + testCases/simple/workspace/test/utils.js | 5 + 10 files changed, 229 insertions(+), 27 deletions(-) create mode 100644 src/test-function-specifier-config.ts create mode 100644 src/utils.ts create mode 100644 testCases/simple/workspace/inRootFolderWrapped.test.mjs create mode 100644 testCases/simple/workspace/otherFolder/otherFolderWrapped.test.mjs create mode 100644 testCases/simple/workspace/test/inTestFolderWrapped.mjs create mode 100644 testCases/simple/workspace/test/utils.js diff --git a/package.json b/package.json index d785708..ae8482f 100644 --- a/package.json +++ b/package.json @@ -147,6 +147,52 @@ } } }, + "nodejs-testing.testSpecifiers": { + "type": "array", + "markdownDescription": "_Advanced_: A list of specifiers that indicate test function to search for:\nIt defaults to:\n\n```json\n[\n {\n \"import\": \"node:test\",\n \"name\": [\"default\", \"it\", \"test\", \"describe\", \"suite\"]\n }\n]\n```\n\nBut in case your test function is wrapped, you can specify it with a relative import:\nNOTE: relative imports must be prefixed with ./\n\n```json\n[\n {\n \"import\": \"./test/utils.js\",\n \"name\": \"test\"\n }\n]\n```\n", + "default": [ + { + "import": "node:test", + "name": [ + "default", + "it", + "test", + "describe", + "suite" + ] + } + ], + "items": { + "type": "object", + "default": { + "import": "node:test", + "name": [ + "default", + "it", + "test", + "describe", + "suite" + ] + }, + "required": [ + "import", + "name" + ], + "properties": { + "import": { + "type": "string", + "markdownDescription": "A package specifier (i.e. node:test) or workspace-relative path beginning with ./ (like ./test/utils.js) that indicates where the 'test' function can be imported from in your codebase" + }, + "name": { + "type": "array", + "markdownDescription": "A list of functions that are imported from `import` that should be treated as test functions", + "items": { + "type": "string" + } + } + } + } + }, "nodejs-testing.envFile": { "type": "string", "markdownDescription": "Absolute path to a file containing environment variable definitions.\n\nNote: template parameters like ${workspaceFolder} will be resolved.", diff --git a/src/controller.ts b/src/controller.ts index b369a0b..9ebe4fb 100644 --- a/src/controller.ts +++ b/src/controller.ts @@ -5,12 +5,13 @@ import picomatch from "picomatch"; import * as vscode from "vscode"; import { coverageContext } from "./coverage"; import { DisposableStore, MutableDisposable } from "./disposable"; +import { ExtensionConfig } from "./extension-config"; import { last } from "./iterable"; import { ICreateOpts, ItemType, getContainingItemsForFile, testMetadata } from "./metadata"; import { IParsedNode, parseSource } from "./parsing"; import { RunHandler, TestRunner } from "./runner"; import { ISourceMapMaintainer, SourceMapStore } from "./source-map-store"; -import { ExtensionConfig } from './extension-config'; +import type { TestFunctionSpecifierConfig } from "./test-function-specifier-config"; const diagnosticCollection = vscode.languages.createDiagnosticCollection("nodejs-testing-dupes"); @@ -61,6 +62,11 @@ export class Controller { } >(); + /** + * The configuration which defines which functions should be treated as tests + */ + private readonly testSpecifiers: TestFunctionSpecifierConfig[]; + /** Change emtiter used for testing, to pick up when the file watcher detects a chagne */ public readonly onDidChange = this.didChangeEmitter.event; /** Handler for a normal test run */ @@ -78,7 +84,9 @@ export class Controller { include: string[], exclude: string[], extensionConfigs: ExtensionConfig[], + testSpecifiers: TestFunctionSpecifierConfig[], ) { + this.testSpecifiers = testSpecifiers; this.disposable.add(ctrl); this.disposable.add(runner); const extensions = extensionConfigs.flatMap((x) => x.extensions); @@ -157,15 +165,18 @@ export class Controller { } } + /** + * Re-check this file for tests, and add them to the UI. + * Assumes that the URI has already passed `this.includeTest` + * + * @param folder the workspace folder this test file belongs to + * @param uri the URI of the file in question to reparse and check for tests + * @param contents the file contents of uri - to be used as an optimization + */ private async _syncFile(uri: vscode.Uri, contents?: string) { + const folder = vscode.workspace.getWorkspaceFolder(uri); contents ??= await fs.readFile(uri.fsPath, "utf8"); - // cheap test for relevancy: - if (!contents.includes("node:test")) { - this.deleteFileTests(uri.toString()); - return; - } - // avoid re-parsing if the contents are the same (e.g. if a file is edited // and then saved.) const previous = this.testsInFiles.get(uri.toString()); @@ -174,7 +185,7 @@ export class Controller { return; } - const tree = parseSource(contents); + const tree = parseSource(contents, folder, uri, this.testSpecifiers); if (!tree.length) { this.deleteFileTests(uri.toString()); return; @@ -224,7 +235,7 @@ export class Controller { return item; }; - // We assume that all tests inside a top-leve describe/test are from the same + // We assume that all tests inside a top-level describe/test are from the same // source file. This is probably a good assumption. Likewise we assume that a single // a single describe/test is not split between different files. const newTestsInFile = new Map(); @@ -295,8 +306,8 @@ export class Controller { new vscode.RelativePattern(this.wf, `**/*`), )); - watcher.onDidCreate((uri) => this.includeTest(uri.fsPath) && this._syncFile(uri)); - watcher.onDidChange((uri) => this.includeTest(uri.fsPath) && this._syncFile(uri)); + watcher.onDidCreate((uri) => this.syncFile(uri)); + watcher.onDidChange((uri) => this.syncFile(uri)); watcher.onDidDelete((uri) => { const prefix = uri.toString(); for (const key of this.testsInFiles.keys()) { diff --git a/src/extension.ts b/src/extension.ts index 6ae67c6..f9a7fc8 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -3,6 +3,7 @@ import { ConfigValue } from "./configValue"; import { Controller } from "./controller"; import { TestRunner } from "./runner"; import { SourceMapStore } from "./source-map-store"; +import { defaultTestFunctionSpecifiers } from "./test-function-specifier-config"; export async function activate(context: vscode.ExtensionContext) { const smStore = new SourceMapStore(); @@ -15,6 +16,8 @@ export async function activate(context: vscode.ExtensionContext) { }, ]); + const testSpecifiers = new ConfigValue("testSpecifiers", defaultTestFunctionSpecifiers); + const ctrls = new Map(); const refreshFolders = () => { for (const ctrl of ctrls.values()) { @@ -48,6 +51,7 @@ export async function activate(context: vscode.ExtensionContext) { includePattern.value, excludePatterns.value, extensions.value, + testSpecifiers.value, ), ); } diff --git a/src/parsing.ts b/src/parsing.ts index ba19fd1..9efb33e 100644 --- a/src/parsing.ts +++ b/src/parsing.ts @@ -1,7 +1,21 @@ import type { Options } from "acorn"; import { parse } from "acorn-loose"; import * as evk from "eslint-visitor-keys"; -import { CallExpression, Expression, Node, SourceLocation, Super } from "estree"; +import { + CallExpression, + Expression, + Node, + SourceLocation, + Super, + type ImportDeclaration, +} from "estree"; +import * as Path from "node:path"; +import * as vscode from "vscode"; +import { + defaultTestFunctionSpecifiers, + type TestFunctionSpecifierConfig, +} from "./test-function-specifier-config"; +import { assertUnreachable } from "./utils"; const enum C { ImportDeclaration = "ImportDeclaration", @@ -76,30 +90,94 @@ export interface IParsedNode { children: IParsedNode[]; } -export const parseSource = (text: string) => { +/** + * Look for test function imports in this AST node + * + * @param folder The workspace folder this file belongs to, used for relative path references to a custom test function + * @param fileUri The URI of the file we are extracting from + * @param testFunctions the tests function imports to check for + * @param node the ImportDelcaration to look for test imports + * @returns + */ +function importDeclarationExtractTests( + folder: vscode.WorkspaceFolder | undefined, + fileUri: vscode.Uri | undefined, + testFunctions: TestFunctionSpecifierConfig[], + node: ImportDeclaration, +): ExtractTest[] { + const idTests: ExtractTest[] = []; + if (typeof node.source.value !== "string") { + return []; + } + + let importValue = node.source.value; + if (node.source.value.startsWith("./") || node.source.value.startsWith("../")) { + if (!folder || !fileUri) { + console.warn(`Trying to match custom test function without specifying a folder or fileUri`); + return []; + } + + // This is a relative import, we need to adjust the import value for matching purposes + const importRelativeToRoot = Path.relative( + folder.uri.fsPath, + Path.resolve(Path.dirname(fileUri.fsPath), node.source.value), + ); + + importValue = `./${importRelativeToRoot}`; + } + + for (const specifier of testFunctions) { + if (specifier.import !== importValue) { + continue; + } + + // Next check to see if the functions imported are tests functions + const validNames = new Set( + typeof specifier.name === "string" ? [specifier.name] : specifier.name, + ); + + for (const spec of node.specifiers) { + const specType = spec.type; + if (specType === C.ImportDefaultSpecifier || specType === C.ImportNamespaceSpecifier) { + if (validNames.has("default")) { + idTests.push(matchNamespaced(spec.local.name)); + } + } else if (specType === C.ImportSpecifier) { + if (spec.imported.type === C.Identifier) { + if (validNames.has(spec.imported.name)) { + idTests.push(matchIdentified(spec.imported.name, spec.local.name)); + } + } + } else { + assertUnreachable(specType, `${specType} was unhandled`); + } + } + } + + return idTests; +} + +export const parseSource = ( + text: string, + folder?: vscode.WorkspaceFolder, + fileUri?: vscode.Uri, + testFunctions?: TestFunctionSpecifierConfig[], +): IParsedNode[] => { const ast = parse(text, acornOptions); + const testMatchers = testFunctions ?? defaultTestFunctionSpecifiers; const idTests: ExtractTest[] = []; + // Since tests can be nested inside of each other, for example a test suite. + // We keep track of the test declarations in a tree. const stack: { node: Node; r: IParsedNode }[] = []; stack.push({ node: undefined, r: { children: [] } } as any); traverse(ast as Node, { enter(node) { - if (node.type === C.ImportDeclaration && node.source.value === C.NodeTest) { - for (const spec of node.specifiers) { - switch (spec.type) { - case C.ImportNamespaceSpecifier: - case C.ImportDefaultSpecifier: - idTests.push(matchNamespaced(spec.local.name)); - break; - case C.ImportSpecifier: - if (spec.imported.type === C.Identifier) { - idTests.push(matchIdentified(spec.imported.name, spec.local.name)); - } - break; - } - } + if (node.type === C.ImportDeclaration) { + const matchers = importDeclarationExtractTests(folder, fileUri, testMatchers, node); + idTests.push(...matchers); } else if ( node.type === C.VariableDeclarator && node.init?.type === C.CallExpression && @@ -137,7 +215,13 @@ export const parseSource = (text: string) => { fn, name, }; + + // We have encountered a test function, record it in the tree. stack[stack.length - 1].r.children.push(child); + + // This test function is potentially a "parent" for subtests, so + // keep it as the "current leaf" of the stack, so future sub-tests + // can be associated with it stack.push({ node, r: child }); break; } @@ -145,6 +229,9 @@ export const parseSource = (text: string) => { } }, leave(node) { + // We are exiting a node that was potentially a test function. If it was, + // we need to pop it of the stack, since there are no more subtests to be + // associated with it. if (stack[stack.length - 1].node === node) { stack.pop(); } diff --git a/src/test-function-specifier-config.ts b/src/test-function-specifier-config.ts new file mode 100644 index 0000000..7ec8432 --- /dev/null +++ b/src/test-function-specifier-config.ts @@ -0,0 +1,20 @@ +/** + * A declarative way to target a function call either imported from a package, + * like node:test or from another file in the project + */ +export interface TestFunctionSpecifierConfig { + /** The names of the functions that should be included in the test runner view */ + name: string[] | string; + + /** + * The import location where thoes functions were imported from. If the import + * starts with `./` it will be treated as a file import relative to the root + * of the workspace, otherwise it refers to a package, like node:test or + * vitest + */ + import: string; +} + +export const defaultTestFunctionSpecifiers: TestFunctionSpecifierConfig[] = [ + { import: "node:test", name: ["default", "it", "test", "describe", "suite"] }, +]; diff --git a/src/utils.ts b/src/utils.ts new file mode 100644 index 0000000..c2981c4 --- /dev/null +++ b/src/utils.ts @@ -0,0 +1,11 @@ +/** + * A utility function to ensure that typescript unions are exhaustively checked. + * This function will fail at compile time if a previously exhaustive check is + * no longer exhaustive (in case a new value is added) + * + * @param _ the value that should have already been exhaustivly checked + * @param message The error message to throw in case this code is reached during runtime + */ +export function assertUnreachable(_: never, message: string): never { + throw new Error(message); +} diff --git a/testCases/simple/workspace/inRootFolderWrapped.test.mjs b/testCases/simple/workspace/inRootFolderWrapped.test.mjs new file mode 100644 index 0000000..f5f740e --- /dev/null +++ b/testCases/simple/workspace/inRootFolderWrapped.test.mjs @@ -0,0 +1,6 @@ +import { test } from "./test/utils.js"; +import { strictEqual } from "node:assert"; + +test("using wrapped test function from the workspace folder", () => { + strictEqual(1 + 1, 2); +}); diff --git a/testCases/simple/workspace/otherFolder/otherFolderWrapped.test.mjs b/testCases/simple/workspace/otherFolder/otherFolderWrapped.test.mjs new file mode 100644 index 0000000..84ae0f5 --- /dev/null +++ b/testCases/simple/workspace/otherFolder/otherFolderWrapped.test.mjs @@ -0,0 +1,6 @@ +import { test } from "../test/utils.js"; +import { strictEqual } from "node:assert"; + +test("using wrapped test function from the workspace/otherFolder folder", () => { + strictEqual(1 + 1, 2); +}); diff --git a/testCases/simple/workspace/test/inTestFolderWrapped.mjs b/testCases/simple/workspace/test/inTestFolderWrapped.mjs new file mode 100644 index 0000000..6f3d395 --- /dev/null +++ b/testCases/simple/workspace/test/inTestFolderWrapped.mjs @@ -0,0 +1,6 @@ +import { test } from "./utils.js"; +import { strictEqual } from "node:assert"; + +test("using wrapped test function from the workspace/test folder", () => { + strictEqual(1 + 1, 2); +}); diff --git a/testCases/simple/workspace/test/utils.js b/testCases/simple/workspace/test/utils.js new file mode 100644 index 0000000..822297c --- /dev/null +++ b/testCases/simple/workspace/test/utils.js @@ -0,0 +1,5 @@ +const { test: nodeTest } = require("node:test"); + +exports.test = function test(name, fn) { + return nodeTest(name, fn); +}; From b7b0e176384023ea12046ad1314f09c70f08dd1c Mon Sep 17 00:00:00 2001 From: Steven Kabbes Date: Mon, 31 Mar 2025 14:06:55 -0700 Subject: [PATCH 2/7] implement a cheap check finding tests in files --- src/controller.ts | 29 ++++++++++-------- src/extension.ts | 1 + src/test-function-specifier-config.ts | 42 ++++++++++++++++++++++++++- 3 files changed, 59 insertions(+), 13 deletions(-) diff --git a/src/controller.ts b/src/controller.ts index 9ebe4fb..d3361da 100644 --- a/src/controller.ts +++ b/src/controller.ts @@ -11,7 +11,10 @@ import { ICreateOpts, ItemType, getContainingItemsForFile, testMetadata } from " import { IParsedNode, parseSource } from "./parsing"; import { RunHandler, TestRunner } from "./runner"; import { ISourceMapMaintainer, SourceMapStore } from "./source-map-store"; -import type { TestFunctionSpecifierConfig } from "./test-function-specifier-config"; +import { + fileMightHaveTests, + type TestFunctionSpecifierConfig, +} from "./test-function-specifier-config"; const diagnosticCollection = vscode.languages.createDiagnosticCollection("nodejs-testing-dupes"); @@ -19,7 +22,7 @@ function jsExtensions(extensions: string[]) { let jsExtensions = ""; if (extensions == null || extensions.length == 0) { - throw "this case never accurs"; + throw "this case never occurs"; } else if (extensions.length == 1) { jsExtensions = `.${extensions[0]}`; } else { @@ -62,12 +65,7 @@ export class Controller { } >(); - /** - * The configuration which defines which functions should be treated as tests - */ - private readonly testSpecifiers: TestFunctionSpecifierConfig[]; - - /** Change emtiter used for testing, to pick up when the file watcher detects a chagne */ + /** Change emtiter used for testing, to pick up when the file watcher detects a change */ public readonly onDidChange = this.didChangeEmitter.event; /** Handler for a normal test run */ public readonly runHandler: RunHandler; @@ -84,9 +82,11 @@ export class Controller { include: string[], exclude: string[], extensionConfigs: ExtensionConfig[], - testSpecifiers: TestFunctionSpecifierConfig[], + /** + * The configuration which defines which functions should be treated as tests + */ + private readonly testSpecifiers: TestFunctionSpecifierConfig[], ) { - this.testSpecifiers = testSpecifiers; this.disposable.add(ctrl); this.disposable.add(runner); const extensions = extensionConfigs.flatMap((x) => x.extensions); @@ -174,9 +174,14 @@ export class Controller { * @param contents the file contents of uri - to be used as an optimization */ private async _syncFile(uri: vscode.Uri, contents?: string) { - const folder = vscode.workspace.getWorkspaceFolder(uri); contents ??= await fs.readFile(uri.fsPath, "utf8"); + // If this file definitely doesn't have any tests, we can skip any expensive processing on it + if (!fileMightHaveTests(this.testSpecifiers, contents)) { + this.deleteFileTests(uri.toString()); + return; + } + // avoid re-parsing if the contents are the same (e.g. if a file is edited // and then saved.) const previous = this.testsInFiles.get(uri.toString()); @@ -185,7 +190,7 @@ export class Controller { return; } - const tree = parseSource(contents, folder, uri, this.testSpecifiers); + const tree = parseSource(contents, this.wf, uri, this.testSpecifiers); if (!tree.length) { this.deleteFileTests(uri.toString()); return; diff --git a/src/extension.ts b/src/extension.ts index f9a7fc8..e2dfb8d 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -106,6 +106,7 @@ export async function activate(context: vscode.ExtensionContext) { includePattern.onChange(refreshFolders), excludePatterns.onChange(refreshFolders), extensions.onChange(refreshFolders), + testSpecifiers.onChange(refreshFolders), new vscode.Disposable(() => ctrls.forEach((c) => c.dispose())), ); diff --git a/src/test-function-specifier-config.ts b/src/test-function-specifier-config.ts index 7ec8432..e756dc3 100644 --- a/src/test-function-specifier-config.ts +++ b/src/test-function-specifier-config.ts @@ -1,3 +1,5 @@ +import * as Path from "node:path"; + /** * A declarative way to target a function call either imported from a package, * like node:test or from another file in the project @@ -7,7 +9,7 @@ export interface TestFunctionSpecifierConfig { name: string[] | string; /** - * The import location where thoes functions were imported from. If the import + * The import location where those functions were imported from. If the import * starts with `./` it will be treated as a file import relative to the root * of the workspace, otherwise it refers to a package, like node:test or * vitest @@ -18,3 +20,41 @@ export interface TestFunctionSpecifierConfig { export const defaultTestFunctionSpecifiers: TestFunctionSpecifierConfig[] = [ { import: "node:test", name: ["default", "it", "test", "describe", "suite"] }, ]; + +function singleFileMightHaveTests( + testSpec: TestFunctionSpecifierConfig, + contents: string, +): boolean { + if (testSpec.import.startsWith("./")) { + // If this test specifier is a relative import, like + // './my/test/functions/utils.ts' it is a little harder to do an easy check + // for tests, since it could be anything like: + // ./utils + // ./utils.js + // ./utils.ts + // ../utils.ts + // ./functions/utils.ts + // ../functions/utils.ts + // etc. + // We look for the extension-less basename of the test-defining file in the test- + const name = Path.basename(testSpec.import, Path.extname(testSpec.import)); + return contents.includes(name); + } + + // This is a test function imported from a package + return contents.includes(testSpec.import); +} + +/** + * Cheaply check if this file _might_ include any tests matched by the given specifications + * + * @param testSpecs the test specifiers to cheaply check the file contents + * @param contents the contents of the file we are checking for tests + * @returns true if this file requires further processing to check for tests + */ +export function fileMightHaveTests( + testSpecs: TestFunctionSpecifierConfig[], + contents: string, +): boolean { + return testSpecs.some((spec) => singleFileMightHaveTests(spec, contents)); +} From 293b3ba4b84a4d78972ab6317faa7b01bf0d9023 Mon Sep 17 00:00:00 2001 From: Steven Kabbes Date: Mon, 31 Mar 2025 16:01:07 -0700 Subject: [PATCH 3/7] parsing: refactor `parseSource` to not rely on any vscode.* code This allows unit test to be written and run without VS code. --- src/controller.ts | 2 +- src/parsing.ts | 51 +++++++++++++++++++++++++++++------------------ 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/src/controller.ts b/src/controller.ts index d3361da..8fcae19 100644 --- a/src/controller.ts +++ b/src/controller.ts @@ -190,7 +190,7 @@ export class Controller { return; } - const tree = parseSource(contents, this.wf, uri, this.testSpecifiers); + const tree = parseSource(contents, this.wf.uri.path, uri.path, this.testSpecifiers); if (!tree.length) { this.deleteFileTests(uri.toString()); return; diff --git a/src/parsing.ts b/src/parsing.ts index 9efb33e..99e452f 100644 --- a/src/parsing.ts +++ b/src/parsing.ts @@ -9,8 +9,7 @@ import { Super, type ImportDeclaration, } from "estree"; -import * as Path from "node:path"; -import * as vscode from "vscode"; +import * as PosixPath from "node:path/posix"; import { defaultTestFunctionSpecifiers, type TestFunctionSpecifierConfig, @@ -93,15 +92,15 @@ export interface IParsedNode { /** * Look for test function imports in this AST node * - * @param folder The workspace folder this file belongs to, used for relative path references to a custom test function - * @param fileUri The URI of the file we are extracting from + * @param workspaceFolderUriPath The path component of the workspace folder URI this file belongs to, used for relative path references to a custom test function + * @param fileUriPath The path component of the URI of the file we are extracting tests from * @param testFunctions the tests function imports to check for * @param node the ImportDelcaration to look for test imports * @returns */ function importDeclarationExtractTests( - folder: vscode.WorkspaceFolder | undefined, - fileUri: vscode.Uri | undefined, + workspaceFolderUriPath: string, + fileUriPath: string, testFunctions: TestFunctionSpecifierConfig[], node: ImportDeclaration, ): ExtractTest[] { @@ -112,15 +111,10 @@ function importDeclarationExtractTests( let importValue = node.source.value; if (node.source.value.startsWith("./") || node.source.value.startsWith("../")) { - if (!folder || !fileUri) { - console.warn(`Trying to match custom test function without specifying a folder or fileUri`); - return []; - } - // This is a relative import, we need to adjust the import value for matching purposes - const importRelativeToRoot = Path.relative( - folder.uri.fsPath, - Path.resolve(Path.dirname(fileUri.fsPath), node.source.value), + const importRelativeToRoot = PosixPath.relative( + workspaceFolderUriPath, + PosixPath.resolve(PosixPath.dirname(fileUriPath), node.source.value), ); importValue = `./${importRelativeToRoot}`; @@ -138,10 +132,14 @@ function importDeclarationExtractTests( for (const spec of node.specifiers) { const specType = spec.type; - if (specType === C.ImportDefaultSpecifier || specType === C.ImportNamespaceSpecifier) { + if (specType === C.ImportDefaultSpecifier) { + // The name "default" is special, it is used when you are trying to + // target a default export from a file in your workspace if (validNames.has("default")) { idTests.push(matchNamespaced(spec.local.name)); } + } else if (specType === C.ImportNamespaceSpecifier) { + idTests.push(matchNamespaced(spec.local.name)); } else if (specType === C.ImportSpecifier) { if (spec.imported.type === C.Identifier) { if (validNames.has(spec.imported.name)) { @@ -157,11 +155,21 @@ function importDeclarationExtractTests( return idTests; } +/** + * Parse the source code in `text` to identify any tests that match `testFunctions` + * + * @param text the contents of the file we want to parse + * @param workspaceFolderUriPath the path component of the URI of the workspace this file exists. + * If you have a `const folder: vscode.WorkspaceFolder`, then use folder.uri.path + * @param fileUriPath the path component of the URI of the file we are checking for tests + * @param testFunctions the configured test specifiers for the kinds of tests we are looking for + * @returns A hierarchical tree of tests that exist in this file + */ export const parseSource = ( text: string, - folder?: vscode.WorkspaceFolder, - fileUri?: vscode.Uri, - testFunctions?: TestFunctionSpecifierConfig[], + workspaceFolderUriPath: string, + fileUriPath: string, + testFunctions: TestFunctionSpecifierConfig[], ): IParsedNode[] => { const ast = parse(text, acornOptions); const testMatchers = testFunctions ?? defaultTestFunctionSpecifiers; @@ -176,7 +184,12 @@ export const parseSource = ( traverse(ast as Node, { enter(node) { if (node.type === C.ImportDeclaration) { - const matchers = importDeclarationExtractTests(folder, fileUri, testMatchers, node); + const matchers = importDeclarationExtractTests( + workspaceFolderUriPath, + fileUriPath, + testMatchers, + node, + ); idTests.push(...matchers); } else if ( node.type === C.VariableDeclarator && From 11d773b5a77d525a992ab5bacdb16c331d9b5365 Mon Sep 17 00:00:00 2001 From: Steven Kabbes Date: Tue, 1 Apr 2025 22:23:57 -0700 Subject: [PATCH 4/7] implement logic for `require` with custom test specifiers --- src/__snapshots__/parsing.test.ts.snap | 306 +++++++++++++++++++++++++ src/controller.ts | 2 +- src/parsing.test.ts | 227 +++++++++++++++++- src/parsing.ts | 125 +++++++--- src/test-function-specifier-config.ts | 5 +- 5 files changed, 619 insertions(+), 46 deletions(-) diff --git a/src/__snapshots__/parsing.test.ts.snap b/src/__snapshots__/parsing.test.ts.snap index 1a56985..3cb018a 100644 --- a/src/__snapshots__/parsing.test.ts.snap +++ b/src/__snapshots__/parsing.test.ts.snap @@ -669,3 +669,309 @@ exports[`extract > works with string literals 1`] = ` }, ] `; + +exports[`extract with custom test specifiers in ESM code > extracts default import tests 1`] = ` +[ + { + "children": [ + { + "children": [], + "fn": "test", + "location": SourceLocation { + "end": Position { + "column": 4, + "line": 8, + }, + "start": Position { + "column": 2, + "line": 6, + }, + }, + "name": "nested test", + }, + ], + "fn": "test", + "location": SourceLocation { + "end": Position { + "column": 2, + "line": 9, + }, + "start": Position { + "column": 0, + "line": 4, + }, + }, + "name": "default import test", + }, +] +`; + +exports[`extract with custom test specifiers in ESM code > extracts named import tests 1`] = ` +[ + { + "children": [], + "fn": "wrappedTest", + "location": SourceLocation { + "end": Position { + "column": 6, + "line": 4, + }, + "start": Position { + "column": 4, + "line": 2, + }, + }, + "name": "addition", + }, +] +`; + +exports[`extract with custom test specifiers in ESM code > extracts renamed named import test 1`] = ` +[ + { + "children": [], + "fn": "wrappedTest", + "location": SourceLocation { + "end": Position { + "column": 6, + "line": 4, + }, + "start": Position { + "column": 4, + "line": 2, + }, + }, + "name": "addition", + }, +] +`; + +exports[`extract with custom test specifiers in ESM code > extracts renamed named import tests 1`] = ` +[ + { + "children": [], + "fn": "wrappedTest", + "location": SourceLocation { + "end": Position { + "column": 6, + "line": 4, + }, + "start": Position { + "column": 4, + "line": 2, + }, + }, + "name": "addition", + }, +] +`; + +exports[`extract with custom test specifiers in ESM code > extracts star import tests 1`] = ` +[ + { + "children": [ + { + "children": [], + "fn": "test", + "location": SourceLocation { + "end": Position { + "column": 8, + "line": 7, + }, + "start": Position { + "column": 6, + "line": 5, + }, + }, + "name": "subtest", + }, + ], + "fn": "test", + "location": SourceLocation { + "end": Position { + "column": 6, + "line": 8, + }, + "start": Position { + "column": 4, + "line": 2, + }, + }, + "name": "addition", + }, +] +`; + +exports[`extract with custom test specifiers in commonjs code > extracts aliased require 1`] = ` +[ + { + "children": [ + { + "children": [], + "fn": "it", + "location": SourceLocation { + "end": Position { + "column": 4, + "line": 7, + }, + "start": Position { + "column": 2, + "line": 5, + }, + }, + "name": "nested test", + }, + ], + "fn": "describe", + "location": SourceLocation { + "end": Position { + "column": 2, + "line": 8, + }, + "start": Position { + "column": 0, + "line": 3, + }, + }, + "name": "destructured test", + }, +] +`; + +exports[`extract with custom test specifiers in commonjs code > extracts default require tests 1`] = ` +[ + { + "children": [ + { + "children": [], + "fn": "test", + "location": SourceLocation { + "end": Position { + "column": 4, + "line": 7, + }, + "start": Position { + "column": 2, + "line": 5, + }, + }, + "name": "nested test", + }, + ], + "fn": "test", + "location": SourceLocation { + "end": Position { + "column": 2, + "line": 8, + }, + "start": Position { + "column": 0, + "line": 3, + }, + }, + "name": "default import test", + }, +] +`; + +exports[`extract with custom test specifiers in commonjs code > extracts destructed require 1`] = ` +[ + { + "children": [ + { + "children": [], + "fn": "it", + "location": SourceLocation { + "end": Position { + "column": 4, + "line": 7, + }, + "start": Position { + "column": 2, + "line": 5, + }, + }, + "name": "nested test", + }, + ], + "fn": "describe", + "location": SourceLocation { + "end": Position { + "column": 2, + "line": 8, + }, + "start": Position { + "column": 0, + "line": 3, + }, + }, + "name": "destructured test", + }, +] +`; + +exports[`extract with custom test specifiers in commonjs code > extracts ts import mangled 1`] = ` +[ + { + "children": [ + { + "children": [], + "fn": "it", + "location": SourceLocation { + "end": Position { + "column": 6, + "line": 8, + }, + "start": Position { + "column": 4, + "line": 6, + }, + }, + "name": "addition", + }, + { + "children": [], + "fn": "it", + "location": SourceLocation { + "end": Position { + "column": 6, + "line": 11, + }, + "start": Position { + "column": 4, + "line": 9, + }, + }, + "name": "addition", + }, + { + "children": [], + "fn": "it", + "location": SourceLocation { + "end": Position { + "column": 6, + "line": 14, + }, + "start": Position { + "column": 4, + "line": 12, + }, + }, + "name": "subtraction", + }, + ], + "fn": "describe", + "location": SourceLocation { + "end": Position { + "column": 2, + "line": 15, + }, + "start": Position { + "column": 0, + "line": 5, + }, + }, + "name": "math", + }, +] +`; diff --git a/src/controller.ts b/src/controller.ts index 8fcae19..5b9b29a 100644 --- a/src/controller.ts +++ b/src/controller.ts @@ -65,7 +65,7 @@ export class Controller { } >(); - /** Change emtiter used for testing, to pick up when the file watcher detects a change */ + /** Change emitter used for testing, to pick up when the file watcher detects a change */ public readonly onDidChange = this.didChangeEmitter.event; /** Handler for a normal test run */ public readonly runHandler: RunHandler; diff --git a/src/parsing.test.ts b/src/parsing.test.ts index e18d76b..f04a37d 100644 --- a/src/parsing.test.ts +++ b/src/parsing.test.ts @@ -1,5 +1,35 @@ import { describe, expect, it } from "vitest"; import { parseSource } from "./parsing"; +import { + defaultTestFunctionSpecifiers, + type TestFunctionSpecifierConfig, +} from "./test-function-specifier-config"; + +function parseSourceSimple(contents: string) { + return parseSource( + contents, + "/workspace/", + "/workspace/test.test.js", + defaultTestFunctionSpecifiers, + ); +} + +type ParseCustomOptions = { + testNames?: string[]; + testImport?: string; + workspace?: string; + path?: string; +}; + +// parseSourceCustom is a ergonomic version of parseSource that has reasonable defaults for the parameters +const parseSourceCustom = (contents: string, options?: ParseCustomOptions) => { + const testNames = options?.testNames ?? ["test"]; + const testImport = options?.testImport ?? "./test/utils"; + const workspace = options?.workspace ?? "/workspace/"; + const path = options?.path ?? "/workspace/test/addition.test.js"; + const testFunctions: TestFunctionSpecifierConfig[] = [{ import: testImport, name: testNames }]; + return parseSource(contents, workspace, path, testFunctions); +}; const testCases = (prefix = "") => `${prefix}describe("math", () => { ${prefix}it("addition", () => { @@ -20,7 +50,7 @@ describe("extract", () => { ${testCases("nt.")}`; - expect(parseSource(src)).toMatchSnapshot(); + expect(parseSourceSimple(src)).toMatchSnapshot(); }); it("extracts star import", () => { @@ -28,7 +58,7 @@ describe("extract", () => { ${testCases("nt.")}`; - expect(parseSource(src)).toMatchSnapshot(); + expect(parseSourceSimple(src)).toMatchSnapshot(); }); it("extracts named imports", () => { @@ -36,7 +66,7 @@ describe("extract", () => { ${testCases()}`; - expect(parseSource(src)).toMatchSnapshot(); + expect(parseSourceSimple(src)).toMatchSnapshot(); }); it("extracts aliased imports", () => { @@ -44,7 +74,7 @@ describe("extract", () => { ${testCases("x")}`; - expect(parseSource(src)).toMatchSnapshot(); + expect(parseSourceSimple(src)).toMatchSnapshot(); }); it("extracts default require", () => { @@ -52,7 +82,7 @@ describe("extract", () => { ${testCases("nt.")}`; - expect(parseSource(src)).toMatchSnapshot(); + expect(parseSourceSimple(src)).toMatchSnapshot(); }); it("extracts destructed require", () => { @@ -60,7 +90,7 @@ describe("extract", () => { ${testCases()}`; - expect(parseSource(src)).toMatchSnapshot(); + expect(parseSourceSimple(src)).toMatchSnapshot(); }); it("extracts aliased require", () => { @@ -68,7 +98,7 @@ describe("extract", () => { ${testCases("x")}`; - expect(parseSource(src)).toMatchSnapshot(); + expect(parseSourceSimple(src)).toMatchSnapshot(); }); it("extracts ts import mangled", () => { @@ -89,7 +119,7 @@ const node_test_1 = require("node:test"); }); //# sourceMappingURL=example.js.map`; - expect(parseSource(src)).toMatchSnapshot(); + expect(parseSourceSimple(src)).toMatchSnapshot(); }); it("does not break on empty call expression (#3)", () => { @@ -99,7 +129,7 @@ const node_test_1 = require("node:test"); ${testCases()}`; - expect(parseSource(src)).toMatchSnapshot(); + expect(parseSourceSimple(src)).toMatchSnapshot(); }); it("works with string literals", () => { @@ -111,7 +141,7 @@ const node_test_1 = require("node:test"); }); `; - expect(parseSource(src)).toMatchSnapshot(); + expect(parseSourceSimple(src)).toMatchSnapshot(); }); it("works with default cjs import", () => { @@ -120,7 +150,7 @@ const node_test_1 = require("node:test"); nt(\`addition\`, () => {}); `; - expect(parseSource(src)).toMatchSnapshot(); + expect(parseSourceSimple(src)).toMatchSnapshot(); }); it("works with default esm import", () => { @@ -129,6 +159,179 @@ const node_test_1 = require("node:test"); nt(\`addition\`, () => {}); `; - expect(parseSource(src)).toMatchSnapshot(); + expect(parseSourceSimple(src)).toMatchSnapshot(); + }); +}); + +describe("extract with custom test specifiers in ESM code", () => { + it("extracts default import tests", () => { + const contents = ` +import nt from "./utils"; + +nt("default import test", () => { + strictEqual(1 + 1, 2); + nt("nested test", () => { + strictEqual(1 + 1, 2); + }); +}); +`; + + const result = parseSourceCustom(contents, { + testNames: ["default"], + }); + + // One test, and one subtest + expect(result.length).toEqual(1); + expect(result[0].children.length).toEqual(1); + expect(result).toMatchSnapshot(); + }); + + it("extracts star import tests", () => { + const contents = `import * as Utils from "./utils"; + Utils.test("addition", () => { + strictEqual(1 + 1, 2); + + Utils.test("subtest", () => { + strictEqual(1 + 1, 2); + }); + });`; + + const result = parseSourceCustom(contents, { + testNames: ["test"], + }); + + expect(result.length).toEqual(1); + expect(result[0].children.length).toEqual(1); + expect(result).toMatchSnapshot(); + }); + + it("extracts named import tests", () => { + const contents = `import { wrappedTest } from "./utils"; + wrappedTest("addition", () => { + strictEqual(1 + 1, 2); + });`; + + const result = parseSourceCustom(contents, { + testNames: ["wrappedTest"], + }); + expect(result.length).toEqual(1); + expect(result).toMatchSnapshot(); + }); + + it("extracts renamed named import tests", () => { + const contents = `import { wrappedTest as renamedTest } from "./utils"; + renamedTest("addition", () => { + strictEqual(1 + 1, 2); + });`; + + const result = parseSourceCustom(contents, { + testNames: ["wrappedTest"], + }); + expect(result.length).toEqual(1); + expect(result).toMatchSnapshot(); + }); + + it("extracts renamed named import test", () => { + const contents = `import { wrappedTest as renamedTest } from "./utils"; + renamedTest("addition", () => { + strictEqual(1 + 1, 2); + });`; + + const result = parseSourceCustom(contents, { + testNames: ["wrappedTest"], + }); + expect(result.length).toEqual(1); + expect(result).toMatchSnapshot(); + }); +}); + +describe("extract with custom test specifiers in commonjs code", () => { + it("extracts default require tests", () => { + const contents = `const nt = require("./utils"); + +nt("default import test", () => { + strictEqual(1 + 1, 2); + nt("nested test", () => { + strictEqual(1 + 1, 2); + }); +}); +`; + const result = parseSourceCustom(contents, { + testNames: ["default"], + }); + + // One test, and one subtest + expect(result.length).toEqual(1); + expect(result[0].children.length).toEqual(1); + expect(result).toMatchSnapshot(); + }); + + it("extracts destructed require", () => { + const contents = `const { describe, it } = require("./utils"); + +describe("destructured test", () => { + strictEqual(1 + 1, 2); + it("nested test", () => { + strictEqual(1 + 1, 2); + }); +}); +`; + const result = parseSourceCustom(contents, { + testNames: ["describe", "it"], + }); + + // One test, and one subtest + expect(result.length).toEqual(1); + expect(result[0].children.length).toEqual(1); + expect(result).toMatchSnapshot(); + }); + + it("extracts aliased require", () => { + const contents = `const { describe: xdescribe, it: xit } = require("./utils"); + +xdescribe("destructured test", () => { + strictEqual(1 + 1, 2); + xit("nested test", () => { + strictEqual(1 + 1, 2); + }); +}); +`; + + const result = parseSourceCustom(contents, { + testNames: ["describe", "it"], + }); + + // One test, and one subtest + expect(result.length).toEqual(1); + expect(result[0].children.length).toEqual(1); + expect(result).toMatchSnapshot(); + }); + + it("extracts ts import mangled", () => { + const contents = `"use strict"; +Object.defineProperty(exports, "__esModule", { value: true }); +const assert_1 = require("assert"); +const node_test_1 = require("./utils"); +(0, node_test_1.describe)("math", () => { + (0, node_test_1.it)("addition", () => { + (0, assert_1.strictEqual)(1 + 1, 2); + }); + (0, node_test_1.it)("addition", () => { + (0, assert_1.strictEqual)(1 + 1, 2); + }); + (0, node_test_1.it)("subtraction", () => { + (0, assert_1.strictEqual)(1 - 1, 0); + }); +}); +//# sourceMappingURL=example.js.map`; + + const result = parseSourceCustom(contents, { + testNames: ["describe", "it"], + }); + + // 1 test, 3 subtests + expect(result.length).toEqual(1); + expect(result[0].children.length).toEqual(3); + expect(result).toMatchSnapshot(); }); }); diff --git a/src/parsing.ts b/src/parsing.ts index 99e452f..5ffe85f 100644 --- a/src/parsing.ts +++ b/src/parsing.ts @@ -8,6 +8,7 @@ import { SourceLocation, Super, type ImportDeclaration, + type VariableDeclarator, } from "estree"; import * as PosixPath from "node:path/posix"; import { @@ -90,12 +91,36 @@ export interface IParsedNode { } /** - * Look for test function imports in this AST node + * Resolve the import of `importString` from the file at `fileUriPath` relative to the workspace root. + * + * If the `importString` refers a package it remains unchanged. + */ +function importRelativeToWorkspaceRoot( + workspaceFolderUriPath: string, + fileUriPath: string, + importString: string, +) { + let importValue = importString; + if (importString.startsWith("./") || importString.startsWith("../")) { + // This is a relative import, we need to adjust the import value for matching purposes + const importRelativeToRoot = PosixPath.relative( + workspaceFolderUriPath, + PosixPath.resolve(PosixPath.dirname(fileUriPath), importString), + ); + + importValue = `./${importRelativeToRoot}`; + } + + return importValue; +} + +/** + * Look for test ESM imports in this AST node * * @param workspaceFolderUriPath The path component of the workspace folder URI this file belongs to, used for relative path references to a custom test function * @param fileUriPath The path component of the URI of the file we are extracting tests from * @param testFunctions the tests function imports to check for - * @param node the ImportDelcaration to look for test imports + * @param node the ImportDeclaration to look for test imports * @returns */ function importDeclarationExtractTests( @@ -106,22 +131,17 @@ function importDeclarationExtractTests( ): ExtractTest[] { const idTests: ExtractTest[] = []; if (typeof node.source.value !== "string") { - return []; + return idTests; } - let importValue = node.source.value; - if (node.source.value.startsWith("./") || node.source.value.startsWith("../")) { - // This is a relative import, we need to adjust the import value for matching purposes - const importRelativeToRoot = PosixPath.relative( - workspaceFolderUriPath, - PosixPath.resolve(PosixPath.dirname(fileUriPath), node.source.value), - ); - - importValue = `./${importRelativeToRoot}`; - } + const fromRootImport = importRelativeToWorkspaceRoot( + workspaceFolderUriPath, + fileUriPath, + node.source.value, + ); for (const specifier of testFunctions) { - if (specifier.import !== importValue) { + if (specifier.import !== fromRootImport) { continue; } @@ -155,6 +175,60 @@ function importDeclarationExtractTests( return idTests; } +function requireCallExtractTests( + workspaceFolderUriPath: string, + fileUriPath: string, + testFunctions: TestFunctionSpecifierConfig[], + node: VariableDeclarator, +) { + const idTests: ExtractTest[] = []; + if (node.init?.type !== C.CallExpression) { + return idTests; + } + + const firstArg = getStringish(node.init.arguments[0]); + if (!firstArg) { + return idTests; + } + + const fromRootImport = importRelativeToWorkspaceRoot( + workspaceFolderUriPath, + fileUriPath, + firstArg, + ); + + for (const specifier of testFunctions) { + if (specifier.import !== fromRootImport) { + continue; + } + + if (node.id.type === C.Identifier) { + idTests.push(matchNamespaced(node.id.name)); + continue; + } + + // Next check to see if the functions imported are tests functions + const validNames = new Set( + typeof specifier.name === "string" ? [specifier.name] : specifier.name, + ); + + if (node.id.type === C.ObjectPattern) { + for (const prop of node.id.properties) { + if ( + prop.type === C.Property && + prop.key.type === C.Identifier && + prop.value.type === C.Identifier && + validNames.has(prop.key.name) + ) { + idTests.push(matchIdentified(prop.key.name, prop.value.name)); + } + } + } + } + + return idTests; +} + /** * Parse the source code in `text` to identify any tests that match `testFunctions` * @@ -197,22 +271,13 @@ export const parseSource = ( node.init.callee.type === C.Identifier && node.init.callee.name === "require" ) { - const firstArg = getStringish(node.init.arguments[0]); - if (firstArg === C.NodeTest) { - if (node.id.type === C.ObjectPattern) { - for (const prop of node.id.properties) { - if ( - prop.type === C.Property && - prop.key.type === C.Identifier && - prop.value.type === C.Identifier - ) { - idTests.push(matchIdentified(prop.key.name, prop.value.name)); - } - } - } else if (node.id.type === C.Identifier) { - idTests.push(matchNamespaced(node.id.name)); - } - } + const matchers = requireCallExtractTests( + workspaceFolderUriPath, + fileUriPath, + testMatchers, + node, + ); + idTests.push(...matchers); } else if (node.type === C.CallExpression) { const name = getStringish(node.arguments[0]); if (name === undefined) { diff --git a/src/test-function-specifier-config.ts b/src/test-function-specifier-config.ts index e756dc3..010902d 100644 --- a/src/test-function-specifier-config.ts +++ b/src/test-function-specifier-config.ts @@ -1,4 +1,4 @@ -import * as Path from "node:path"; +import * as PathPosix from "node:path/posix"; /** * A declarative way to target a function call either imported from a package, @@ -37,8 +37,7 @@ function singleFileMightHaveTests( // ../functions/utils.ts // etc. // We look for the extension-less basename of the test-defining file in the test- - const name = Path.basename(testSpec.import, Path.extname(testSpec.import)); - return contents.includes(name); + return contents.includes(PathPosix.parse(testSpec.import).name); } // This is a test function imported from a package From dde896fdb9638fe4be4d9d6de8586ff503716e16 Mon Sep 17 00:00:00 2001 From: Steven Kabbes Date: Tue, 1 Apr 2025 22:47:43 -0700 Subject: [PATCH 5/7] self code review --- src/controller.ts | 1 - src/parsing.test.ts | 2 +- src/parsing.ts | 15 +++++++++++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/controller.ts b/src/controller.ts index 5b9b29a..831cfe8 100644 --- a/src/controller.ts +++ b/src/controller.ts @@ -169,7 +169,6 @@ export class Controller { * Re-check this file for tests, and add them to the UI. * Assumes that the URI has already passed `this.includeTest` * - * @param folder the workspace folder this test file belongs to * @param uri the URI of the file in question to reparse and check for tests * @param contents the file contents of uri - to be used as an optimization */ diff --git a/src/parsing.test.ts b/src/parsing.test.ts index f04a37d..911f8ca 100644 --- a/src/parsing.test.ts +++ b/src/parsing.test.ts @@ -163,7 +163,7 @@ const node_test_1 = require("node:test"); }); }); -describe("extract with custom test specifiers in ESM code", () => { +describe("extract tests with custom specifiers", () => { it("extracts default import tests", () => { const contents = ` import nt from "./utils"; diff --git a/src/parsing.ts b/src/parsing.ts index 5ffe85f..027094d 100644 --- a/src/parsing.ts +++ b/src/parsing.ts @@ -175,6 +175,21 @@ function importDeclarationExtractTests( return idTests; } +/** + * Look for test imports in a `require` call + * For example: + * ``` + * const test = require("node:test"); + * const { test } = require("node:test"); + * const { test: renamedTest } = require("node:test"); + * ``` + * + * @param workspaceFolderUriPath The path component of the workspace folder URI this file belongs to, used for relative path references to a custom test function + * @param fileUriPath The path component of the URI of the file we are extracting tests from + * @param testFunctions the tests function imports to check for + * @param node the VariableDeclarator to look for + * @returns + */ function requireCallExtractTests( workspaceFolderUriPath: string, fileUriPath: string, From 70c00ce2d914cecec88238d1f7faec7c8fc5b553 Mon Sep 17 00:00:00 2001 From: Steven Kabbes Date: Tue, 1 Apr 2025 22:59:33 -0700 Subject: [PATCH 6/7] clarify documentation for configuration value --- package.json | 4 ++-- src/__snapshots__/parsing.test.ts.snap | 14 +++++++------- src/parsing.test.ts | 6 +++--- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/package.json b/package.json index ae8482f..fb49f59 100644 --- a/package.json +++ b/package.json @@ -149,7 +149,7 @@ }, "nodejs-testing.testSpecifiers": { "type": "array", - "markdownDescription": "_Advanced_: A list of specifiers that indicate test function to search for:\nIt defaults to:\n\n```json\n[\n {\n \"import\": \"node:test\",\n \"name\": [\"default\", \"it\", \"test\", \"describe\", \"suite\"]\n }\n]\n```\n\nBut in case your test function is wrapped, you can specify it with a relative import:\nNOTE: relative imports must be prefixed with ./\n\n```json\n[\n {\n \"import\": \"./test/utils.js\",\n \"name\": \"test\"\n }\n]\n```\n", + "markdownDescription": "_Advanced_: A list of specifiers that indicate test function to search for:\nIt defaults to:\n\n```json\n[\n {\n \"import\": \"node:test\",\n \"name\": [\"default\", \"it\", \"test\", \"describe\", \"suite\"]\n }\n]\n```\n\nBut in case your test function is wrapped, you can specify it with a relative import:\nNOTE: relative imports must be prefixed with ./\nNOTE: A `name` of \"default\" is special and means the default export of that module is a test function\n\n```json\n[\n {\n \"import\": \"./test/utils.js\",\n \"name\": \"test\"\n }\n]\n```\n", "default": [ { "import": "node:test", @@ -185,7 +185,7 @@ }, "name": { "type": "array", - "markdownDescription": "A list of functions that are imported from `import` that should be treated as test functions", + "markdownDescription": "A list of functions that are imported from `import` that should be treated as test functions, the special name 'default' refers to the default export of a module", "items": { "type": "string" } diff --git a/src/__snapshots__/parsing.test.ts.snap b/src/__snapshots__/parsing.test.ts.snap index 3cb018a..62064e1 100644 --- a/src/__snapshots__/parsing.test.ts.snap +++ b/src/__snapshots__/parsing.test.ts.snap @@ -670,7 +670,7 @@ exports[`extract > works with string literals 1`] = ` ] `; -exports[`extract with custom test specifiers in ESM code > extracts default import tests 1`] = ` +exports[`extract tests with custom specifiers > extracts default import tests 1`] = ` [ { "children": [ @@ -706,7 +706,7 @@ exports[`extract with custom test specifiers in ESM code > extracts default impo ] `; -exports[`extract with custom test specifiers in ESM code > extracts named import tests 1`] = ` +exports[`extract tests with custom specifiers > extracts named import tests 1`] = ` [ { "children": [], @@ -726,7 +726,7 @@ exports[`extract with custom test specifiers in ESM code > extracts named import ] `; -exports[`extract with custom test specifiers in ESM code > extracts renamed named import test 1`] = ` +exports[`extract tests with custom specifiers > extracts renamed named import test 1`] = ` [ { "children": [], @@ -746,7 +746,7 @@ exports[`extract with custom test specifiers in ESM code > extracts renamed name ] `; -exports[`extract with custom test specifiers in ESM code > extracts renamed named import tests 1`] = ` +exports[`extract tests with custom specifiers > extracts renamed named import tests 1`] = ` [ { "children": [], @@ -766,13 +766,13 @@ exports[`extract with custom test specifiers in ESM code > extracts renamed name ] `; -exports[`extract with custom test specifiers in ESM code > extracts star import tests 1`] = ` +exports[`extract tests with custom specifiers > extracts star import tests 1`] = ` [ { "children": [ { "children": [], - "fn": "test", + "fn": "specialTest", "location": SourceLocation { "end": Position { "column": 8, @@ -786,7 +786,7 @@ exports[`extract with custom test specifiers in ESM code > extracts star import "name": "subtest", }, ], - "fn": "test", + "fn": "specialTest", "location": SourceLocation { "end": Position { "column": 6, diff --git a/src/parsing.test.ts b/src/parsing.test.ts index 911f8ca..a7d455a 100644 --- a/src/parsing.test.ts +++ b/src/parsing.test.ts @@ -188,16 +188,16 @@ nt("default import test", () => { it("extracts star import tests", () => { const contents = `import * as Utils from "./utils"; - Utils.test("addition", () => { + Utils.specialTest("addition", () => { strictEqual(1 + 1, 2); - Utils.test("subtest", () => { + Utils.specialTest("subtest", () => { strictEqual(1 + 1, 2); }); });`; const result = parseSourceCustom(contents, { - testNames: ["test"], + testNames: ["specialTest"], }); expect(result.length).toEqual(1); From 1099debaf612b1faa98a2dd182445d864b54be61 Mon Sep 17 00:00:00 2001 From: Steven Kabbes Date: Wed, 2 Apr 2025 10:23:41 -0700 Subject: [PATCH 7/7] Check tests have valid names when using 'namespaced' imports --- src/__snapshots__/parsing.test.ts.snap | 6 ++--- src/parsing.test.ts | 6 ++++- src/parsing.ts | 37 +++++++++++++++----------- 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/__snapshots__/parsing.test.ts.snap b/src/__snapshots__/parsing.test.ts.snap index 62064e1..2537b78 100644 --- a/src/__snapshots__/parsing.test.ts.snap +++ b/src/__snapshots__/parsing.test.ts.snap @@ -776,11 +776,11 @@ exports[`extract tests with custom specifiers > extracts star import tests 1`] = "location": SourceLocation { "end": Position { "column": 8, - "line": 7, + "line": 10, }, "start": Position { "column": 6, - "line": 5, + "line": 8, }, }, "name": "subtest", @@ -790,7 +790,7 @@ exports[`extract tests with custom specifiers > extracts star import tests 1`] = "location": SourceLocation { "end": Position { "column": 6, - "line": 8, + "line": 11, }, "start": Position { "column": 4, diff --git a/src/parsing.test.ts b/src/parsing.test.ts index a7d455a..cb8a1a1 100644 --- a/src/parsing.test.ts +++ b/src/parsing.test.ts @@ -191,10 +191,14 @@ nt("default import test", () => { Utils.specialTest("addition", () => { strictEqual(1 + 1, 2); + // this should not be identified as test + Utils.log("something") + Utils.specialTest("subtest", () => { strictEqual(1 + 1, 2); }); - });`; + }); + `; const result = parseSourceCustom(contents, { testNames: ["specialTest"], diff --git a/src/parsing.ts b/src/parsing.ts index 027094d..42c0355 100644 --- a/src/parsing.ts +++ b/src/parsing.ts @@ -59,19 +59,26 @@ const matchIdentified = }; const matchNamespaced = - (name: string): ExtractTest => + (objectName: string, validTestNames: Set): ExtractTest => (n) => { const callee = unpackCalleeExpression(n); - if (callee.type === C.Identifier && callee.name === name) { + if (callee.type === C.Identifier && callee.name === objectName) { return "test"; // default export, #42 } - return callee.type === C.MemberExpression && - callee.object.type === C.Identifier && - callee.object.name === name && - callee.property.type === C.Identifier - ? callee.property.name - : undefined; + if ( + !( + callee.type === C.MemberExpression && + callee.object.type === C.Identifier && + callee.object.name === objectName && + callee.property.type === C.Identifier + ) + ) { + return undefined; + } + + const calleeName = callee.property.name; + return validTestNames.has(calleeName) ? calleeName : undefined; }; const getStringish = (nameArg: Node | undefined): string | undefined => { @@ -156,10 +163,10 @@ function importDeclarationExtractTests( // The name "default" is special, it is used when you are trying to // target a default export from a file in your workspace if (validNames.has("default")) { - idTests.push(matchNamespaced(spec.local.name)); + idTests.push(matchNamespaced(spec.local.name, validNames)); } } else if (specType === C.ImportNamespaceSpecifier) { - idTests.push(matchNamespaced(spec.local.name)); + idTests.push(matchNamespaced(spec.local.name, validNames)); } else if (specType === C.ImportSpecifier) { if (spec.imported.type === C.Identifier) { if (validNames.has(spec.imported.name)) { @@ -217,16 +224,16 @@ function requireCallExtractTests( continue; } - if (node.id.type === C.Identifier) { - idTests.push(matchNamespaced(node.id.name)); - continue; - } - // Next check to see if the functions imported are tests functions const validNames = new Set( typeof specifier.name === "string" ? [specifier.name] : specifier.name, ); + if (node.id.type === C.Identifier) { + idTests.push(matchNamespaced(node.id.name, validNames)); + continue; + } + if (node.id.type === C.ObjectPattern) { for (const prop of node.id.properties) { if (