diff --git a/src/lib/apiClient.ts b/src/lib/apiClient.ts index e902d1e9..591d1636 100644 --- a/src/lib/apiClient.ts +++ b/src/lib/apiClient.ts @@ -150,8 +150,18 @@ class ApiClient { const res = await fn(this.axiosAgent); return new ApiResponse(res); } catch (error: any) { - if (error.response && !raise_error) { - return new ApiResponse(error.response); + if (error.response) { + if (!raise_error) { + return new ApiResponse(error.response); + } + const body = error.response.data; + const serverMessage = + typeof body === "string" + ? body + : (body?.message ?? body?.error ?? JSON.stringify(body)); + if (serverMessage) { + error.message = `Request failed with status code ${error.response.status}: ${serverMessage}`; + } } throw error; } diff --git a/src/tools/testmanagement-utils/TCG-utils/api.ts b/src/tools/testmanagement-utils/TCG-utils/api.ts index c9fc19ee..447be4e8 100644 --- a/src/tools/testmanagement-utils/TCG-utils/api.ts +++ b/src/tools/testmanagement-utils/TCG-utils/api.ts @@ -5,16 +5,27 @@ import { FETCH_DETAILS_URL, FORM_FIELDS_URL, BULK_CREATE_URL, + TC_DETAILS_MAX_BATCH, + BULK_CREATE_MAX_BATCH, + MAX_SCENARIOS_PER_DOCUMENT, } from "./config.js"; import { DefaultFieldMaps, Scenario, CreateTestCasesFromFileArgs, } from "./types.js"; -import { createTestCasePayload } from "./helpers.js"; +import { + createTestCasePayload, + chunkArray, + canAcceptScenario, +} from "./helpers.js"; import { getBrowserStackAuth } from "../../../lib/get-auth.js"; import { BrowserStackConfig } from "../../../lib/types.js"; import { getTMBaseURL } from "../../../lib/tm-base-url.js"; +import logger from "../../../logger.js"; + +const POLL_INTERVAL_MS = 10000; +const MAX_POLL_DURATION_MS = 8 * 60 * 1000; /** * Fetch default and custom form fields for a project. @@ -132,6 +143,7 @@ export async function fetchTestCaseDetails( export async function pollTestCaseDetails( traceRequestId: string, config: BrowserStackConfig, + deadline: number = Date.now() + MAX_POLL_DURATION_MS, ): Promise> { const detailMap: Record = {}; let done = false; @@ -140,7 +152,12 @@ export async function pollTestCaseDetails( while (!done) { // add a bit of jitter to avoid synchronized polling storms - await new Promise((r) => setTimeout(r, 10000 + Math.random() * 5000)); + await new Promise((r) => + setTimeout(r, POLL_INTERVAL_MS + Math.random() * 5000), + ); + + // Give up before the backend key TTL expires; return whatever we collected. + if (Date.now() > deadline) break; const poll = await apiClient.post({ url: `${TCG_POLL_URL_VALUE}?x-bstack-traceRequestId=${encodeURIComponent(traceRequestId)}`, @@ -148,10 +165,14 @@ export async function pollTestCaseDetails( "API-TOKEN": getBrowserStackAuth(config), }, body: {}, + // Don't throw on a non-2xx: an expired request key returns 400 + // ("Request ids does not exists") and simply means there is nothing more + // to fetch — stop gracefully instead of failing the whole run. + raise_error: false, }); - if (!poll.data.data.success) { - throw new Error(`Polling failed: ${poll.data.data.message}`); + if (poll.status !== 200 || !poll.data?.data?.success) { + break; } for (const msg of poll.data.data.message) { @@ -189,10 +210,14 @@ export async function pollScenariosTestDetails( let iteratorCount = 0; const tmBaseUrl = await getTMBaseURL(config); const TCG_POLL_URL_VALUE = TCG_POLL_URL(tmBaseUrl); + const deadline = Date.now() + MAX_POLL_DURATION_MS; // Promisify interval-style polling using a wrapper await new Promise((resolve, reject) => { - const intervalId = setInterval(async () => { + let stopped = false; + + const pollOnce = async () => { + if (stopped) return; try { const poll = await apiClient.post({ url: `${TCG_POLL_URL_VALUE}?x-bstack-traceRequestId=${encodeURIComponent(traceId)}`, @@ -200,18 +225,40 @@ export async function pollScenariosTestDetails( "API-TOKEN": getBrowserStackAuth(config), }, body: {}, + raise_error: false, }); if (poll.status !== 200) { - clearInterval(intervalId); - reject(new Error(`Polling error: ${poll.statusText || poll.status}`)); + stopped = true; + if (Object.keys(scenariosMap).length > 0) { + resolve(); + } else { + reject( + new Error( + `Polling error: ${poll.status} ${typeof poll.data === "string" ? poll.data : JSON.stringify(poll.data)}`, + ), + ); + } return; } + let terminated = false; for (const msg of poll.data.data.message) { if (msg.type === "scenario") { msg.data.scenarios.forEach((sc: any) => { - scenariosMap[sc.id] = { id: sc.id, name: sc.name, testcases: [] }; + if ( + canAcceptScenario( + scenariosMap, + sc.id, + MAX_SCENARIOS_PER_DOCUMENT, + ) + ) { + scenariosMap[sc.id] ||= { + id: sc.id, + name: sc.name, + testcases: [], + }; + } }); const count = Object.keys(scenariosMap).length; await context.sendNotification({ @@ -227,23 +274,32 @@ export async function pollScenariosTestDetails( if (msg.type === "testcase") { const sc = msg.data.scenario; - if (sc) { + if ( + sc && + canAcceptScenario(scenariosMap, sc.id, MAX_SCENARIOS_PER_DOCUMENT) + ) { const array = Array.isArray(msg.data.testcases) ? msg.data.testcases : msg.data.testcases ? [msg.data.testcases] : []; - const ids = array.map((tc: any) => tc.id || tc.test_case_id); - - const reqId = await fetchTestCaseDetails( - documentId, - folderId, - projectReferenceId, - ids, - source, - config, + const ids: string[] = array.map( + (tc: any) => tc.id || tc.test_case_id, ); - detailPromises.push(pollTestCaseDetails(reqId, config)); + + for (const idChunk of chunkArray(ids, TC_DETAILS_MAX_BATCH)) { + const reqId = await fetchTestCaseDetails( + documentId, + folderId, + projectReferenceId, + idChunk, + source, + config, + ); + detailPromises.push( + pollTestCaseDetails(reqId, config, deadline), + ); + } scenariosMap[sc.id] ||= { id: sc.id, @@ -267,20 +323,41 @@ export async function pollScenariosTestDetails( } if (msg.type === "termination") { - clearInterval(intervalId); - resolve(); + terminated = true; } } + + if (terminated || Date.now() > deadline) { + stopped = true; + logger.info( + `TCG scenario poll stopped (${terminated ? "termination received" : "max duration reached"}); ${Object.keys(scenariosMap).length} scenarios, ${detailPromises.length} detail fetches`, + ); + resolve(); + return; + } + setTimeout(pollOnce, POLL_INTERVAL_MS); } catch (err) { - clearInterval(intervalId); + stopped = true; reject(err); } - }, 10000); // 10 second interval + }; + setTimeout(pollOnce, POLL_INTERVAL_MS); }); - // once all detail fetches are triggered, wait for them to complete - const detailsList = await Promise.all(detailPromises); - const allDetails = detailsList.reduce((acc, cur) => ({ ...acc, ...cur }), {}); + const detailsList = await Promise.allSettled(detailPromises); + const rejectedDetails = detailsList.filter( + (r) => r.status === "rejected", + ).length; + if (rejectedDetails > 0) { + logger.info( + `TCG detail fetches: ${detailsList.length - rejectedDetails}/${detailsList.length} succeeded, ${rejectedDetails} failed (degrading gracefully)`, + ); + } + const allDetails = detailsList.reduce>( + (acc, result) => + result.status === "fulfilled" ? { ...acc, ...result.value } : acc, + {}, + ); // attach the fetched detail objects back to each testcase for (const scenario of Object.values(scenariosMap)) { @@ -307,41 +384,65 @@ export async function bulkCreateTestCases( documentId: number, config: BrowserStackConfig, ): Promise { - const results: Record = {}; const total = Object.keys(scenariosMap).length; let doneCount = 0; let testCaseCount = 0; + const failedScenarios: string[] = []; const tmBaseUrl = await getTMBaseURL(config); const BULK_CREATE_URL_VALUE = BULK_CREATE_URL(tmBaseUrl, projectId, folderId); for (const { id, testcases } of Object.values(scenariosMap)) { - const testCaseLength = testcases.length; - testCaseCount += testCaseLength; - if (testCaseLength === 0) continue; - const payload = { - test_cases: testcases.map((tc) => - createTestCasePayload( - tc, - id, - folderId, - fieldMaps, - documentId, - booleanFieldId, - traceId, + if (testcases.length === 0) continue; + + const batches = chunkArray(testcases, BULK_CREATE_MAX_BATCH); + let createdInScenario = 0; + let scenarioFailed = false; + + for (const batch of batches) { + const payload = { + test_cases: batch.map((tc) => + createTestCasePayload( + tc, + id, + folderId, + fieldMaps, + documentId, + booleanFieldId, + traceId, + ), ), - ), - }; + }; - try { - const resp = await apiClient.post({ - url: BULK_CREATE_URL_VALUE, - headers: { - "API-TOKEN": getBrowserStackAuth(config), - "Content-Type": "application/json", - }, - body: payload, - }); - results[id] = resp.data; + try { + await apiClient.post({ + url: BULK_CREATE_URL_VALUE, + headers: { + "API-TOKEN": getBrowserStackAuth(config), + "Content-Type": "application/json", + }, + body: payload, + }); + createdInScenario += batch.length; + } catch (error) { + scenarioFailed = true; + await context.sendNotification({ + method: "notifications/progress", + params: { + progressToken: context._meta?.progressToken ?? traceId, + message: `Creation failed for scenario ${id}: ${error instanceof Error ? error.message : "Unknown error"}`, + total, + progress: doneCount, + }, + }); + } + } + + testCaseCount += createdInScenario; + if (scenarioFailed) { + failedScenarios.push(id); + } + if (createdInScenario > 0) { + doneCount++; await context.sendNotification({ method: "notifications/progress", params: { @@ -351,23 +452,12 @@ export async function bulkCreateTestCases( progress: doneCount, }, }); - } catch (error) { - //send notification - await context.sendNotification({ - method: "notifications/progress", - params: { - progressToken: context._meta?.progressToken ?? traceId, - message: `Creation failed for scenario ${id}: ${error instanceof Error ? error.message : "Unknown error"}`, - total, - progress: doneCount, - }, - }); - //continue to next scenario - continue; } - doneCount++; } - const resultString = `Total of ${testCaseCount} test cases created in ${total} scenarios.`; + let resultString = `Total of ${testCaseCount} test cases created in ${doneCount} of ${total} scenarios.`; + if (failedScenarios.length > 0) { + resultString += ` Failed to create test cases for ${failedScenarios.length} scenario(s): ${failedScenarios.join(", ")}.`; + } return resultString; } diff --git a/src/tools/testmanagement-utils/TCG-utils/config.ts b/src/tools/testmanagement-utils/TCG-utils/config.ts index bca26ec4..5b50a1bd 100644 --- a/src/tools/testmanagement-utils/TCG-utils/config.ts +++ b/src/tools/testmanagement-utils/TCG-utils/config.ts @@ -1,3 +1,10 @@ +export const TC_DETAILS_MAX_BATCH = 10; + +export const BULK_CREATE_MAX_BATCH = 10; + +// Cap scenarios per document (mirrors TCG's former maxScenariosPerDocument=10). +export const MAX_SCENARIOS_PER_DOCUMENT = 10; + export const TCG_TRIGGER_URL = (baseUrl: string) => `${baseUrl}/api/v1/integration/tcg/test-generation/suggest-test-cases`; diff --git a/src/tools/testmanagement-utils/TCG-utils/helpers.ts b/src/tools/testmanagement-utils/TCG-utils/helpers.ts index 1a25328e..9bb2e746 100644 --- a/src/tools/testmanagement-utils/TCG-utils/helpers.ts +++ b/src/tools/testmanagement-utils/TCG-utils/helpers.ts @@ -28,6 +28,32 @@ export function findBooleanFieldId(customFields: any[]): number | undefined { return boolField?.id; } +// True if the scenario is already tracked or we're still under `max`. +export function canAcceptScenario( + scenariosMap: Record, + id: string, + max: number, +): boolean { + return ( + scenariosMap[id] !== undefined || Object.keys(scenariosMap).length < max + ); +} + +/** + * Split an array into consecutive chunks of at most `size` items. + * Used to keep test-case-detail fetches within the backend's per-request limit. + */ +export function chunkArray(items: T[], size: number): T[][] { + if (size < 1) { + throw new Error("chunk size must be at least 1"); + } + const chunks: T[][] = []; + for (let i = 0; i < items.length; i += size) { + chunks.push(items.slice(i, i + size)); + } + return chunks; +} + /** * Construct payload for creating a single test case in bulk. */ diff --git a/tests/lib/apiClient.test.ts b/tests/lib/apiClient.test.ts new file mode 100644 index 00000000..86d6466e --- /dev/null +++ b/tests/lib/apiClient.test.ts @@ -0,0 +1,83 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const { postMock } = vi.hoisted(() => ({ postMock: vi.fn() })); + +vi.mock("axios", () => ({ + default: { + create: () => ({ + get: vi.fn(), + post: postMock, + put: vi.fn(), + patch: vi.fn(), + delete: vi.fn(), + }), + }, +})); + +vi.mock("../../src/config", () => ({ + default: { + browserstackLocalOptions: {}, + }, +})); + +// utils.ts (a transitive import of apiClient) imports trackMCP from index.ts, +// whose top-level main() would otherwise run on import. Stub it out. +vi.mock("../../src/index", () => ({ + trackMCP: vi.fn(), +})); + +import { apiClient } from "../../src/lib/apiClient"; + +describe("apiClient error surfacing", () => { + beforeEach(() => { + postMock.mockReset(); + }); + + it("includes the server's JSON error message in the thrown error", async () => { + postMock.mockRejectedValueOnce({ + message: "Request failed with status code 400", + response: { + status: 400, + data: { + success: false, + message: "Max Testcase IDs supported limit exceeded.", + }, + }, + }); + + await expect( + apiClient.post({ url: "https://example.com/api", body: {} }), + ).rejects.toMatchObject({ + message: + "Request failed with status code 400: Max Testcase IDs supported limit exceeded.", + }); + }); + + it("falls back to a string error body", async () => { + postMock.mockRejectedValueOnce({ + message: "Request failed with status code 422", + response: { status: 422, data: "Unprocessable Entity" }, + }); + + await expect( + apiClient.post({ url: "https://example.com/api", body: {} }), + ).rejects.toMatchObject({ + message: "Request failed with status code 422: Unprocessable Entity", + }); + }); + + it("returns the response instead of throwing when raise_error is false", async () => { + postMock.mockRejectedValueOnce({ + message: "Request failed with status code 400", + response: { status: 400, data: { success: false } }, + }); + + const res = await apiClient.post({ + url: "https://example.com/api", + body: {}, + raise_error: false, + }); + expect(res.status).toBe(400); + expect(res.data).toEqual({ success: false }); + }); +}); diff --git a/tests/tools/bulk-create-chunking.test.ts b/tests/tools/bulk-create-chunking.test.ts new file mode 100644 index 00000000..f233ed7b --- /dev/null +++ b/tests/tools/bulk-create-chunking.test.ts @@ -0,0 +1,111 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +const { postMock } = vi.hoisted(() => ({ postMock: vi.fn() })); + +vi.mock("../../src/lib/apiClient", () => ({ + apiClient: { post: postMock, get: vi.fn() }, +})); +vi.mock("../../src/lib/tm-base-url", () => ({ + getTMBaseURL: vi.fn().mockResolvedValue("https://tm.example.com"), +})); +vi.mock("../../src/lib/get-auth", () => ({ + getBrowserStackAuth: vi.fn().mockReturnValue("user:key"), +})); + +import { bulkCreateTestCases } from "../../src/tools/testmanagement-utils/TCG-utils/api"; + +const fieldMaps = { + priority: {}, + status: { active: "active" }, + caseType: { functional: "functional" }, +} as any; + +const context = { + sendNotification: vi.fn().mockResolvedValue(undefined), + _meta: {}, +}; + +function scenario(id: string, n: number) { + return { + id, + name: id, + testcases: Array.from({ length: n }, (_, i) => ({ + name: `tc-${i}`, + steps: [], + priority: "Medium", + })), + }; +} + +function callSizes() { + return postMock.mock.calls.map((c: any) => c[0].body.test_cases.length); +} + +describe("bulkCreateTestCases chunking", () => { + beforeEach(() => { + postMock.mockReset(); + postMock.mockResolvedValue({ data: { success: true } }); + context.sendNotification.mockClear(); + }); + + it("splits a >10-case scenario into <=10-case requests (23 -> 10+10+3)", async () => { + const scenariosMap = { s1: scenario("s1", 23) } as any; + const result = await bulkCreateTestCases( + scenariosMap, + "proj", + "folder", + fieldMaps, + undefined, + "trace", + context, + 1, + {} as any, + ); + + expect(postMock).toHaveBeenCalledTimes(3); + expect(callSizes()).toEqual([10, 10, 3]); + expect(callSizes().every((s: number) => s <= 10)).toBe(true); + expect(result).toContain("Total of 23 test cases created in 1 of 1 scenarios"); + }); + + it("counts only created cases and reports a scenario whose batch failed", async () => { + // 15 cases -> 2 batches (10, 5); first ok, second rejected + postMock + .mockResolvedValueOnce({ data: {} }) + .mockRejectedValueOnce(new Error("More than permitted test cases sent")); + + const scenariosMap = { s1: scenario("s1", 15) } as any; + const result = await bulkCreateTestCases( + scenariosMap, + "proj", + "folder", + fieldMaps, + undefined, + "trace", + context, + 1, + {} as any, + ); + + expect(postMock).toHaveBeenCalledTimes(2); + expect(result).toContain("Total of 10 test cases created"); + expect(result).toContain("Failed to create test cases for 1 scenario"); + }); + + it("does not call the API for an empty scenario", async () => { + const scenariosMap = { s1: scenario("s1", 0) } as any; + const result = await bulkCreateTestCases( + scenariosMap, + "proj", + "folder", + fieldMaps, + undefined, + "trace", + context, + 1, + {} as any, + ); + expect(postMock).not.toHaveBeenCalled(); + expect(result).toContain("Total of 0 test cases created in 0 of 1 scenarios"); + }); +}); diff --git a/tests/tools/tcg-helpers.test.ts b/tests/tools/tcg-helpers.test.ts new file mode 100644 index 00000000..c231d374 --- /dev/null +++ b/tests/tools/tcg-helpers.test.ts @@ -0,0 +1,50 @@ +import { describe, it, expect } from "vitest"; +import { + chunkArray, + canAcceptScenario, +} from "../../src/tools/testmanagement-utils/TCG-utils/helpers"; +import { TC_DETAILS_MAX_BATCH } from "../../src/tools/testmanagement-utils/TCG-utils/config"; + +describe("chunkArray", () => { + it("returns an empty array when given no items", () => { + expect(chunkArray([], 10)).toEqual([]); + }); + + it("keeps a single chunk when items fit within the size", () => { + expect(chunkArray([1, 2, 3], 10)).toEqual([[1, 2, 3]]); + }); + + it("does not split at exactly the batch size", () => { + const ids = Array.from({ length: TC_DETAILS_MAX_BATCH }, (_, i) => i + 1); + expect(chunkArray(ids, TC_DETAILS_MAX_BATCH)).toEqual([ids]); + }); + + it("splits a scenario with more than the batch size into <=size chunks", () => { + // 23 test cases in one scenario -> 10 + 10 + 3, the PMAA-147 case. + const ids = Array.from({ length: 23 }, (_, i) => i + 1); + const chunks = chunkArray(ids, TC_DETAILS_MAX_BATCH); + expect(chunks).toHaveLength(3); + expect(chunks.every((c) => c.length <= TC_DETAILS_MAX_BATCH)).toBe(true); + expect(chunks.flat()).toEqual(ids); + }); + + it("throws on a chunk size below 1", () => { + expect(() => chunkArray([1, 2], 0)).toThrow(); + }); +}); + +describe("canAcceptScenario", () => { + it("accepts while under the cap", () => { + expect(canAcceptScenario({ a: 1 }, "b", 3)).toBe(true); + }); + + it("rejects a new scenario once the cap is reached", () => { + const map = { a: 1, b: 1, c: 1 }; + expect(canAcceptScenario(map, "d", 3)).toBe(false); + }); + + it("still accepts an already-tracked scenario at the cap", () => { + const map = { a: 1, b: 1, c: 1 }; + expect(canAcceptScenario(map, "a", 3)).toBe(true); + }); +});