Skip to content

Commit 85aed79

Browse files
betegoncodex
andcommitted
refactor(tools): simplify suspect commit regression coverage
Reuse the structured-content test helper, consolidate lookup failure cases, and remove redundant absence checks and an unused type. Bind event fixtures to the requested endpoint with a unique ID so event-selection regressions cannot use shared fallback data. Co-Authored-By: GPT-6 <noreply@openai.com>
1 parent 3bdc0bc commit 85aed79

2 files changed

Lines changed: 55 additions & 71 deletions

File tree

‎packages/mcp-core/src/api-client/types.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ import type {
5858
ClientKeySchema,
5959
CommitListSchema,
6060
CommitSchema,
61-
CommitterSchema,
6261
CommittersResponseSchema,
6362
DashboardListItemSchema,
6463
DashboardSchema,
@@ -156,7 +155,6 @@ export type Release = z.infer<typeof ReleaseSchema>;
156155
export type ReleaseDetails = z.infer<typeof ReleaseDetailsSchema>;
157156
export type Deploy = z.infer<typeof DeploySchema>;
158157
export type Commit = z.infer<typeof CommitSchema>;
159-
export type Committer = z.infer<typeof CommitterSchema>;
160158
export type Issue = z.infer<typeof IssueSchema>;
161159
export type IssueActivity = z.infer<typeof IssueActivitySchema>;
162160
export type IssueComment = z.infer<typeof IssueCommentSchema>;
@@ -203,7 +201,9 @@ export type MetricAlertRuleList = z.infer<typeof MetricAlertRuleListSchema>;
203201
export type ReleaseList = z.infer<typeof ReleaseListSchema>;
204202
export type DeployList = z.infer<typeof DeployListSchema>;
205203
export type CommitList = z.infer<typeof CommitListSchema>;
206-
export type CommitterList = z.infer<typeof CommittersResponseSchema>["committers"];
204+
export type CommitterList = z.infer<
205+
typeof CommittersResponseSchema
206+
>["committers"];
207207
export type IssueList = z.infer<typeof IssueListSchema>;
208208
export type IssueActivityList = z.infer<
209209
typeof IssueActivityListResponseSchema

‎packages/mcp-core/src/tools/catalog/get-issue-details.test.ts‎

Lines changed: 52 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,14 @@ import {
1111
issueNullCulpritFixture,
1212
mswServer,
1313
} from "@sentry/mcp-server-mocks";
14-
import { http, HttpResponse } from "msw";
14+
import { HttpResponse, http } from "msw";
1515
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
1616
import type { Skill } from "../../skills";
1717
import * as logging from "../../telem/logging";
18-
import { getTextContent } from "../../test-utils/structured-content";
18+
import {
19+
getStructuredContent,
20+
getTextContent,
21+
} from "../../test-utils/structured-content";
1922
import getIssueDetails, {
2023
getIssueDetailsOutputSchema,
2124
} from "./get-issue-details.js";
@@ -2661,6 +2664,7 @@ describe("structuredContent", () => {
26612664

26622665
describe("suspect commits", () => {
26632666
const sha = "2ce6a2700fec4913a2cde8e2d41dee362ce6a270";
2667+
const fixtureEventId = "8d17c61b471a4a2ab0c79b32cae564ef";
26642668
const params = {
26652669
organizationSlug: "sentry-mcp-evals",
26662670
issueId: "CLOUDFLARE-MCP-41",
@@ -2672,14 +2676,23 @@ describe("suspect commits", () => {
26722676
format: "json",
26732677
content: JSON.stringify({ title: { text: "Example error" } }),
26742678
};
2675-
const committersUrl =
2676-
"https://sentry.io/api/0/projects/sentry-mcp-evals/CLOUDFLARE-MCP/events/abc123def456/committers/";
2679+
const committersUrl = `https://sentry.io/api/0/projects/sentry-mcp-evals/CLOUDFLARE-MCP/events/${fixtureEventId}/committers/`;
26772680

2678-
function mockEvent(options: { type?: string; formatted?: unknown } = {}) {
2681+
function mockEvent(
2682+
options: { type?: string; formatted?: unknown } = {},
2683+
eventSelector = "latest",
2684+
) {
26792685
mswServer.use(
26802686
http.get(
2681-
"https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/6507376925/events/:eventId/",
2682-
() => HttpResponse.json({ ...createDefaultEvent(), ...options }),
2687+
`https://sentry.io/api/0/organizations/sentry-mcp-evals/issues/6507376925/events/${eventSelector}/`,
2688+
() =>
2689+
HttpResponse.json({
2690+
...createDefaultEvent({
2691+
id: fixtureEventId,
2692+
eventID: fixtureEventId,
2693+
}),
2694+
...options,
2695+
}),
26832696
),
26842697
);
26852698
}
@@ -2709,15 +2722,15 @@ describe("suspect commits", () => {
27092722
{ selection: "latest event", eventId: undefined },
27102723
{
27112724
selection: "explicit event ID",
2712-
eventId: "7ca573c0f4814912aaa9bdc77d1a7d51",
2725+
eventId: fixtureEventId,
27132726
},
27142727
])(
27152728
"includes the suspect commit for the $selection",
27162729
async ({ eventId }) => {
2717-
mockEvent({ type, formatted });
2730+
mockEvent({ type, formatted }, eventId);
27182731
mswServer.use(
2719-
http.get(committersUrl, () => {
2720-
return HttpResponse.json({
2732+
http.get(committersUrl, () =>
2733+
HttpResponse.json({
27212734
committers: [
27222735
{
27232736
author: { name: "Jane Developer", email: "jane@example.com" },
@@ -2730,8 +2743,8 @@ describe("suspect commits", () => {
27302743
],
27312744
},
27322745
],
2733-
});
2734-
}),
2746+
}),
2747+
),
27352748
);
27362749

27372750
const result = await getIssueDetails.handler(
@@ -2741,7 +2754,7 @@ describe("suspect commits", () => {
27412754

27422755
if (structured) {
27432756
const payload = getIssueDetailsOutputSchema.parse(
2744-
(result as { structuredContent: unknown }).structuredContent,
2757+
getStructuredContent(result),
27452758
);
27462759
expect(payload.suspectCommit).toEqual({
27472760
id: sha,
@@ -2783,66 +2796,33 @@ describe("suspect commits", () => {
27832796
});
27842797

27852798
it.each([
2786-
{ mode: "structured JSON", formatted, structured: true },
2787-
{ mode: "Markdown", formatted: undefined, structured: false },
2788-
])(
2789-
"omits an absent suspect commit in $mode",
2790-
async ({ formatted, structured }) => {
2791-
mockEvent({ formatted });
2792-
2793-
const result = await getIssueDetails.handler(params, baseContext);
2794-
2795-
if (structured) {
2796-
const payload = getIssueDetailsOutputSchema.parse(
2797-
(result as { structuredContent: unknown }).structuredContent,
2798-
);
2799-
expect(payload.suspectCommit).toBeNull();
2800-
} else {
2801-
expect(result).not.toContain("## Suspect Commit");
2802-
expect(result).toContain("## Event Details");
2803-
}
2799+
{
2800+
failure: "permission denied",
2801+
status: 403,
2802+
body: { detail: "Permission denied" },
2803+
reported: false,
28042804
},
2805-
);
2806-
2807-
it.each([403, 404])(
2808-
"preserves issue details without reporting expected HTTP %s failures",
2809-
async (status) => {
2810-
mockEvent({ formatted });
2811-
const logIssue = vi.spyOn(logging, "logIssue").mockReturnValue(undefined);
2812-
mswServer.use(
2813-
http.get(committersUrl, () =>
2814-
HttpResponse.json(
2815-
{ detail: "Commit tracking unavailable" },
2816-
{ status },
2817-
),
2818-
),
2819-
);
2820-
2821-
const result = await getIssueDetails.handler(params, baseContext);
2822-
const payload = getIssueDetailsOutputSchema.parse(
2823-
(result as { structuredContent: unknown }).structuredContent,
2824-
);
2825-
2826-
expect(payload.issue.shortId).toBe("CLOUDFLARE-MCP-41");
2827-
expect(payload.suspectCommit).toBeNull();
2828-
expect(logIssue).not.toHaveBeenCalled();
2805+
{
2806+
failure: "no committers found",
2807+
status: 404,
2808+
body: { detail: "No committers found" },
2809+
reported: false,
28292810
},
2830-
);
2831-
2832-
it.each([
28332811
{
28342812
failure: "server failure",
28352813
status: 500,
28362814
body: { detail: "Internal error" },
2815+
reported: true,
28372816
},
28382817
{
28392818
failure: "invalid response schema",
28402819
status: 200,
28412820
body: { committers: [{ commits: [{ message: "Missing commit ID" }] }] },
2821+
reported: true,
28422822
},
28432823
])(
2844-
"reports a $failure while preserving issue details",
2845-
async ({ status, body }) => {
2824+
"preserves issue details for $failure (reported: $reported)",
2825+
async ({ status, body, reported }) => {
28462826
mockEvent({ formatted });
28472827
const logIssue = vi.spyOn(logging, "logIssue").mockReturnValue(undefined);
28482828
mswServer.use(
@@ -2851,17 +2831,21 @@ describe("suspect commits", () => {
28512831

28522832
const result = await getIssueDetails.handler(params, baseContext);
28532833
const payload = getIssueDetailsOutputSchema.parse(
2854-
(result as { structuredContent: unknown }).structuredContent,
2834+
getStructuredContent(result),
28552835
);
28562836

28572837
expect(payload.issue.shortId).toBe("CLOUDFLARE-MCP-41");
28582838
expect(payload.suspectCommit).toBeNull();
2859-
expect(logIssue).toHaveBeenCalledExactlyOnceWith(
2860-
expect.any(Error),
2861-
expect.objectContaining({
2862-
loggerScope: ["tools", "get-issue-details", "committers"],
2863-
}),
2864-
);
2839+
if (reported) {
2840+
expect(logIssue).toHaveBeenCalledExactlyOnceWith(
2841+
expect.any(Error),
2842+
expect.objectContaining({
2843+
loggerScope: ["tools", "get-issue-details", "committers"],
2844+
}),
2845+
);
2846+
} else {
2847+
expect(logIssue).not.toHaveBeenCalled();
2848+
}
28652849
},
28662850
);
28672851
});

0 commit comments

Comments
 (0)