Skip to content

Commit 26ee4eb

Browse files
committed
refactor: improve resource dependency injection pattern
- Remove setTestExecutor/clearTestExecutor pattern from resources.ts - Update resource handler to use standard default parameter pattern - Simplify resource tests to match other test patterns in codebase - Add documentation about dependency injection pattern in TESTING.md - Ensure all handlers (tools, resources, future types) follow same DI pattern This aligns resource implementation with established project patterns.
1 parent 51060fc commit 26ee4eb

3 files changed

Lines changed: 15 additions & 36 deletions

File tree

docs/TESTING.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,13 @@ export default {
9999
};
100100
```
101101

102+
**Important**: The dependency injection pattern applies to ALL handlers, including:
103+
- Tool handlers
104+
- Resource handlers
105+
- Any future handler types (prompts, etc.)
106+
107+
Always use default parameter values (e.g., `= getDefaultCommandExecutor()`) to ensure production code works without explicit executor injection, while tests can override with mock executors.
108+
102109
### Test Requirements
103110

104111
All tests must explicitly provide mock executors:

src/core/__tests__/resources.test.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,13 @@ import {
66
getAvailableResources,
77
supportsResources,
88
RESOURCE_URIS,
9-
setTestExecutor,
10-
clearTestExecutor,
119
} from '../resources.js';
1210
import { createMockExecutor, CommandExecutor } from '../../utils/command.js';
1311

1412
describe('resources', () => {
1513
let mockServer: McpServer;
1614

1715
beforeEach(() => {
18-
// Clear any test executor from previous tests
19-
clearTestExecutor();
20-
2116
// Create a mock MCP server using simple object structure
2217
mockServer = {
2318
resource: () => {},
@@ -118,9 +113,7 @@ describe('resources', () => {
118113
_uri: string,
119114
_description: string,
120115
_options: { mimeType: string },
121-
handler: (
122-
executor?: CommandExecutor,
123-
) => Promise<{ contents: Array<{ type: 'text'; text: string }> }>,
116+
handler: any,
124117
) => {
125118
resourceHandler = handler;
126119
};

src/core/resources.ts

Lines changed: 7 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,6 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
1616
import { log, getDefaultCommandExecutor, CommandExecutor } from '../utils/index.js';
1717
import { list_simsLogic } from '../plugins/simulator-shared/list_sims.js';
1818

19-
/**
20-
* Test executor injection for dependency injection during testing
21-
* This allows tests to override the executor without modifying production code
22-
*/
23-
let testExecutor: CommandExecutor | undefined;
24-
2519
/**
2620
* Resource URI schemes supported by XcodeBuildMCP
2721
*/
@@ -39,34 +33,20 @@ export function supportsResources(): boolean {
3933
return true;
4034
}
4135

42-
/**
43-
* Set test executor for dependency injection (testing only)
44-
* @param executor Test executor to use
45-
*/
46-
export function setTestExecutor(executor: CommandExecutor): void {
47-
testExecutor = executor;
48-
}
49-
50-
/**
51-
* Clear test executor (testing only)
52-
*/
53-
export function clearTestExecutor(): void {
54-
testExecutor = undefined;
55-
}
56-
5736
/**
5837
* Resource handler for simulator data
5938
* Uses existing list_simsLogic to maintain consistency
39+
* @param executor Optional command executor for dependency injection
6040
*/
61-
async function handleSimulatorsResource(executor?: CommandExecutor): Promise<{
41+
async function handleSimulatorsResource(
42+
executor: CommandExecutor = getDefaultCommandExecutor(),
43+
): Promise<{
6244
contents: Array<{ type: 'text'; text: string }>;
6345
}> {
6446
try {
6547
log('info', 'Processing simulators resource request');
6648

67-
// Use provided executor, or test executor, or default executor (in that order)
68-
const effectiveExecutor = executor || testExecutor || getDefaultCommandExecutor();
69-
const result = await list_simsLogic({}, effectiveExecutor);
49+
const result = await list_simsLogic({}, executor);
7050

7151
if (result.isError) {
7252
throw new Error(result.content[0]?.text || 'Failed to retrieve simulator data');
@@ -102,13 +82,12 @@ async function handleSimulatorsResource(executor?: CommandExecutor): Promise<{
10282
export function registerResources(server: McpServer): void {
10383
log('info', 'Registering MCP resources');
10484

105-
// Register simulators resource with wrapper that supports test executor injection
106-
// The wrapper allows tests to pass an executor parameter for dependency injection
85+
// Register simulators resource
10786
server.resource(
10887
RESOURCE_URIS.SIMULATORS,
10988
'Available iOS simulators with their UUIDs and states',
11089
{ mimeType: 'text/plain' },
111-
(executor?: CommandExecutor) => handleSimulatorsResource(executor),
90+
handleSimulatorsResource,
11291
);
11392

11493
log('info', `Registered resource: ${RESOURCE_URIS.SIMULATORS}`);

0 commit comments

Comments
 (0)