From 0bf856adbb656b9aa86973aeaad6a6a2b9c73c5f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 18:10:46 +0000 Subject: [PATCH 1/2] ci: keep comment-cop working when GITHUB_TOKEN's GraphQL quota is exhausted The scan step fetched the PR's review threads through one unguarded GraphQL query before doing anything else, so whenever the repo's shared GraphQL quota was used up the step died with an unhandled GraphqlResponseError and the check went red on every claude-labeled PR at once. Dedup now reads the PR's review comments over REST, which draws from a separate quota and is the same quota the rest of the step already needs. GraphQL is only used to find the unresolved threads whose flagged block left the diff; that lookup runs only when such a key exists and a failure is a warning, after which the scan still posts. source-lints runs the new test, which executes the embedded script against a fake github object, whenever comment-cop.yml changes. --- .github/workflows/comment-cop.yml | 46 +++- .github/workflows/source-lints.yml | 2 + .../internal/source-lints/comment-cop.test.ts | 231 ++++++++++++++++++ 3 files changed, 266 insertions(+), 13 deletions(-) create mode 100644 test/internal/source-lints/comment-cop.test.ts diff --git a/.github/workflows/comment-cop.yml b/.github/workflows/comment-cop.yml index f6cf8005a8d2..db3351ae5d3a 100644 --- a/.github/workflows/comment-cop.yml +++ b/.github/workflows/comment-cop.yml @@ -104,10 +104,26 @@ jobs: const keyFor = g => `${g.path}:${crypto.createHash('sha256').update(g.text).digest('hex').slice(0, 12)}`; const presentKeys = new Set(groups.map(keyFor)); + const MARKER = //; - // Fetch existing comment-cop review threads (for dedup + auto-resolve). - const threads = []; - { + // Dedup goes through REST on purpose: GITHUB_TOKEN's GraphQL quota is + // shared by every workflow run in the repo and is routinely exhausted, + // the REST quota is a separate pool, and a scan that cannot dedup + // would re-post every comment already on the PR. + const reviewComments = await github.paginate(github.rest.pulls.listReviewComments, { + owner, repo, pull_number, per_page: 100, + }); + const seenKeys = new Set(); + for (const c of reviewComments) { + const m = MARKER.exec(c.body || ''); + if (m) seenKeys.add(m[1]); + } + const staleKeys = new Set([...seenKeys].filter(k => !presentKeys.has(k))); + + // Ids of the unresolved threads for staleKeys. Thread ids and resolved + // state only exist in GraphQL, so this lookup is best effort: when it + // fails, nothing gets resolved and the scan below still runs. + async function staleThreadIds() { const q = ` query($owner: String!, $repo: String!, $pr: Int!, $after: String) { repository(owner: $owner, name: $repo) { @@ -119,25 +135,29 @@ jobs: } } }`; + const ids = []; let after = null; for (;;) { const res = await github.graphql(q, { owner, repo, pr: pull_number, after }); const page = res.repository.pullRequest.reviewThreads; - for (const t of page.nodes) threads.push(t); + for (const t of page.nodes) { + if (t.isResolved) continue; + const m = MARKER.exec(t.comments.nodes[0]?.body || ''); + if (m && staleKeys.has(m[1])) ids.push(t.id); + } if (!page.pageInfo.hasNextPage) break; after = page.pageInfo.endCursor; } + return ids; } - const seenKeys = new Set(); - const toResolve = []; - for (const t of threads) { - const body = t.comments.nodes[0]?.body || ''; - const m = //.exec(body); - if (!m) continue; - const key = m[1]; - seenKeys.add(key); - if (!t.isResolved && !presentKeys.has(key)) toResolve.push(t.id); + let toResolve = []; + if (staleKeys.size > 0) { + try { + toResolve = await staleThreadIds(); + } catch (e) { + core.warning(`Listing review threads failed, not resolving up to ${staleKeys.size} stale comment-cop thread(s): ${e.message}`); + } } // Auto-resolve threads whose flagged block is gone from the current diff. diff --git a/.github/workflows/source-lints.yml b/.github/workflows/source-lints.yml index afeebaf9aa0c..582a1e11da7c 100644 --- a/.github/workflows/source-lints.yml +++ b/.github/workflows/source-lints.yml @@ -24,6 +24,7 @@ on: - "test/_util/**" - "test/internal/source-lints/**" - ".github/workflows/source-lints.yml" + - ".github/workflows/comment-cop.yml" - ".github/actions/setup-bun/**" pull_request: paths: @@ -39,6 +40,7 @@ on: - "test/_util/**" - "test/internal/source-lints/**" - ".github/workflows/source-lints.yml" + - ".github/workflows/comment-cop.yml" - ".github/actions/setup-bun/**" merge_group: diff --git a/test/internal/source-lints/comment-cop.test.ts b/test/internal/source-lints/comment-cop.test.ts new file mode 100644 index 000000000000..d293793c847e --- /dev/null +++ b/test/internal/source-lints/comment-cop.test.ts @@ -0,0 +1,231 @@ +/** + * The script embedded in .github/workflows/comment-cop.yml, run the way + * actions/github-script runs it (as an async function body with `github`, + * `context`, `core` and `require` in scope) against a fake `github` that + * records writes instead of sending them. + * + * The scenarios pin the workflow's dependence on the two GitHub API quotas: + * GITHUB_TOKEN's GraphQL quota is shared by every workflow run in the repo and + * is routinely exhausted, so scanning, dedup and posting have to work with + * GraphQL unavailable, and only the auto-resolve of stale threads (thread ids + * and resolved state exist only in GraphQL) may depend on it, as a warning. + */ +import { describe, expect, test } from "bun:test"; +import { createHash } from "node:crypto"; +import { readFileSync } from "node:fs"; +import { createRequire } from "node:module"; +import { join } from "node:path"; + +const repoRoot = join(import.meta.dir, "..", "..", ".."); + +const workflow = Bun.YAML.parse(readFileSync(join(repoRoot, ".github", "workflows", "comment-cop.yml"), "utf8")) as { + jobs: { "comment-cop": { steps: { with?: { script?: string } }[] } }; +}; +const scanScript = workflow.jobs["comment-cop"].steps.find(step => step.with?.script)!.with!.script!; + +const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor as new ( + ...args: string[] +) => (...args: unknown[]) => Promise; +const runScan = new AsyncFunction("require", "github", "context", "core", scanScript); +const require = createRequire(import.meta.url); + +const HEAD_SHA = "0123456789abcdef0123456789abcdef01234567"; +const PATH = "src/runtime/example.rs"; + +/** Same key the workflow derives for a comment group: path plus a hash of the comment text. */ +const keyFor = (text: string) => `${PATH}:${createHash("sha256").update(text).digest("hex").slice(0, 12)}`; +const markerComment = (key: string) => `\nplease delete this comment\n`; + +// Two three-line comment groups added at lines 2-4 and 6-8 of the new file. +const firstGroup = ["// first group, line one", "// first group, line two", "// first group, line three"]; +const secondGroup = ["// second group, line one", "// second group, line two", "// second group, line three"]; +const patch = [ + "@@ -1,2 +1,9 @@", + " use bun_core::Output;", + ...firstGroup.map(line => `+${line}`), + "+fn first() {}", + ...secondGroup.map(line => `+${line}`), + "+fn second() {}", + " fn existing() {}", +].join("\n"); +const FIRST = keyFor(firstGroup.join("\n")); +const SECOND = keyFor(secondGroup.join("\n")); +/** Flagged on an earlier push, and the comment it flagged is no longer in the diff. */ +const STALE_UNRESOLVED = `${PATH}:000000000001`; +const STALE_RESOLVED = `${PATH}:000000000002`; + +const expectedPost = (key: string, start: number, end: number) => ({ + owner: "oven-sh", + repo: "bun", + pull_number: 4242, + commit_id: HEAD_SHA, + path: PATH, + start_line: start, + start_side: "RIGHT", + line: end, + side: "RIGHT", + body: expect.stringContaining(``), +}); + +const rateLimitError = () => + Object.assign( + new Error( + "Request failed due to following response errors:\n - API rate limit already exceeded for site ID installation.", + ), + { name: "GraphqlResponseError", errors: [{ type: "RATE_LIMIT", code: "graphql_rate_limit" }] }, + ); + +type Thread = { id: string; isResolved: boolean; comments: { nodes: { body: string }[] } }; +const thread = (id: string, body: string | null, isResolved = false): Thread => ({ + id, + isResolved, + comments: { nodes: body === null ? [] : [{ body }] }, +}); + +interface Scenario { + /** Bodies of the review comments already on the PR, as `pulls.listReviewComments` returns them. */ + reviewComments: string[]; + /** Pages served for the reviewThreads query; omitted means every GraphQL request fails on the rate limit. */ + threadPages?: Thread[][]; +} + +async function scan({ reviewComments, threadPages }: Scenario) { + const posted: unknown[] = []; + const resolved: string[] = []; + const graphqlRequests: string[] = []; + const restPaginated: string[] = []; + const warnings: string[] = []; + const info: string[] = []; + + const endpoint = (name: string, rows: () => T[]) => + Object.assign(async () => ({ data: rows() }), { endpointName: name }); + const github = { + rest: { + pulls: { + listFiles: endpoint("pulls.listFiles", () => [{ filename: PATH, status: "modified", patch }]), + listReviewComments: endpoint("pulls.listReviewComments", () => + reviewComments.map((body, i) => ({ id: i + 1, body, user: { login: "github-actions[bot]" } })), + ), + createReviewComment: async (params: unknown) => { + posted.push(params); + return { data: {} }; + }, + }, + }, + paginate: async (fn: { endpointName: string; (): Promise<{ data: unknown[] }> }) => { + restPaginated.push(fn.endpointName); + return (await fn()).data; + }, + graphql: async (query: string, variables: { id?: string; after?: string | null }) => { + const isMutation = /^\s*mutation\b/.test(query); + graphqlRequests.push(isMutation ? "mutation" : "query"); + if (threadPages === undefined) throw rateLimitError(); + if (isMutation) { + resolved.push(variables.id!); + return { resolveReviewThread: { thread: { id: variables.id } } }; + } + const pageIndex = variables.after == null ? 0 : Number(variables.after); + return { + repository: { + pullRequest: { + reviewThreads: { + pageInfo: { hasNextPage: pageIndex + 1 < threadPages.length, endCursor: String(pageIndex + 1) }, + nodes: threadPages[pageIndex], + }, + }, + }, + }; + }, + }; + const context = { + repo: { owner: "oven-sh", repo: "bun" }, + payload: { pull_request: { number: 4242, head: { sha: HEAD_SHA }, base: { ref: "main" } } }, + }; + const core = { + info: (message: string) => info.push(message), + warning: (message: string) => warnings.push(message), + }; + + await runScan(require, github, context, core); + return { posted, resolved, graphqlRequests, restPaginated, warnings, info }; +} + +describe("comment-cop with the GraphQL quota exhausted", () => { + test("still posts the new groups and dedups the already flagged one", async () => { + const result = await scan({ + reviewComments: [ + markerComment(FIRST), + "nit: rename this (a human review comment)", + markerComment(STALE_UNRESOLVED), + ], + }); + expect(result).toEqual({ + posted: [expectedPost(SECOND, 6, 8)], + resolved: [], + // The stale key makes it try the thread lookup once; the failure is a warning, not a failed check. + graphqlRequests: ["query"], + restPaginated: ["pulls.listFiles", "pulls.listReviewComments"], + warnings: [expect.stringContaining("API rate limit already exceeded")], + info: ["Posted 1 review comment(s)."], + }); + }); + + test("does not touch GraphQL when no flagged block has gone stale", async () => { + const result = await scan({ reviewComments: [markerComment(FIRST), "looks good"] }); + expect(result).toEqual({ + posted: [expectedPost(SECOND, 6, 8)], + resolved: [], + graphqlRequests: [], + restPaginated: ["pulls.listFiles", "pulls.listReviewComments"], + warnings: [], + info: ["Posted 1 review comment(s)."], + }); + }); + + test("a second run after posting finds its own comments through REST and posts nothing", async () => { + const firstRun = await scan({ reviewComments: [] }); + expect(firstRun.posted).toEqual([expectedPost(FIRST, 2, 4), expectedPost(SECOND, 6, 8)]); + + const secondRun = await scan({ reviewComments: firstRun.posted.map(params => (params as { body: string }).body) }); + expect(secondRun).toEqual({ + posted: [], + resolved: [], + graphqlRequests: [], + restPaginated: ["pulls.listFiles", "pulls.listReviewComments"], + warnings: [], + info: ["No new comment groups to flag (2 present, all already flagged)."], + }); + }); +}); + +describe("comment-cop with GraphQL available", () => { + test("resolves only the unresolved threads whose flagged block left the diff", async () => { + const result = await scan({ + reviewComments: [ + markerComment(FIRST), + markerComment(STALE_UNRESOLVED), + markerComment(STALE_RESOLVED), + "nit: rename this (a human review comment)", + ], + threadPages: [ + [ + thread("THREAD_FIRST", markerComment(FIRST)), + thread("THREAD_STALE_UNRESOLVED", markerComment(STALE_UNRESOLVED)), + ], + [ + thread("THREAD_STALE_RESOLVED", markerComment(STALE_RESOLVED), true), + thread("THREAD_HUMAN", "nit: rename this (a human review comment)"), + thread("THREAD_EMPTY", null), + ], + ], + }); + expect(result).toEqual({ + posted: [expectedPost(SECOND, 6, 8)], + resolved: ["THREAD_STALE_UNRESOLVED"], + graphqlRequests: ["query", "query", "mutation"], + restPaginated: ["pulls.listFiles", "pulls.listReviewComments"], + warnings: [], + info: ["Resolved 1 stale comment-cop thread(s).", "Posted 1 review comment(s)."], + }); + }); +}); From 98cc47e5d61048956972283aec56bb1b0d66d542 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 22:24:41 +0000 Subject: [PATCH 2/2] ci: drop comment-cop's GraphQL auto-resolve instead of guarding it Under the Actions GITHUB_TOKEN every resolveReviewThread mutation fails with "Resource not accessible by integration" (runs 31900717034 and 31900354924 today: 20/20 and 4/4 failed, each followed by a "Resolved N" log line), so keeping the thread lookup behind a try only preserved requests that fail and a misleading log line. With the lookup gone the step makes no GraphQL request at all; dedup comes from the REST review comment list. Resolving stale threads stays with #36959. The test shrinks to the dedup contract with GraphQL unavailable. --- .github/workflows/comment-cop.yml | 69 +------ .../internal/source-lints/comment-cop.test.ts | 173 ++++-------------- 2 files changed, 43 insertions(+), 199 deletions(-) diff --git a/.github/workflows/comment-cop.yml b/.github/workflows/comment-cop.yml index db3351ae5d3a..bd4f586a74d8 100644 --- a/.github/workflows/comment-cop.yml +++ b/.github/workflows/comment-cop.yml @@ -103,78 +103,19 @@ jobs: } const keyFor = g => `${g.path}:${crypto.createHash('sha256').update(g.text).digest('hex').slice(0, 12)}`; - const presentKeys = new Set(groups.map(keyFor)); - const MARKER = //; - // Dedup goes through REST on purpose: GITHUB_TOKEN's GraphQL quota is - // shared by every workflow run in the repo and is routinely exhausted, - // the REST quota is a separate pool, and a scan that cannot dedup - // would re-post every comment already on the PR. + // Keys already flagged on this PR, read from its review comments. REST + // only: GITHUB_TOKEN's GraphQL quota is shared by every workflow run in + // the repo and is routinely exhausted; the REST quota is separate, and + // posting below needs it anyway. const reviewComments = await github.paginate(github.rest.pulls.listReviewComments, { owner, repo, pull_number, per_page: 100, }); const seenKeys = new Set(); for (const c of reviewComments) { - const m = MARKER.exec(c.body || ''); + const m = //.exec(c.body || ''); if (m) seenKeys.add(m[1]); } - const staleKeys = new Set([...seenKeys].filter(k => !presentKeys.has(k))); - - // Ids of the unresolved threads for staleKeys. Thread ids and resolved - // state only exist in GraphQL, so this lookup is best effort: when it - // fails, nothing gets resolved and the scan below still runs. - async function staleThreadIds() { - const q = ` - query($owner: String!, $repo: String!, $pr: Int!, $after: String) { - repository(owner: $owner, name: $repo) { - pullRequest(number: $pr) { - reviewThreads(first: 100, after: $after) { - pageInfo { hasNextPage endCursor } - nodes { id isResolved comments(first: 1) { nodes { body } } } - } - } - } - }`; - const ids = []; - let after = null; - for (;;) { - const res = await github.graphql(q, { owner, repo, pr: pull_number, after }); - const page = res.repository.pullRequest.reviewThreads; - for (const t of page.nodes) { - if (t.isResolved) continue; - const m = MARKER.exec(t.comments.nodes[0]?.body || ''); - if (m && staleKeys.has(m[1])) ids.push(t.id); - } - if (!page.pageInfo.hasNextPage) break; - after = page.pageInfo.endCursor; - } - return ids; - } - - let toResolve = []; - if (staleKeys.size > 0) { - try { - toResolve = await staleThreadIds(); - } catch (e) { - core.warning(`Listing review threads failed, not resolving up to ${staleKeys.size} stale comment-cop thread(s): ${e.message}`); - } - } - - // Auto-resolve threads whose flagged block is gone from the current diff. - if (toResolve.length > 0) { - const mut = ` - mutation($id: ID!) { - resolveReviewThread(input: { threadId: $id }) { thread { id } } - }`; - for (const id of toResolve) { - try { - await github.graphql(mut, { id }); - } catch (e) { - core.warning(`resolveReviewThread failed for ${id}: ${e.message}`); - } - } - core.info(`Resolved ${toResolve.length} stale comment-cop thread(s).`); - } // Post new line comments for groups not already flagged. const fresh = groups.filter(g => !seenKeys.has(keyFor(g))); diff --git a/test/internal/source-lints/comment-cop.test.ts b/test/internal/source-lints/comment-cop.test.ts index d293793c847e..260153bd892b 100644 --- a/test/internal/source-lints/comment-cop.test.ts +++ b/test/internal/source-lints/comment-cop.test.ts @@ -1,14 +1,12 @@ /** * The script embedded in .github/workflows/comment-cop.yml, run the way - * actions/github-script runs it (as an async function body with `github`, - * `context`, `core` and `require` in scope) against a fake `github` that - * records writes instead of sending them. + * actions/github-script runs it (an async function body with `require`, + * `github`, `context` and `core` in scope) against a fake `github` that + * records the comments the script posts instead of sending them. * - * The scenarios pin the workflow's dependence on the two GitHub API quotas: * GITHUB_TOKEN's GraphQL quota is shared by every workflow run in the repo and - * is routinely exhausted, so scanning, dedup and posting have to work with - * GraphQL unavailable, and only the auto-resolve of stale threads (thread ids - * and resolved state exist only in GraphQL) may depend on it, as a warning. + * is routinely exhausted, so the fake fails every GraphQL request the way + * GitHub does then; the step has to dedup and post using REST alone. */ import { describe, expect, test } from "bun:test"; import { createHash } from "node:crypto"; @@ -34,9 +32,10 @@ const PATH = "src/runtime/example.rs"; /** Same key the workflow derives for a comment group: path plus a hash of the comment text. */ const keyFor = (text: string) => `${PATH}:${createHash("sha256").update(text).digest("hex").slice(0, 12)}`; -const markerComment = (key: string) => `\nplease delete this comment\n`; +/** What one of the workflow's own comments looks like when read back from the PR. */ +const botComment = (key: string) => `\nplease delete this comment\n`; -// Two three-line comment groups added at lines 2-4 and 6-8 of the new file. +// Two three-line comment groups, added at lines 2-4 and 6-8 of the new file. const firstGroup = ["// first group, line one", "// first group, line two", "// first group, line three"]; const secondGroup = ["// second group, line one", "// second group, line two", "// second group, line three"]; const patch = [ @@ -50,9 +49,8 @@ const patch = [ ].join("\n"); const FIRST = keyFor(firstGroup.join("\n")); const SECOND = keyFor(secondGroup.join("\n")); -/** Flagged on an earlier push, and the comment it flagged is no longer in the diff. */ -const STALE_UNRESOLVED = `${PATH}:000000000001`; -const STALE_RESOLVED = `${PATH}:000000000002`; +/** Flagged on an earlier push; the comment it flagged is no longer in the diff. */ +const STALE = `${PATH}:000000000001`; const expectedPost = (key: string, start: number, end: number) => ({ owner: "oven-sh", @@ -67,74 +65,34 @@ const expectedPost = (key: string, start: number, end: number) => ({ body: expect.stringContaining(``), }); -const rateLimitError = () => - Object.assign( - new Error( - "Request failed due to following response errors:\n - API rate limit already exceeded for site ID installation.", - ), - { name: "GraphqlResponseError", errors: [{ type: "RATE_LIMIT", code: "graphql_rate_limit" }] }, - ); - -type Thread = { id: string; isResolved: boolean; comments: { nodes: { body: string }[] } }; -const thread = (id: string, body: string | null, isResolved = false): Thread => ({ - id, - isResolved, - comments: { nodes: body === null ? [] : [{ body }] }, -}); - -interface Scenario { - /** Bodies of the review comments already on the PR, as `pulls.listReviewComments` returns them. */ - reviewComments: string[]; - /** Pages served for the reviewThreads query; omitted means every GraphQL request fails on the rate limit. */ - threadPages?: Thread[][]; -} - -async function scan({ reviewComments, threadPages }: Scenario) { - const posted: unknown[] = []; - const resolved: string[] = []; +/** Bodies of the review comments already on the PR, as `pulls.listReviewComments` returns them. */ +async function scan(reviewComments: string[]) { + const posted: { body: string }[] = []; const graphqlRequests: string[] = []; - const restPaginated: string[] = []; const warnings: string[] = []; - const info: string[] = []; - const endpoint = (name: string, rows: () => T[]) => - Object.assign(async () => ({ data: rows() }), { endpointName: name }); const github = { rest: { pulls: { - listFiles: endpoint("pulls.listFiles", () => [{ filename: PATH, status: "modified", patch }]), - listReviewComments: endpoint("pulls.listReviewComments", () => - reviewComments.map((body, i) => ({ id: i + 1, body, user: { login: "github-actions[bot]" } })), - ), - createReviewComment: async (params: unknown) => { + listFiles: async () => ({ data: [{ filename: PATH, status: "modified", patch }] }), + listReviewComments: async () => ({ + data: reviewComments.map((body, i) => ({ id: i + 1, body, user: { login: "github-actions[bot]" } })), + }), + createReviewComment: async (params: { body: string }) => { posted.push(params); return { data: {} }; }, }, }, - paginate: async (fn: { endpointName: string; (): Promise<{ data: unknown[] }> }) => { - restPaginated.push(fn.endpointName); - return (await fn()).data; - }, - graphql: async (query: string, variables: { id?: string; after?: string | null }) => { - const isMutation = /^\s*mutation\b/.test(query); - graphqlRequests.push(isMutation ? "mutation" : "query"); - if (threadPages === undefined) throw rateLimitError(); - if (isMutation) { - resolved.push(variables.id!); - return { resolveReviewThread: { thread: { id: variables.id } } }; - } - const pageIndex = variables.after == null ? 0 : Number(variables.after); - return { - repository: { - pullRequest: { - reviewThreads: { - pageInfo: { hasNextPage: pageIndex + 1 < threadPages.length, endCursor: String(pageIndex + 1) }, - nodes: threadPages[pageIndex], - }, - }, - }, - }; + paginate: async (endpoint: () => Promise<{ data: unknown[] }>) => (await endpoint()).data, + graphql: async (query: string) => { + graphqlRequests.push(query); + throw Object.assign( + new Error( + "Request failed due to following response errors:\n - API rate limit already exceeded for site ID installation.", + ), + { name: "GraphqlResponseError", errors: [{ type: "RATE_LIMIT", code: "graphql_rate_limit" }] }, + ); }, }; const context = { @@ -142,90 +100,35 @@ async function scan({ reviewComments, threadPages }: Scenario) { payload: { pull_request: { number: 4242, head: { sha: HEAD_SHA }, base: { ref: "main" } } }, }; const core = { - info: (message: string) => info.push(message), + info: () => {}, warning: (message: string) => warnings.push(message), }; await runScan(require, github, context, core); - return { posted, resolved, graphqlRequests, restPaginated, warnings, info }; + return { posted, graphqlRequests, warnings }; } describe("comment-cop with the GraphQL quota exhausted", () => { - test("still posts the new groups and dedups the already flagged one", async () => { - const result = await scan({ - reviewComments: [ - markerComment(FIRST), - "nit: rename this (a human review comment)", - markerComment(STALE_UNRESOLVED), - ], - }); - expect(result).toEqual({ - posted: [expectedPost(SECOND, 6, 8)], - resolved: [], - // The stale key makes it try the thread lookup once; the failure is a warning, not a failed check. - graphqlRequests: ["query"], - restPaginated: ["pulls.listFiles", "pulls.listReviewComments"], - warnings: [expect.stringContaining("API rate limit already exceeded")], - info: ["Posted 1 review comment(s)."], - }); - }); - - test("does not touch GraphQL when no flagged block has gone stale", async () => { - const result = await scan({ reviewComments: [markerComment(FIRST), "looks good"] }); - expect(result).toEqual({ + test("posts the groups that are not flagged yet and skips the one that is", async () => { + expect(await scan([botComment(FIRST), "nit: rename this (a human review comment)", botComment(STALE)])).toEqual({ posted: [expectedPost(SECOND, 6, 8)], - resolved: [], graphqlRequests: [], - restPaginated: ["pulls.listFiles", "pulls.listReviewComments"], warnings: [], - info: ["Posted 1 review comment(s)."], }); }); - test("a second run after posting finds its own comments through REST and posts nothing", async () => { - const firstRun = await scan({ reviewComments: [] }); - expect(firstRun.posted).toEqual([expectedPost(FIRST, 2, 4), expectedPost(SECOND, 6, 8)]); - - const secondRun = await scan({ reviewComments: firstRun.posted.map(params => (params as { body: string }).body) }); - expect(secondRun).toEqual({ - posted: [], - resolved: [], + test("a second run recognizes the comments the first run posted and posts nothing", async () => { + const firstRun = await scan([]); + expect(firstRun).toEqual({ + posted: [expectedPost(FIRST, 2, 4), expectedPost(SECOND, 6, 8)], graphqlRequests: [], - restPaginated: ["pulls.listFiles", "pulls.listReviewComments"], warnings: [], - info: ["No new comment groups to flag (2 present, all already flagged)."], }); - }); -}); -describe("comment-cop with GraphQL available", () => { - test("resolves only the unresolved threads whose flagged block left the diff", async () => { - const result = await scan({ - reviewComments: [ - markerComment(FIRST), - markerComment(STALE_UNRESOLVED), - markerComment(STALE_RESOLVED), - "nit: rename this (a human review comment)", - ], - threadPages: [ - [ - thread("THREAD_FIRST", markerComment(FIRST)), - thread("THREAD_STALE_UNRESOLVED", markerComment(STALE_UNRESOLVED)), - ], - [ - thread("THREAD_STALE_RESOLVED", markerComment(STALE_RESOLVED), true), - thread("THREAD_HUMAN", "nit: rename this (a human review comment)"), - thread("THREAD_EMPTY", null), - ], - ], - }); - expect(result).toEqual({ - posted: [expectedPost(SECOND, 6, 8)], - resolved: ["THREAD_STALE_UNRESOLVED"], - graphqlRequests: ["query", "query", "mutation"], - restPaginated: ["pulls.listFiles", "pulls.listReviewComments"], + expect(await scan(firstRun.posted.map(comment => comment.body))).toEqual({ + posted: [], + graphqlRequests: [], warnings: [], - info: ["Resolved 1 stale comment-cop thread(s).", "Posted 1 review comment(s)."], }); }); });