Skip to content

Commit 3df1573

Browse files
chore: address PR feedback
1. assert function are not assertions so fixed them with relevant names 2. moved static method logic to cli entry point and added e2e for test cases
1 parent 9c54a3b commit 3df1573

File tree

8 files changed

+92
-74
lines changed

8 files changed

+92
-74
lines changed

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@
6565
"generate": "npm run generate:api && npm run generate:arguments",
6666
"generate:api": "./scripts/generate.sh",
6767
"generate:arguments": "tsx scripts/generateArguments.ts",
68+
"pretest": "npm run build",
6869
"test": "vitest --project eslint-rules --project unit-and-integration --coverage",
69-
"pretest:accuracy": "npm run build",
7070
"test:accuracy": "sh ./scripts/accuracy/runAccuracyTests.sh",
7171
"test:long-running-tests": "vitest --project long-running-tests --coverage",
7272
"atlas:cleanup": "vitest --project atlas-cleanup"

src/common/config/argsParserOptions.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export const OPTIONS = {
6060
"apiDeprecationErrors",
6161
"apiStrict",
6262
"disableEmbeddingsValidation",
63-
"dry",
63+
"dryRun",
6464
"help",
6565
"indexCheck",
6666
"ipv6",

src/common/config/userConfig.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ export const UserConfigSchema = z4.object({
167167
)
168168
.default([])
169169
.describe("An array of preview features that are enabled."),
170-
dry: z4
170+
dryRun: z4
171171
.boolean()
172172
.default(false)
173173
.describe(

src/index.ts

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,23 @@ import { StdioRunner } from "./transports/stdio.js";
4444
import { StreamableHttpRunner } from "./transports/streamableHttp.js";
4545
import { systemCA } from "@mongodb-js/devtools-proxy-support";
4646
import { Keychain } from "./common/keychain.js";
47-
import { DryModeRunner } from "./transports/dryModeRunner.js";
47+
import { DryRunModeRunner } from "./transports/dryModeRunner.js";
4848

4949
async function main(): Promise<void> {
5050
systemCA().catch(() => undefined); // load system CA asynchronously as in mongosh
5151

5252
const config = createUserConfig();
53-
assertHelpMode(config);
54-
assertVersionMode(config);
55-
await DryModeRunner.assertDryMode({ userConfig: config });
53+
if (config.help) {
54+
handleHelpRequest();
55+
}
56+
57+
if (config.version) {
58+
handleVersionRequest();
59+
}
60+
61+
if (config.dryRun) {
62+
await handleDryRunRequest(config);
63+
}
5664

5765
const transportRunner =
5866
config.transport === "stdio"
@@ -135,17 +143,34 @@ main().catch((error: unknown) => {
135143
process.exit(1);
136144
});
137145

138-
function assertHelpMode(config: UserConfig): void | never {
139-
if (config.help) {
140-
console.log("For usage information refer to the README.md:");
141-
console.log("https://github.com/mongodb-js/mongodb-mcp-server?tab=readme-ov-file#quick-start");
142-
process.exit(0);
143-
}
146+
function handleHelpRequest(): never {
147+
console.log("For usage information refer to the README.md:");
148+
console.log("https://github.com/mongodb-js/mongodb-mcp-server?tab=readme-ov-file#quick-start");
149+
process.exit(0);
144150
}
145151

146-
function assertVersionMode(config: UserConfig): void | never {
147-
if (config.version) {
148-
console.log(packageInfo.version);
152+
function handleVersionRequest(): never {
153+
console.log(packageInfo.version);
154+
process.exit(0);
155+
}
156+
157+
export async function handleDryRunRequest(config: UserConfig): Promise<never> {
158+
try {
159+
const runner = new DryRunModeRunner({
160+
userConfig: config,
161+
logger: {
162+
log(message): void {
163+
console.log(message);
164+
},
165+
error(message): void {
166+
console.error(message);
167+
},
168+
},
169+
});
170+
await runner.start();
149171
process.exit(0);
172+
} catch (error) {
173+
console.error(`Fatal error running server in dry run mode: ${error as string}`);
174+
process.exit(1);
150175
}
151176
}

src/transports/dryModeRunner.ts

Lines changed: 12 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -2,49 +2,34 @@ import { InMemoryTransport } from "./inMemoryTransport.js";
22
import { TransportRunnerBase, type TransportRunnerConfig } from "./base.js";
33
import { type Server } from "../server.js";
44

5-
export type DryModeTestHelpers = {
6-
exit(this: void, exitCode: number): never;
5+
export type DryRunModeTestHelpers = {
76
logger: {
87
log(this: void, message: string): void;
98
error(this: void, message: string): void;
109
};
1110
};
1211

13-
type DryModeRunnerConfig = TransportRunnerConfig & DryModeTestHelpers;
12+
type DryRunModeRunnerConfig = TransportRunnerConfig & DryRunModeTestHelpers;
1413

15-
const defaultLogger: DryModeTestHelpers["logger"] = {
16-
log(message) {
17-
console.warn(message);
18-
},
19-
error(message) {
20-
console.error(message);
21-
},
22-
};
23-
24-
export class DryModeRunner extends TransportRunnerBase {
14+
export class DryRunModeRunner extends TransportRunnerBase {
2515
private server: Server | undefined;
26-
private exitProcess: DryModeTestHelpers["exit"];
27-
private consoleLogger: DryModeTestHelpers["logger"];
16+
private consoleLogger: DryRunModeTestHelpers["logger"];
2817

29-
constructor({ exit, logger, ...transportRunnerConfig }: DryModeRunnerConfig) {
18+
constructor({ logger, ...transportRunnerConfig }: DryRunModeRunnerConfig) {
3019
super(transportRunnerConfig);
31-
this.exitProcess = exit;
3220
this.consoleLogger = logger;
3321
}
3422

35-
async start(): Promise<void> {
36-
try {
37-
this.server = await this.setupServer();
38-
const transport = new InMemoryTransport();
23+
override async start(): Promise<void> {
24+
this.server = await this.setupServer();
25+
const transport = new InMemoryTransport();
3926

40-
await this.server.connect(transport);
41-
} catch (error: unknown) {
42-
this.consoleLogger.error(`Fatal error running server: ${error as string}`);
43-
this.exitProcess(1);
44-
}
27+
await this.server.connect(transport);
28+
this.dumpConfig();
29+
this.dumpTools();
4530
}
4631

47-
async closeTransport(): Promise<void> {
32+
override async closeTransport(): Promise<void> {
4833
await this.server?.close();
4934
}
5035

@@ -56,24 +41,9 @@ export class DryModeRunner extends TransportRunnerBase {
5641
private dumpTools(): void {
5742
const tools = this.server?.tools.map((tool) => ({
5843
name: tool.name,
59-
description: tool.description,
6044
category: tool.category,
6145
}));
6246
this.consoleLogger.log("Enabled tools:");
6347
this.consoleLogger.log(JSON.stringify(tools, null, 2));
6448
}
65-
66-
static async assertDryMode(
67-
runnerConfig: TransportRunnerConfig,
68-
exit: DryModeTestHelpers["exit"] = (exitCode: number) => process.exit(exitCode),
69-
logger: DryModeTestHelpers["logger"] = defaultLogger
70-
): Promise<void> | never {
71-
if (runnerConfig.userConfig.dry) {
72-
const runner = new this({ ...runnerConfig, exit, logger });
73-
await runner.start();
74-
runner.dumpConfig();
75-
runner.dumpTools();
76-
exit(0);
77-
}
78-
}
7949
}

tests/e2e/cli.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import path from "path";
2+
import { exec } from "child_process";
3+
import { promisify } from "util";
4+
import { describe, expect, it } from "vitest";
5+
import packageJson from "../../package.json" with { type: "json" };
6+
7+
const execAsync = promisify(exec);
8+
const CLI_PATH = path.join(import.meta.dirname, "..", "..", "dist", "index.js");
9+
10+
describe("CLI entrypoint", () => {
11+
it("should handle version request", async () => {
12+
const { stdout, stderr } = await execAsync(`${process.execPath} ${CLI_PATH} --version`);
13+
expect(stdout).toContain(packageJson.version);
14+
expect(stderr).toEqual("");
15+
});
16+
17+
it("should handle help request", async () => {
18+
const { stdout, stderr } = await execAsync(`${process.execPath} ${CLI_PATH} --help`);
19+
expect(stdout).toContain("For usage information refer to the README.md");
20+
expect(stderr).toEqual("");
21+
});
22+
23+
it("should handle dry run request", async () => {
24+
const { stdout, stderr } = await execAsync(`${process.execPath} ${CLI_PATH} --dryRun`);
25+
expect(stdout).toContain("Configuration:");
26+
expect(stdout).toContain("Enabled tools:");
27+
expect(stderr).toEqual("");
28+
});
29+
});

tests/unit/common/config.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ const expectedDefaults = {
5959
vectorSearchSimilarityFunction: "euclidean",
6060
disableEmbeddingsValidation: false,
6161
previewFeatures: [],
62-
dry: false,
62+
dryRun: false,
6363
};
6464

6565
describe("config", () => {
Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,14 @@
11
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
2-
import { DryModeRunner, type DryModeTestHelpers } from "../../../src/transports/dryModeRunner.js";
2+
import { DryRunModeRunner, type DryRunModeTestHelpers } from "../../../src/transports/dryModeRunner.js";
33
import { type UserConfig } from "../../../src/common/config/userConfig.js";
44
import { type TransportRunnerConfig } from "../../../src/transports/base.js";
55
import { defaultTestConfig } from "../../integration/helpers.js";
66

7-
describe("DryModeRunner", () => {
8-
let exitMock: DryModeTestHelpers["exit"];
9-
let loggerMock: DryModeTestHelpers["logger"];
7+
describe.only("DryModeRunner", () => {
8+
let loggerMock: DryRunModeTestHelpers["logger"];
109
let runnerConfig: TransportRunnerConfig;
1110

1211
beforeEach(() => {
13-
exitMock = vi.fn<DryModeTestHelpers["exit"]>();
1412
loggerMock = {
1513
log: vi.fn(),
1614
error: vi.fn(),
@@ -24,25 +22,21 @@ describe("DryModeRunner", () => {
2422
vi.clearAllMocks();
2523
});
2624

27-
it("should not do anything if dry mode is disabled", async () => {
28-
await DryModeRunner.assertDryMode(runnerConfig, exitMock, loggerMock);
29-
expect(exitMock).not.toHaveBeenCalled();
30-
expect(loggerMock.log).not.toHaveBeenCalled();
31-
});
32-
3325
it.each([{ transport: "http", httpHost: "127.0.0.1", httpPort: "3001" }, { transport: "stdio" }] as Array<
3426
Partial<UserConfig>
35-
>)("should run in dry mode if enabled for transport - $transport", async (partialConfig) => {
27+
>)("should handle dry run request for transport - $transport", async (partialConfig) => {
3628
runnerConfig.userConfig = {
3729
...runnerConfig.userConfig,
3830
...partialConfig,
39-
dry: true,
31+
dryRun: true,
4032
};
41-
await DryModeRunner.assertDryMode(runnerConfig, exitMock, loggerMock);
42-
expect(exitMock).toHaveBeenCalledWith(0);
33+
const runner = new DryRunModeRunner({ logger: loggerMock, ...runnerConfig });
34+
await runner.start();
4335
expect(loggerMock.log).toHaveBeenNthCalledWith(1, "Configuration:");
4436
expect(loggerMock.log).toHaveBeenNthCalledWith(2, JSON.stringify(runnerConfig.userConfig, null, 2));
4537
expect(loggerMock.log).toHaveBeenNthCalledWith(3, "Enabled tools:");
4638
expect(loggerMock.log).toHaveBeenNthCalledWith(4, expect.stringContaining('"name": "connect"'));
39+
// Because switch-connection is not enabled by default
40+
expect(loggerMock.log).toHaveBeenNthCalledWith(4, expect.not.stringContaining('"name": "switch-connection"'));
4741
});
4842
});

0 commit comments

Comments
 (0)