Skip to content

Commit 84883f0

Browse files
cdcabreracdcabrera
committed
test: review updates
1 parent 2a22bd0 commit 84883f0

File tree

5 files changed

+89
-37
lines changed

5 files changed

+89
-37
lines changed

tests/__fixtures__/tool.echo.js renamed to tests/__fixtures__/tool.echoBasic.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
// Fixture exports a creator function directly;
22

33
const echo_plugin_tool = options => [
4-
'echo_plugin_tool',
4+
'echo_basic_tool',
55
{
6-
description: 'Echo back the provided args, but with a different description',
6+
description: 'Echo basic tool. Echos back the provided args.',
77
inputSchema: { additionalProperties: true }
88
},
99
args => ({

‎tests/__fixtures__/tool.echoToolHelper.js‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Fixture exports a createMcpTool module directly;
2+
import { createMcpTool } from '../../dist/index.js';
3+
4+
export default createMcpTool({
5+
name: 'echo_createMcp_tool',
6+
description: 'Echo create MCP tool. Echos back the provided args.',
7+
inputSchema: { additionalProperties: true },
8+
handler: async args => ({
9+
args,
10+
content: [
11+
{
12+
type: 'text',
13+
text: JSON.stringify(args)
14+
}
15+
]
16+
})
17+
});

‎tests/__snapshots__/stdioTransport.test.ts.snap‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ exports[`PatternFly MCP, STDIO should expose expected tools and stable shape 1`]
442442
}
443443
`;
444444

445-
exports[`Tools should interact with the new tool 1`] = `
445+
exports[`Tools should interact with a tool, echo basic tool 1`] = `
446446
{
447447
"args": {
448448
"dolor": "sit amet",
@@ -463,3 +463,19 @@ exports[`Tools should interact with the new tool 1`] = `
463463
],
464464
}
465465
`;
466+
467+
exports[`Tools should interact with a tool, echo create MCP tool 1`] = `
468+
{
469+
"args": {
470+
"dolor": "sit amet",
471+
"lorem": "ipsum",
472+
"type": "echo",
473+
},
474+
"content": [
475+
{
476+
"text": "{"type":"echo","lorem":"ipsum","dolor":"sit amet"}",
477+
"type": "text",
478+
},
479+
],
480+
}
481+
`;

‎tests/httpTransport.test.ts‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
11
/**
22
* Requires: npm run build prior to running Jest.
3+
* - If typings are needed, use public types from dist to avoid type identity mismatches between src and dist
34
*/
4-
// import { resolve } from 'node:path';
5-
// import { pathToFileURL } from 'node:url';
65
// @ts-ignore - dist/index.js isn't necessarily built yet, remember to build before running tests
76
import { createMcpTool } from '../dist/index.js';
87
import { startServer, type HttpTransportClient, type RpcRequest } from './utils/httpTransportClient';
98
import { setupFetchMock } from './utils/fetchMock';
10-
// Use public types from dist to avoid type identity mismatches between src and dist
119

1210
describe('PatternFly MCP, HTTP Transport', () => {
1311
let FETCH_MOCK: Awaited<ReturnType<typeof setupFetchMock>> | undefined;
@@ -47,7 +45,6 @@ describe('PatternFly MCP, HTTP Transport', () => {
4745

4846
afterAll(async () => {
4947
if (CLIENT) {
50-
// You may still receive jest warnings about a running process, but clean up case we forget at the test level.
5148
await CLIENT.close();
5249
CLIENT = undefined;
5350
}
@@ -97,9 +94,6 @@ describe('PatternFly MCP, HTTP Transport', () => {
9794
const response = await CLIENT?.send(req);
9895
const text = response?.result?.content?.[0]?.text || '';
9996

100-
// expect(CLIENT?.logs()).toMatchSnapshot();
101-
// expect(CLIENT?.protocolLogs()).toMatchSnapshot();
102-
10397
expect(text.startsWith('# Documentation from')).toBe(true);
10498
expect(text).toMatchSnapshot();
10599
});
@@ -126,7 +120,7 @@ describe('PatternFly MCP, HTTP Transport', () => {
126120

127121
expect(text.startsWith('# Documentation from')).toBe(true);
128122
expect(text).toMatchSnapshot();
129-
CLIENT.close();
123+
await CLIENT.close();
130124
});
131125
});
132126

‎tests/stdioTransport.test.ts‎

Lines changed: 51 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
/**
22
* Requires: npm run build prior to running Jest.
3+
* - If typings are needed, use public types from dist to avoid type identity mismatches between src and dist
4+
* - We're unable to mock fetch for stdio since it runs in a separate process, so we run a server and use that path for mocking external URLs.
35
*/
46
import { resolve } from 'node:path';
57
import { pathToFileURL } from 'node:url';
@@ -13,7 +15,6 @@ import { setupFetchMock } from './utils/fetchMock';
1315
describe('PatternFly MCP, STDIO', () => {
1416
let FETCH_MOCK: Awaited<ReturnType<typeof setupFetchMock>> | undefined;
1517
let CLIENT: StdioTransportClient;
16-
// We're unable to mock fetch for stdio since it runs in a separate process, so we run a server and use that path for mocking external URLs.
1718
let URL_MOCK: string;
1819

1920
beforeAll(async () => {
@@ -22,7 +23,6 @@ describe('PatternFly MCP, STDIO', () => {
2223
routes: [
2324
{
2425
url: /\/README\.md$/,
25-
// url: '/notARealPath/README.md',
2626
status: 200,
2727
headers: { 'Content-Type': 'text/markdown; charset=utf-8' },
2828
body: `# PatternFly Development Rules
@@ -39,7 +39,6 @@ describe('PatternFly MCP, STDIO', () => {
3939
},
4040
{
4141
url: /.*\.md$/,
42-
// url: '/notARealPath/AboutModal.md',
4342
status: 200,
4443
headers: { 'Content-Type': 'text/markdown; charset=utf-8' },
4544
body: '# Test Document\n\nThis is a test document for mocking remote HTTP requests.'
@@ -53,7 +52,6 @@ describe('PatternFly MCP, STDIO', () => {
5352

5453
afterAll(async () => {
5554
if (CLIENT) {
56-
// You may still receive jest warnings about a running process, but clean up case we forget at the test level.
5755
await CLIENT.close();
5856
}
5957

@@ -116,7 +114,7 @@ describe('PatternFly MCP, STDIO', () => {
116114
const response = await CLIENT.send(req, { timeoutMs: 10000 });
117115
const text = response?.result?.content?.[0]?.text || '';
118116

119-
// expect(text.startsWith('# Documentation from')).toBe(true);
117+
expect(text.startsWith('# Documentation from')).toBe(true);
120118
expect(text).toMatchSnapshot();
121119
});
122120
});
@@ -179,15 +177,25 @@ describe('Tools', () => {
179177
let CLIENT: StdioTransportClient;
180178

181179
beforeEach(async () => {
182-
const abs = resolve(process.cwd(), 'tests/__fixtures__/tool.echo.js');
183-
const url = pathToFileURL(abs).href;
184-
185-
CLIENT = await startServer({ args: ['--log-stderr', '--plugin-isolation', 'strict', '--tool', url] });
180+
const echoBasicFileUrl = pathToFileURL(resolve(process.cwd(), 'tests/__fixtures__/tool.echoBasic.js')).href;
181+
const echoToolHelperFileUrl = pathToFileURL(resolve(process.cwd(), 'tests/__fixtures__/tool.echoToolHelper.js')).href;
182+
183+
CLIENT = await startServer({
184+
args: [
185+
'--log-stderr',
186+
'--plugin-isolation',
187+
'strict',
188+
'--tool',
189+
echoBasicFileUrl,
190+
'--tool',
191+
echoToolHelperFileUrl
192+
]
193+
});
186194
});
187195

188196
afterEach(async () => CLIENT.stop());
189197

190-
itSkip(envNodeVersion >= 22)('should access a new tool', async () => {
198+
itSkip(envNodeVersion >= 22)('should access new tools', async () => {
191199
const req = {
192200
method: 'tools/list',
193201
params: {}
@@ -196,8 +204,11 @@ describe('Tools', () => {
196204
const resp = await CLIENT.send(req);
197205
const names = (resp?.result?.tools ?? []).map((tool: any) => tool.name);
198206

199-
expect(CLIENT.logs().join(',')).toContain('Registered tool: echo_plugin_tool');
200-
expect(names).toContain('echo_plugin_tool');
207+
expect(CLIENT.logs().join(',')).toContain('Registered tool: echo_basic_tool');
208+
expect(names).toContain('echo_basic_tool');
209+
210+
expect(CLIENT.logs().join(',')).toContain('Registered tool: echo_createMcp_tool');
211+
expect(names).toContain('echo_createMcp_tool');
201212
});
202213

203214
itSkip(envNodeVersion <= 20)('should fail to access a new tool', async () => {
@@ -211,16 +222,23 @@ describe('Tools', () => {
211222
expect(CLIENT.logs().join(',')).toContain('External tool plugins require Node >= 22; skipping file-based tools.');
212223
});
213224

214-
itSkip(envNodeVersion >= 22)('should interact with the new tool', async () => {
225+
itSkip(envNodeVersion >= 22).each([
226+
{
227+
description: 'echo basic tool',
228+
name: 'echo_basic_tool',
229+
args: { type: 'echo', lorem: 'ipsum', dolor: 'sit amet' }
230+
},
231+
{
232+
description: 'echo create MCP tool',
233+
name: 'echo_createMcp_tool',
234+
args: { type: 'echo', lorem: 'ipsum', dolor: 'sit amet' }
235+
}
236+
])('should interact with a tool, $description', async ({ name, args }) => {
215237
const req = {
216238
method: 'tools/call',
217239
params: {
218-
name: 'echo_plugin_tool',
219-
arguments: {
220-
type: 'echo',
221-
lorem: 'ipsum',
222-
dolor: 'sit amet'
223-
}
240+
name,
241+
arguments: args
224242
}
225243
};
226244

@@ -230,16 +248,23 @@ describe('Tools', () => {
230248
expect(resp.result.isError).toBeUndefined();
231249
});
232250

233-
itSkip(envNodeVersion <= 20)('should fail to interact with the new tool', async () => {
251+
itSkip(envNodeVersion <= 20).each([
252+
{
253+
description: 'echo basic tool',
254+
name: 'echo_basic_tool',
255+
args: { type: 'echo', lorem: 'ipsum', dolor: 'sit amet' }
256+
},
257+
{
258+
description: 'echo create MCP tool',
259+
name: 'echo_createMcp_tool',
260+
args: { type: 'echo', lorem: 'ipsum', dolor: 'sit amet' }
261+
}
262+
])('should fail to interact with a tool, $description', async ({ name, args }) => {
234263
const req = {
235264
method: 'tools/call',
236265
params: {
237-
name: 'echo_plugin_tool',
238-
arguments: {
239-
type: 'echo',
240-
lorem: 'ipsum',
241-
dolor: 'sit amet'
242-
}
266+
name,
267+
arguments: args
243268
}
244269
};
245270

0 commit comments

Comments
 (0)