From 3c31ddc791bf6c9ad829cf1f5ed8d1f765ed63b9 Mon Sep 17 00:00:00 2001 From: Nicholas Roscino Date: Tue, 21 Apr 2026 16:24:57 +0200 Subject: [PATCH 1/3] test: refactor tests to reduce duplication --- tests/index.test.ts | 148 +++++++++++++++++--------------------------- 1 file changed, 58 insertions(+), 90 deletions(-) diff --git a/tests/index.test.ts b/tests/index.test.ts index e9adb3537..91b2a2ce3 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -12,10 +12,7 @@ import {Client} from '@modelcontextprotocol/sdk/client/index.js'; import {StdioClientTransport} from '@modelcontextprotocol/sdk/client/stdio.js'; import {executablePath} from 'puppeteer'; -import { - OFF_BY_DEFAULT_CATEGORIES, - ToolCategory, -} from '../src/tools/categories.js'; +import {OFF_BY_DEFAULT_CATEGORIES} from '../src/tools/categories.js'; import type {ToolDefinition} from '../src/tools/ToolDefinition.js'; describe('e2e', () => { @@ -80,44 +77,7 @@ describe('e2e', () => { async client => { const {tools} = await client.listTools(); const exposedNames = tools.map(t => t.name).sort(); - const files = fs.readdirSync('build/src/tools'); - const definedNames = []; - for (const file of files) { - if ( - file === 'ToolDefinition.js' || - file === 'tools.js' || - file === 'slim' - ) { - continue; - } - const fileTools = await import(`../src/tools/${file}`); - for (const maybeTool of Object.values(fileTools)) { - if (typeof maybeTool === 'function') { - const tool = (maybeTool as (val: boolean) => ToolDefinition)( - false, - ); - if (tool && typeof tool === 'object' && 'name' in tool) { - if (tool.annotations?.conditions) { - continue; - } - definedNames.push(tool.name); - } - continue; - } - if ( - typeof maybeTool === 'object' && - maybeTool !== null && - 'name' in maybeTool - ) { - const tool = maybeTool as ToolDefinition; - if (tool.annotations?.conditions) { - continue; - } - definedNames.push(tool.name); - } - } - } - + const definedNames = await getToolsWithFilteredCategories(); definedNames.sort(); assert.deepStrictEqual(exposedNames, definedNames); }, @@ -129,54 +89,9 @@ describe('e2e', () => { await withClient(async client => { const {tools} = await client.listTools(); const exposedNames = tools.map(t => t.name).sort(); - const files = fs.readdirSync('build/src/tools'); - const definedNames = []; - for (const file of files) { - if ( - file === 'ToolDefinition.js' || - file === 'tools.js' || - file === 'slim' - ) { - continue; - } - const fileTools = await import(`../src/tools/${file}`); - for (const maybeTool of Object.values(fileTools)) { - if (typeof maybeTool === 'function') { - const tool = (maybeTool as (val: boolean) => ToolDefinition)(false); - if (tool && typeof tool === 'object' && 'name' in tool) { - if (tool.annotations?.conditions) { - continue; - } - if ( - tool.annotations?.category && - tool.annotations?.category === ToolCategory.EXTENSIONS - ) { - continue; - } - definedNames.push(tool.name); - } - continue; - } - if ( - typeof maybeTool === 'object' && - maybeTool !== null && - 'name' in maybeTool - ) { - const tool = maybeTool as ToolDefinition; - if (tool.annotations?.conditions) { - continue; - } - if ( - tool.annotations?.category && - tool.annotations?.category === ToolCategory.EXTENSIONS - ) { - continue; - } - definedNames.push(tool.name); - } - } - } - + const definedNames = await getToolsWithFilteredCategories( + OFF_BY_DEFAULT_CATEGORIES, + ); definedNames.sort(); assert.deepStrictEqual(exposedNames, definedNames); }); @@ -245,3 +160,56 @@ describe('e2e', () => { ); }); }); + +async function getToolsWithFilteredCategories( + filterOutCategories: string[] = [], +): Promise { + const files = fs.readdirSync('build/src/tools'); + const definedNames = []; + for (const file of files) { + if ( + file === 'ToolDefinition.js' || + file === 'tools.js' || + file === 'slim' + ) { + continue; + } + const fileTools = await import(`../src/tools/${file}`); + for (const maybeTool of Object.values(fileTools)) { + if (typeof maybeTool === 'function') { + const tool = (maybeTool as (val: boolean) => ToolDefinition)(false); + if (tool && typeof tool === 'object' && 'name' in tool) { + if (tool.annotations?.conditions) { + continue; + } + if ( + tool.annotations?.category && + filterOutCategories.includes(tool.annotations?.category) + ) { + continue; + } + definedNames.push(tool.name); + } + continue; + } + if ( + typeof maybeTool === 'object' && + maybeTool !== null && + 'name' in maybeTool + ) { + const tool = maybeTool as ToolDefinition; + if (tool.annotations?.conditions) { + continue; + } + if ( + tool.annotations?.category && + filterOutCategories.includes(tool.annotations?.category) + ) { + continue; + } + definedNames.push(tool.name); + } + } + } + return definedNames; +} From c2028192ede4021c3803561d01406e3da7e43185 Mon Sep 17 00:00:00 2001 From: Nicholas Roscino Date: Tue, 21 Apr 2026 17:07:12 +0200 Subject: [PATCH 2/3] test: decoupling common logic --- tests/index.test.ts | 62 ++++++++++++++++++++++----------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/tests/index.test.ts b/tests/index.test.ts index 91b2a2ce3..fae967054 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -12,6 +12,7 @@ import {Client} from '@modelcontextprotocol/sdk/client/index.js'; import {StdioClientTransport} from '@modelcontextprotocol/sdk/client/stdio.js'; import {executablePath} from 'puppeteer'; +import type {ToolCategory} from '../src/tools/categories.js'; import {OFF_BY_DEFAULT_CATEGORIES} from '../src/tools/categories.js'; import type {ToolDefinition} from '../src/tools/ToolDefinition.js'; @@ -162,7 +163,7 @@ describe('e2e', () => { }); async function getToolsWithFilteredCategories( - filterOutCategories: string[] = [], + filterOutCategories: ToolCategory[] = [], ): Promise { const files = fs.readdirSync('build/src/tools'); const definedNames = []; @@ -176,40 +177,39 @@ async function getToolsWithFilteredCategories( } const fileTools = await import(`../src/tools/${file}`); for (const maybeTool of Object.values(fileTools)) { + let tool; if (typeof maybeTool === 'function') { - const tool = (maybeTool as (val: boolean) => ToolDefinition)(false); - if (tool && typeof tool === 'object' && 'name' in tool) { - if (tool.annotations?.conditions) { - continue; - } - if ( - tool.annotations?.category && - filterOutCategories.includes(tool.annotations?.category) - ) { - continue; - } - definedNames.push(tool.name); - } - continue; + tool = (maybeTool as (val: boolean) => ToolDefinition)(false); + } else { + tool = maybeTool as ToolDefinition; } - if ( - typeof maybeTool === 'object' && - maybeTool !== null && - 'name' in maybeTool - ) { - const tool = maybeTool as ToolDefinition; - if (tool.annotations?.conditions) { - continue; - } - if ( - tool.annotations?.category && - filterOutCategories.includes(tool.annotations?.category) - ) { - continue; - } - definedNames.push(tool.name); + + if (toolShouldBeSkipped(tool, filterOutCategories)) { + continue; } + definedNames.push(tool.name); } } return definedNames; } + +function toolShouldBeSkipped( + tool: ToolDefinition, + filteredOutCategories: ToolCategory[], +) { + if (tool === null || typeof tool !== 'object' || !('name' in tool)) { + return true; + } + + if (tool.annotations?.conditions) { + return true; + } + if ( + tool.annotations?.category && + filteredOutCategories.includes(tool.annotations?.category) + ) { + return true; + } + + return false; +} From 2596656ccbb307ea5683d6bf284de7250b29ebe1 Mon Sep 17 00:00:00 2001 From: Nicholas Roscino Date: Tue, 21 Apr 2026 17:57:16 +0200 Subject: [PATCH 3/3] chore: move tool file check before function call --- tests/index.test.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/index.test.ts b/tests/index.test.ts index fae967054..a1c7765bb 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -176,6 +176,7 @@ async function getToolsWithFilteredCategories( continue; } const fileTools = await import(`../src/tools/${file}`); + for (const maybeTool of Object.values(fileTools)) { let tool; if (typeof maybeTool === 'function') { @@ -184,6 +185,11 @@ async function getToolsWithFilteredCategories( tool = maybeTool as ToolDefinition; } + // Skipping all files that are not tool files + if (tool === null || typeof tool !== 'object' || !('name' in tool)) { + continue; + } + if (toolShouldBeSkipped(tool, filterOutCategories)) { continue; } @@ -197,10 +203,6 @@ function toolShouldBeSkipped( tool: ToolDefinition, filteredOutCategories: ToolCategory[], ) { - if (tool === null || typeof tool !== 'object' || !('name' in tool)) { - return true; - } - if (tool.annotations?.conditions) { return true; }