Skip to content

Commit 0a0eb31

Browse files
author
Krishna Prasath D
committed
fixing issues with rest api approach
1 parent 2a1b1b8 commit 0a0eb31

2 files changed

Lines changed: 88 additions & 40 deletions

File tree

src/tools/test-plans.ts

Lines changed: 40 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
55
import { WebApi } from "azure-devops-node-api";
66
import { TestPlanCreateParams } from "azure-devops-node-api/interfaces/TestPlanInterfaces.js";
77
import { z } from "zod";
8+
import { apiVersion } from "../utils.js";
89

910
const Test_Plan_Tools = {
1011
create_test_plan: "testplan_create_test_plan",
@@ -32,43 +33,43 @@ function configureTestPlanTools(server: McpServer, tokenProvider: () => Promise<
3233
try {
3334
const connection = await connectionProvider();
3435
const accessToken = await tokenProvider();
35-
const params = new URLSearchParams({ "api-version": "7.2-preview.1" });
36+
const params = new URLSearchParams({ "api-version": apiVersion });
3637
if (filterActivePlans) params.append("filterActivePlans", "true");
3738
if (includePlanDetails) params.append("includePlanDetails", "true");
3839
if (continuationToken) params.append("continuationToken", continuationToken);
39-
const url = `${connection.serverUrl}/${project}/_apis/testplan/Plans?${params.toString()}`;
40+
const url = `${connection.serverUrl}/${encodeURIComponent(project)}/_apis/testplan/Plans?${params.toString()}`;
4041
const headers: Record<string, string> = {
41-
"Content-Type": "application/json",
42-
"Authorization": `Bearer ${accessToken}`,
42+
Authorization: `Bearer ${accessToken}`,
4343
};
4444

4545
const userAgent = userAgentProvider?.();
4646
if (userAgent) {
4747
headers["User-Agent"] = userAgent;
4848
}
4949

50-
const res = await fetch(url, {
50+
const response = await fetch(url, {
5151
method: "GET",
5252
headers,
5353
});
5454

55-
if (!res.ok) {
56-
throw new Error(`Azure DevOps Test Plan API error: ${res.status} ${res.statusText}`);
55+
if (!response.ok) {
56+
const errorText = await response.text();
57+
throw new Error(`Failed to list test plans (${response.status}): ${errorText}`);
5758
}
5859

59-
const body = await res.json();
60+
const body = await response.json();
6061
const testPlans = body.value ?? [];
61-
const nextToken = res.headers.get("x-ms-continuationtoken") ?? undefined;
62+
const nextToken = response.headers.get("x-ms-continuationtoken") ?? undefined;
6263

63-
const response: { testPlans: typeof testPlans; continuationToken?: string } = {
64+
const result: { testPlans: typeof testPlans; continuationToken?: string } = {
6465
testPlans: testPlans,
6566
};
6667
if (nextToken) {
67-
response.continuationToken = nextToken;
68+
result.continuationToken = nextToken;
6869
}
6970

7071
return {
71-
content: [{ type: "text", text: JSON.stringify(response, null, 2) }],
72+
content: [{ type: "text", text: JSON.stringify(result, null, 2) }],
7273
};
7374
} catch (error) {
7475
const errorMessage = error instanceof Error ? error.message : "Unknown error occurred";
@@ -368,39 +369,41 @@ function configureTestPlanTools(server: McpServer, tokenProvider: () => Promise<
368369
try {
369370
const connection = await connectionProvider();
370371
const accessToken = await tokenProvider();
371-
const url = `${connection.serverUrl}/${project}/_apis/testplan/Plans/${planid}/Suites/${suiteid}/TestCase?api-version=7.2-preview.3${continuationToken ? `&continuationToken=${encodeURIComponent(continuationToken)}` : ""}`;
372+
const params = new URLSearchParams({ "api-version": "7.2-preview.3" });
373+
if (continuationToken) params.append("continuationToken", continuationToken);
374+
const url = `${connection.serverUrl}/${encodeURIComponent(project)}/_apis/testplan/Plans/${planid}/Suites/${suiteid}/TestCase?${params.toString()}`;
372375
const headers: Record<string, string> = {
373-
"Content-Type": "application/json",
374-
"Authorization": `Bearer ${accessToken}`,
376+
Authorization: `Bearer ${accessToken}`,
375377
};
376378

377379
const userAgent = userAgentProvider?.();
378380
if (userAgent) {
379381
headers["User-Agent"] = userAgent;
380382
}
381383

382-
const res = await fetch(url, {
384+
const response = await fetch(url, {
383385
method: "GET",
384386
headers,
385387
});
386388

387-
if (!res.ok) {
388-
throw new Error(`Azure DevOps Test Plan API error: ${res.status} ${res.statusText}`);
389+
if (!response.ok) {
390+
const errorText = await response.text();
391+
throw new Error(`Failed to list test cases (${response.status}): ${errorText}`);
389392
}
390393

391-
const body = await res.json();
394+
const body = await response.json();
392395
const testcases = body.value ?? [];
393-
const nextToken = res.headers.get("x-ms-continuationtoken") ?? undefined;
396+
const nextToken = response.headers.get("x-ms-continuationtoken") ?? undefined;
394397

395-
const response: { testCases: typeof testcases; continuationToken?: string } = {
398+
const result: { testCases: typeof testcases; continuationToken?: string } = {
396399
testCases: testcases,
397400
};
398401
if (nextToken) {
399-
response.continuationToken = nextToken;
402+
result.continuationToken = nextToken;
400403
}
401404

402405
return {
403-
content: [{ type: "text", text: JSON.stringify(response, null, 2) }],
406+
content: [{ type: "text", text: JSON.stringify(result, null, 2) }],
404407
};
405408
} catch (error) {
406409
const errorMessage = error instanceof Error ? error.message : "Unknown error occurred";
@@ -489,31 +492,31 @@ function configureTestPlanTools(server: McpServer, tokenProvider: () => Promise<
489492
try {
490493
const connection = await connectionProvider();
491494
const accessToken = await tokenProvider();
492-
const params = new URLSearchParams({ "api-version": "7.2-preview.1", "expand": "children" });
495+
const params = new URLSearchParams({ "api-version": apiVersion, "expand": "children" });
493496
if (continuationToken) params.append("continuationToken", continuationToken);
494-
const url = `${connection.serverUrl}/${project}/_apis/testplan/Plans/${planId}/Suites?${params.toString()}`;
497+
const url = `${connection.serverUrl}/${encodeURIComponent(project)}/_apis/testplan/Plans/${planId}/Suites?${params.toString()}`;
495498
const headers: Record<string, string> = {
496-
"Content-Type": "application/json",
497-
"Authorization": `Bearer ${accessToken}`,
499+
Authorization: `Bearer ${accessToken}`,
498500
};
499501

500502
const userAgent = userAgentProvider?.();
501503
if (userAgent) {
502504
headers["User-Agent"] = userAgent;
503505
}
504506

505-
const res = await fetch(url, {
507+
const response = await fetch(url, {
506508
method: "GET",
507509
headers,
508510
});
509511

510-
if (!res.ok) {
511-
throw new Error(`Azure DevOps Test Plan API error: ${res.status} ${res.statusText}`);
512+
if (!response.ok) {
513+
const errorText = await response.text();
514+
throw new Error(`Failed to list test suites (${response.status}): ${errorText}`);
512515
}
513516

514-
const body = await res.json();
517+
const body = await response.json();
515518
const testSuites = body.value ?? [];
516-
const nextToken = res.headers.get("x-ms-continuationtoken") ?? undefined;
519+
const nextToken = response.headers.get("x-ms-continuationtoken") ?? undefined;
517520

518521
// The API returns a flat list where the root suite is first, followed by all nested suites
519522
// We need to build a proper hierarchy by creating a map and assembling the tree
@@ -554,17 +557,17 @@ function configureTestPlanTools(server: McpServer, tokenProvider: () => Promise<
554557
return cleaned;
555558
};
556559

557-
const result = roots.map((root: any) => cleanSuite(root));
560+
const cleanedSuites = roots.map((root: any) => cleanSuite(root));
558561

559-
const response: { testSuites: typeof result; continuationToken?: string } = {
560-
testSuites: result,
562+
const result: { testSuites: typeof cleanedSuites; continuationToken?: string } = {
563+
testSuites: cleanedSuites,
561564
};
562565
if (nextToken) {
563-
response.continuationToken = nextToken;
566+
result.continuationToken = nextToken;
564567
}
565568

566569
return {
567-
content: [{ type: "text", text: JSON.stringify(response, null, 2) }],
570+
content: [{ type: "text", text: JSON.stringify(result, null, 2) }],
568571
};
569572
} catch (error) {
570573
const errorMessage = error instanceof Error ? error.message : "Unknown error occurred";

test/src/tools/test-plan.test.ts

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ describe("configureTestPlanTools", () => {
8484
});
8585

8686
describe("list_test_plans tool", () => {
87-
function mockFetchPlansResponse(value: any[], continuationToken?: string, ok = true, status = 200) {
87+
function mockFetchPlansResponse(value: any[], continuationToken?: string, ok = true, status = 200, errorText = "Not Found") {
8888
const headers = new Map<string, string>();
8989
if (continuationToken) {
9090
headers.set("x-ms-continuationtoken", continuationToken);
@@ -94,6 +94,7 @@ describe("configureTestPlanTools", () => {
9494
status,
9595
statusText: ok ? "OK" : "Not Found",
9696
json: jest.fn().mockResolvedValue({ value }),
97+
text: jest.fn().mockResolvedValue(errorText),
9798
headers: { get: (key: string) => headers.get(key) ?? null },
9899
});
99100
}
@@ -152,10 +153,24 @@ describe("configureTestPlanTools", () => {
152153
const parsed = JSON.parse(result.content[0].text);
153154
expect(parsed.continuationToken).toBe("nextPageToken");
154155
});
156+
157+
it("should handle non-ok response with status and error text", async () => {
158+
configureTestPlanTools(server, tokenProvider, connectionProvider, userAgentProvider);
159+
const call = (server.tool as jest.Mock).mock.calls.find(([toolName]) => toolName === "testplan_list_test_plans");
160+
if (!call) throw new Error("testplan_list_test_plans tool not registered");
161+
const [, , , handler] = call;
162+
163+
mockFetchPlansResponse([], undefined, false, 404, "Resource not found");
164+
165+
const result = await handler({ project: "proj1", filterActivePlans: true, includePlanDetails: false });
166+
expect(result.isError).toBe(true);
167+
expect(result.content[0].text).toContain("Failed to list test plans (404)");
168+
expect(result.content[0].text).toContain("Resource not found");
169+
});
155170
});
156171

157172
describe("list_test_suites tool", () => {
158-
function mockFetchSuitesResponse(value: any[], continuationToken?: string, ok = true, status = 200) {
173+
function mockFetchSuitesResponse(value: any[], continuationToken?: string, ok = true, status = 200, errorText = "Not Found") {
159174
const headers = new Map<string, string>();
160175
if (continuationToken) {
161176
headers.set("x-ms-continuationtoken", continuationToken);
@@ -165,6 +180,7 @@ describe("configureTestPlanTools", () => {
165180
status,
166181
statusText: ok ? "OK" : "Not Found",
167182
json: jest.fn().mockResolvedValue({ value }),
183+
text: jest.fn().mockResolvedValue(errorText),
168184
headers: { get: (key: string) => headers.get(key) ?? null },
169185
});
170186
}
@@ -406,6 +422,20 @@ describe("configureTestPlanTools", () => {
406422
expect(parsed.testSuites[0].children[0]).toEqual({ id: 501, name: "Child with no children" });
407423
expect(parsed.testSuites[0].children[0].children).toBeUndefined();
408424
});
425+
426+
it("should handle non-ok response with status and error text", async () => {
427+
configureTestPlanTools(server, tokenProvider, connectionProvider, userAgentProvider);
428+
const call = (server.tool as jest.Mock).mock.calls.find(([toolName]) => toolName === "testplan_list_test_suites");
429+
if (!call) throw new Error("testplan_list_test_suites tool not registered");
430+
const [, , , handler] = call;
431+
432+
mockFetchSuitesResponse([], undefined, false, 404, "Suite not found");
433+
434+
const result = await handler({ project: "proj1", planId: 1 });
435+
expect(result.isError).toBe(true);
436+
expect(result.content[0].text).toContain("Failed to list test suites (404)");
437+
expect(result.content[0].text).toContain("Suite not found");
438+
});
409439
});
410440

411441
describe("create_test_plan tool", () => {
@@ -578,7 +608,7 @@ describe("configureTestPlanTools", () => {
578608
});
579609

580610
describe("list_test_cases tool", () => {
581-
function mockFetchResponse(value: any[], continuationToken?: string, ok = true, status = 200) {
611+
function mockFetchResponse(value: any[], continuationToken?: string, ok = true, status = 200, errorText = "Not Found") {
582612
const headers = new Map<string, string>();
583613
if (continuationToken) {
584614
headers.set("x-ms-continuationtoken", continuationToken);
@@ -588,6 +618,7 @@ describe("configureTestPlanTools", () => {
588618
status,
589619
statusText: ok ? "OK" : "Not Found",
590620
json: jest.fn().mockResolvedValue({ value }),
621+
text: jest.fn().mockResolvedValue(errorText),
591622
headers: { get: (key: string) => headers.get(key) ?? null },
592623
});
593624
}
@@ -649,6 +680,20 @@ describe("configureTestPlanTools", () => {
649680
expect(parsed.continuationToken).toBeUndefined();
650681
expect(parsed.testCases).toEqual([{ id: 1, name: "Test Case 1" }]);
651682
});
683+
684+
it("should handle non-ok response with status and error text", async () => {
685+
configureTestPlanTools(server, tokenProvider, connectionProvider, userAgentProvider);
686+
const call = (server.tool as jest.Mock).mock.calls.find(([toolName]) => toolName === "testplan_list_test_cases");
687+
if (!call) throw new Error("testplan_list_test_cases tool not registered");
688+
const [, , , handler] = call;
689+
690+
mockFetchResponse([], undefined, false, 404, "Test case not found");
691+
692+
const result = await handler({ project: "proj1", planid: 1, suiteid: 2 });
693+
expect(result.isError).toBe(true);
694+
expect(result.content[0].text).toContain("Failed to list test cases (404)");
695+
expect(result.content[0].text).toContain("Test case not found");
696+
});
652697
});
653698

654699
describe("test_results_from_build_id tool", () => {

0 commit comments

Comments
 (0)