From 0a45ad1a62b963922b6127012410fdb80a02ac99 Mon Sep 17 00:00:00 2001 From: luciferlive112116 <291889058+luciferlive112116@users.noreply.github.com> Date: Mon, 6 Jul 2026 11:42:16 +0800 Subject: [PATCH] test(review): auto_review round-trip and precedence matrix Consolidated pure tests for review.auto_review parse/serialize round-trip, malformed-config warnings, evaluateAutoReviewSkipReason precedence, and decideReviewEligibility alignment. Co-authored-by: Cursor --- test/unit/auto-review-config-matrix.test.ts | 176 ++++++++++++++++++++ 1 file changed, 176 insertions(+) create mode 100644 test/unit/auto-review-config-matrix.test.ts diff --git a/test/unit/auto-review-config-matrix.test.ts b/test/unit/auto-review-config-matrix.test.ts new file mode 100644 index 0000000000..41865b0cdd --- /dev/null +++ b/test/unit/auto-review-config-matrix.test.ts @@ -0,0 +1,176 @@ +// Consolidated `review.auto_review` round-trip and predicate-precedence matrix (#2071). +import { describe, expect, it } from "vitest"; +import { decideReviewEligibility } from "../../src/review/review-eligibility"; +import { + EMPTY_AUTO_REVIEW_CONFIG, + evaluateAutoReviewSkipReason, + parseFocusManifest, + reviewConfigToJson, + type AutoReviewConfig, + type AutoReviewEligibilityInput, +} from "../../src/signals/focus-manifest"; + +describe("review.auto_review parse ↔ reviewConfigToJson round-trip (#2071)", () => { + it("omits auto_review when every field is the byte-identical default", () => { + expect(reviewConfigToJson(parseFocusManifest({}).review)).toBeNull(); + expect(reviewConfigToJson(parseFocusManifest({ review: {} }).review)).toBeNull(); + expect(reviewConfigToJson(parseFocusManifest({ review: { auto_review: {} } }).review)).toBeNull(); + }); + + const roundTripCases: Array<{ name: string; autoReview: Record }> = [ + { name: "skip_drafts: true", autoReview: { skip_drafts: true } }, + { name: "skip_drafts: false", autoReview: { skip_drafts: false } }, + { name: "ignore_authors", autoReview: { ignore_authors: ["*[bot]", "dependabot[bot]"] } }, + { name: "ignore_title_keywords", autoReview: { ignore_title_keywords: ["WIP", "draft"] } }, + { name: "base_branches", autoReview: { base_branches: ["main", "release/**"] } }, + { name: "auto_pause_after_reviewed_commits", autoReview: { auto_pause_after_reviewed_commits: 3 } }, + { + name: "all knobs together", + autoReview: { + skip_drafts: true, + ignore_authors: ["*[bot]"], + ignore_title_keywords: ["WIP"], + base_branches: ["main"], + auto_pause_after_reviewed_commits: 2, + }, + }, + ]; + + for (const testCase of roundTripCases) { + it(`round-trips ${testCase.name}`, () => { + const parsed = parseFocusManifest({ review: { auto_review: testCase.autoReview } }); + const json = reviewConfigToJson(parsed.review); + expect(json).not.toBeNull(); + const reparsed = parseFocusManifest({ review: json as Record }); + expect(reparsed.review.autoReview).toEqual(parsed.review.autoReview); + expect(reviewConfigToJson(reparsed.review)).toEqual(json); + }); + } +}); + +describe("review.auto_review malformed config (#2071)", () => { + it("warns and resets on a non-mapping auto_review value", () => { + const bad = parseFocusManifest({ review: { auto_review: "nope" } }); + expect(bad.review.autoReview).toEqual({ ...EMPTY_AUTO_REVIEW_CONFIG }); + expect(bad.warnings.some((w) => /auto_review.*must be a mapping/.test(w))).toBe(true); + expect(decideReviewEligibility({ authorLogin: "renovate", ignoreAuthors: bad.review.autoReview.ignoreAuthors })).toEqual({ + eligible: true, + skipReason: null, + matchedPattern: null, + }); + }); + + it("warns on non-list ignore_authors and keeps defaults byte-identical", () => { + const bad = parseFocusManifest({ review: { auto_review: { ignore_authors: "dependabot" } } }); + expect(bad.review.autoReview.ignoreAuthors).toEqual([]); + expect(bad.warnings.some((w) => /ignore_authors.*must be a list/.test(w))).toBe(true); + }); + + it("warns on negative auto_pause_after_reviewed_commits and drops the knob", () => { + const bad = parseFocusManifest({ review: { auto_review: { auto_pause_after_reviewed_commits: -1 } } }); + expect(bad.review.autoReview.autoPauseAfterReviewedCommits).toBeNull(); + expect(bad.warnings.some((w) => /auto_pause_after_reviewed_commits.*non-negative integer/.test(w))).toBe(true); + }); +}); + +describe("evaluateAutoReviewSkipReason predicate precedence (#2071)", () => { + const allTriggers: AutoReviewEligibilityInput = { + isDraft: true, + author: "dependabot[bot]", + title: "WIP: bump deps", + baseRef: "develop", + reviewedCommitCount: 5, + }; + + const allConfigured: AutoReviewConfig = { + skipDrafts: true, + ignoreAuthors: ["*[bot]"], + ignoreTitleKeywords: ["wip"], + baseBranches: ["main"], + autoPauseAfterReviewedCommits: 1, + }; + + const precedenceCases: Array<{ + name: string; + config: AutoReviewConfig; + input: AutoReviewEligibilityInput; + reason: string | null; + }> = [ + { + name: "draft wins when every predicate would match", + config: allConfigured, + input: allTriggers, + reason: "review skipped (draft)", + }, + { + name: "ignored author when draft filter is off", + config: { ...allConfigured, skipDrafts: false }, + input: { ...allTriggers, isDraft: false }, + reason: "review skipped (ignored author)", + }, + { + name: "WIP title when draft and author filters are off", + config: { ...allConfigured, skipDrafts: false, ignoreAuthors: [] }, + input: { ...allTriggers, isDraft: false, author: "alice" }, + reason: "review skipped (WIP title)", + }, + { + name: "base branch when earlier filters are off", + config: { ...allConfigured, skipDrafts: false, ignoreAuthors: [], ignoreTitleKeywords: [] }, + input: { ...allTriggers, isDraft: false, author: "alice", title: "chore: bump" }, + reason: "review skipped (base branch out of scope)", + }, + { + name: "commit threshold when earlier filters are off", + config: { + ...EMPTY_AUTO_REVIEW_CONFIG, + autoPauseAfterReviewedCommits: 2, + }, + input: { ...allTriggers, isDraft: false, author: "alice", title: "feat", baseRef: "main", reviewedCommitCount: 2 }, + reason: "review paused (commit threshold)", + }, + { + name: "eligible when every configured filter is off or non-matching", + config: allConfigured, + input: { + isDraft: false, + author: "alice", + title: "feat: add widget", + baseRef: "main", + reviewedCommitCount: 0, + }, + reason: null, + }, + ]; + + for (const testCase of precedenceCases) { + it(testCase.name, () => { + expect(evaluateAutoReviewSkipReason(testCase.config, testCase.input)).toBe(testCase.reason); + }); + } +}); + +describe("decideReviewEligibility aligns with auto_review ignore_authors (#2071)", () => { + const cases: Array<{ authorLogin: string; ignoreAuthors: string[]; eligible: boolean }> = [ + { authorLogin: "dependabot[bot]", ignoreAuthors: ["*[bot]"], eligible: false }, + { authorLogin: "alice", ignoreAuthors: ["*[bot]"], eligible: true }, + { authorLogin: "renovate", ignoreAuthors: ["renovate", "dependabot"], eligible: false }, + { authorLogin: "", ignoreAuthors: ["*"], eligible: true }, + ]; + + for (const testCase of cases) { + it(`${testCase.authorLogin || "(blank)"} with [${testCase.ignoreAuthors.join(", ")}]`, () => { + const decision = decideReviewEligibility({ + authorLogin: testCase.authorLogin, + ignoreAuthors: testCase.ignoreAuthors, + }); + expect(decision.eligible).toBe(testCase.eligible); + if (!testCase.eligible) { + expect(decision.skipReason).toBe("ignored_author"); + } else { + expect(decision.skipReason).toBeNull(); + expect(decision.matchedPattern).toBeNull(); + } + }); + } +});