Skip to content

Commit b649971

Browse files
author
JSONbored
committed
test(engine): cover the package-local buildIssueQualityReport in the engine suite
`buildIssueQualityReport` is re-exported from the engine barrel (src/index.ts) but this package's own node:test suite never called it. `npm run engine:coverage` therefore saw the module load — top-level constants only — and reported it LF:313 LH:96 with BRF:0, i.e. no function-level data at all, while the root vitest suite covered the same file 87/87 lines and 143/143 branches. Two uploads disagreeing about the same file is invisible while patch coverage only reads changed lines, because nothing changes all 313 at once. Renormalizing the file's line endings did, and the merged report capped at ~57%: of the 217 lines the engine flag called uncovered, 84 were rescued by vitest and 133 were function-signature and closing-brace lines v8 never lists as statements, so nothing could rescue them. Cover it where this package is actually graded rather than leaning on the vitest duplicate — the outcome scripts/engine-coverage.ts (#9064) was written to encourage. The file now reports 313/313 lines and 163/163 branches under the engine model. 17 cases covering the lane arms (issue_discovery / direct_pr / split / unknown), every bounty lifecycle, duplicate and invalid labelling, self-solved loops, maintainer-authored vs maintainer-WIP, linkedPrs back-references for both open and merged PRs, collision risk tiers, sort order, the lifecycle and report caps, and the null-repo / unparseable-date degradation paths.
1 parent 267a99c commit b649971

1 file changed

Lines changed: 360 additions & 0 deletions

File tree

Lines changed: 360 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,360 @@
1+
import { test } from "node:test";
2+
import assert from "node:assert/strict";
3+
4+
import { buildIssueQualityReport } from "../dist/index.js";
5+
6+
// The package-local twin of the host engine's buildIssueQualityReport (#6057) was reachable from the
7+
// barrel but never invoked by this suite: `npm run engine:coverage` saw the module load (top-level
8+
// constants only) and reported it BRF:0 with 96/313 lines, while the root vitest suite exercised it
9+
// fully. Two uploads disagreeing about the same file is what surfaced as a phantom patch-coverage
10+
// failure the moment a whole-file diff touched it (#9798). Cover it here, in the suite that actually
11+
// grades this package, rather than leaning on the vitest duplicate.
12+
13+
type Json = Record<string, unknown>;
14+
15+
function now(): string {
16+
return new Date().toISOString();
17+
}
18+
19+
function daysAgoIso(days: number): string {
20+
return new Date(Date.now() - days * 86_400_000).toISOString();
21+
}
22+
23+
function registryConfig(overrides: Json = {}): Json {
24+
return { repo: "acme/widgets", emissionShare: 1, issueDiscoveryShare: 0.5, labelMultipliers: {}, maintainerCut: 0, raw: {}, ...overrides };
25+
}
26+
27+
function repo(fullName: string, overrides: Json = {}): Json {
28+
const [owner, name] = fullName.split("/");
29+
return {
30+
fullName,
31+
owner,
32+
name,
33+
installationId: undefined,
34+
isInstalled: true,
35+
isRegistered: true,
36+
isPrivate: false,
37+
htmlUrl: `https://github.com/${fullName}`,
38+
defaultBranch: "main",
39+
registryConfig: registryConfig({ repo: fullName }),
40+
...overrides,
41+
};
42+
}
43+
44+
/** issueDiscoveryShare 1 → `issue_discovery` lane; 0 → `direct_pr`; 0.4 with emission → `split`. */
45+
function laneRepo(fullName: string, issueDiscoveryShare: number): Json {
46+
return repo(fullName, { registryConfig: registryConfig({ repo: fullName, issueDiscoveryShare }) });
47+
}
48+
49+
function issue(repoFullName: string, number: number, title: string, overrides: Json = {}): Json {
50+
return {
51+
repoFullName,
52+
number,
53+
title,
54+
state: "open",
55+
authorLogin: "reporter",
56+
authorAssociation: "NONE",
57+
htmlUrl: `https://github.com/${repoFullName}/issues/${number}`,
58+
body: "x".repeat(220),
59+
createdAt: now(),
60+
updatedAt: now(),
61+
closedAt: null,
62+
labels: [],
63+
linkedPrs: [],
64+
...overrides,
65+
};
66+
}
67+
68+
function pr(repoFullName: string, number: number, overrides: Json = {}): Json {
69+
return {
70+
repoFullName,
71+
number,
72+
title: `PR ${number}`,
73+
state: "open",
74+
authorLogin: "contributor",
75+
authorAssociation: "NONE",
76+
headSha: "abc",
77+
headRef: "branch",
78+
baseRef: "main",
79+
htmlUrl: `https://github.com/${repoFullName}/pull/${number}`,
80+
mergedAt: null,
81+
isDraft: false,
82+
mergeableState: "clean",
83+
reviewDecision: null,
84+
body: "",
85+
createdAt: now(),
86+
updatedAt: now(),
87+
closedAt: null,
88+
labels: [],
89+
linkedIssues: [],
90+
...overrides,
91+
};
92+
}
93+
94+
function merged(repoFullName: string, number: number, overrides: Json = {}): Json {
95+
return {
96+
repoFullName,
97+
number,
98+
title: `Merged ${number}`,
99+
authorLogin: "contributor",
100+
htmlUrl: `https://github.com/${repoFullName}/pull/${number}`,
101+
labels: [],
102+
linkedIssues: [],
103+
...overrides,
104+
};
105+
}
106+
107+
function bounty(repoFullName: string, issueNumber: number, status: string, overrides: Json = {}): Json {
108+
return { id: `b-${issueNumber}-${status}`, repoFullName, issueNumber, status, discoveredAt: now(), updatedAt: now(), payload: {}, ...overrides };
109+
}
110+
111+
function emptyCollisions(fullName: string): Json {
112+
return { repoFullName: fullName, generatedAt: now(), clusters: [], summary: { clusterCount: 0, highRiskCount: 0, itemsReviewed: 0 } };
113+
}
114+
115+
/** The dist barrel is consumed untyped here (same as the sibling suites), so pin the shape once. */
116+
const build = buildIssueQualityReport as unknown as (...args: unknown[]) => {
117+
repoFullName: string;
118+
lane: { lane: string };
119+
summary: string;
120+
issues: { number: number; title: string; status: string; score: number; reasons: string[]; warnings: string[] }[];
121+
};
122+
123+
const first = (report: ReturnType<typeof build>) => report.issues[0]!;
124+
125+
test("barrel: the public entrypoint re-exports the package-local issue-quality API", () => {
126+
assert.equal(typeof buildIssueQualityReport, "function");
127+
});
128+
129+
test("buildIssueQualityReport: a detailed open issue with no linked work is ready", () => {
130+
const r = laneRepo("acme/ready", 1);
131+
const report = build(r, [issue("acme/ready", 1, "Actionable")], [], "acme/ready");
132+
assert.equal(report.issues.length, 1);
133+
assert.equal(first(report).status, "ready");
134+
assert.equal(report.repoFullName, "acme/ready");
135+
assert.ok(first(report).reasons.includes("Issue has enough body detail to evaluate."));
136+
assert.ok(first(report).reasons.includes("No active PR is linked in cached metadata."));
137+
assert.match(report.summary, /1 open issue\(s\) evaluated; 1 look ready/);
138+
});
139+
140+
test("buildIssueQualityReport: a linked open PR blocks the issue and a thin body needs proof", () => {
141+
const r = laneRepo("acme/linked", 1);
142+
const withPr = build(r, [issue("acme/linked", 2, "Claimed")], [pr("acme/linked", 9, { linkedIssues: [2] })], "acme/linked");
143+
assert.equal(first(withPr).status, "do_not_use");
144+
assert.ok(first(withPr).warnings.some((w) => /active PR/i.test(w)));
145+
146+
const thin = build(r, [issue("acme/linked", 3, "Thin", { body: "Short." })], [], "acme/linked");
147+
assert.equal(first(thin).status, "needs_proof");
148+
assert.ok(first(thin).warnings.some((w) => /body is thin/i.test(w)));
149+
});
150+
151+
test("buildIssueQualityReport: labels are echoed as a reason and a direct-PR lane warns", () => {
152+
const r = laneRepo("acme/direct", 0);
153+
const report = build(r, [issue("acme/direct", 4, "Direct", { labels: ["bug", "good first issue"] })], [], "acme/direct", [], emptyCollisions("acme/direct"), []);
154+
assert.equal(report.lane.lane, "direct_pr");
155+
assert.equal(first(report).status, "needs_proof");
156+
assert.ok(first(report).reasons.includes("Labels: bug, good first issue."));
157+
assert.ok(first(report).warnings.some((w) => /direct-PR first/i.test(w)));
158+
});
159+
160+
test("buildIssueQualityReport: every bounty lifecycle arm maps to its own status", () => {
161+
const r = laneRepo("acme/bounty", 1);
162+
const open = issue("acme/bounty", 5, "Bountied");
163+
164+
const active = build(r, [open], [], "acme/bounty", [bounty("acme/bounty", 5, "active")]);
165+
assert.ok(first(active).reasons.some((reason) => /Active bounty context/i.test(reason)));
166+
167+
for (const status of ["completed", "cancelled", "historical"]) {
168+
const report = build(r, [open], [], "acme/bounty", [bounty("acme/bounty", 5, status)]);
169+
assert.equal(first(report).status, "do_not_use", `${status} bounty should block`);
170+
}
171+
172+
const stale = build(r, [open], [], "acme/bounty", [bounty("acme/bounty", 5, "active", { updatedAt: daysAgoIso(60), discoveredAt: daysAgoIso(60) })]);
173+
assert.equal(first(stale).status, "needs_proof");
174+
assert.ok(first(stale).warnings.some((w) => /looks stale/i.test(w)));
175+
176+
const ambiguous = build(r, [open], [], "acme/bounty", [bounty("acme/bounty", 5, "weird-unknown-status")]);
177+
assert.equal(first(ambiguous).status, "needs_proof");
178+
assert.ok(first(ambiguous).warnings.some((w) => /ambiguous/i.test(w)));
179+
});
180+
181+
test("buildIssueQualityReport: duplicate and invalid labelling drive lifecycle do_not_use", () => {
182+
const r = laneRepo("acme/labels", 1);
183+
for (const label of ["duplicate", "wontfix", "invalid", "not planned", "won't fix"]) {
184+
const report = build(r, [issue("acme/labels", 10, "Labelled", { labels: [label] })], [], "acme/labels");
185+
assert.equal(first(report).status, "do_not_use", `${label} should block`);
186+
assert.ok(first(report).warnings.some((w) => /lifecycle is/i.test(w)));
187+
}
188+
});
189+
190+
test("buildIssueQualityReport: only open issues are reported, closed ones are filtered", () => {
191+
const r = laneRepo("acme/mixed", 1);
192+
const report = build(r, [issue("acme/mixed", 12, "Closed", { state: "closed" }), issue("acme/mixed", 13, "Open")], [], "acme/mixed");
193+
assert.deepEqual(
194+
report.issues.map((i) => i.number),
195+
[13],
196+
);
197+
});
198+
199+
test("buildIssueQualityReport: merged solvers mark valid_solved, and a self-solved loop stays solved", () => {
200+
const discovery = laneRepo("acme/solved", 1);
201+
const solved = build(discovery, [issue("acme/solved", 20, "Solved")], [], "acme/solved", [], undefined, [
202+
merged("acme/solved", 100, { linkedIssues: [20], authorLogin: "other" }),
203+
]);
204+
assert.equal(first(solved).status, "do_not_use");
205+
assert.ok(first(solved).warnings.some((w) => /merged PR/i.test(w)));
206+
207+
// Reporter authored the solving PR → selfSolvedLoop suppresses valid_solved.
208+
const selfSolved = build(
209+
discovery,
210+
[issue("acme/solved", 21, "Self", { authorLogin: "reporter" })],
211+
[pr("acme/solved", 101, { linkedIssues: [21], authorLogin: "reporter", mergedAt: now(), state: "merged" })],
212+
"acme/solved",
213+
);
214+
assert.equal(first(selfSolved).status, "do_not_use");
215+
216+
// split lane (issueDiscoveryShare 0.4 + emission) also earns valid_solved.
217+
const split = laneRepo("acme/split", 0.4);
218+
const splitSolved = build(split, [issue("acme/split", 90, "Split")], [], "acme/split", [], undefined, [
219+
merged("acme/split", 900, { linkedIssues: [90], authorLogin: "other" }),
220+
]);
221+
assert.equal(first(splitSolved).status, "do_not_use");
222+
});
223+
224+
test("buildIssueQualityReport: stale issues warn and age over 180 days costs score", () => {
225+
const r = laneRepo("acme/stale", 1);
226+
const stale = build(r, [issue("acme/stale", 30, "Old", { updatedAt: daysAgoIso(100), createdAt: daysAgoIso(100) })], [], "acme/stale");
227+
assert.equal(first(stale).status, "needs_proof");
228+
assert.ok(first(stale).warnings.some((w) => /stale/i.test(w)));
229+
230+
const ancient = build(r, [issue("acme/stale", 31, "Ancient", { updatedAt: daysAgoIso(200), createdAt: daysAgoIso(200) })], [], "acme/stale");
231+
const fresh = build(r, [issue("acme/stale", 32, "Fresh")], [], "acme/stale");
232+
assert.ok(first(ancient).score < first(fresh).score);
233+
});
234+
235+
test("buildIssueQualityReport: maintainer-authored and maintainer-WIP issues carry distinct warnings", () => {
236+
const r = laneRepo("acme/maint", 1);
237+
for (const association of ["OWNER", "MEMBER", "COLLABORATOR"]) {
238+
const authored = build(r, [issue("acme/maint", 40, "From staff", { authorAssociation: association })], [], "acme/maint");
239+
assert.ok(first(authored).warnings.some((w) => /Maintainer-authored; confirm/i.test(w)), association);
240+
}
241+
242+
const wip = build(r, [issue("acme/maint", 41, "WIP", { authorAssociation: "MEMBER", labels: ["Work In Progress "] })], [], "acme/maint");
243+
assert.equal(first(wip).status, "needs_proof");
244+
assert.ok(first(wip).warnings.some((w) => /in-progress\/internal/i.test(w)));
245+
// The WIP arm replaces the plain maintainer-authored warning rather than stacking with it.
246+
assert.equal(
247+
first(wip).warnings.some((w) => /Maintainer-authored; confirm/i.test(w)),
248+
false,
249+
);
250+
251+
// A non-maintainer carrying a WIP label is not treated as maintainer WIP.
252+
const outsider = build(r, [issue("acme/maint", 42, "Outsider WIP", { authorAssociation: "NONE", labels: ["wip"] })], [], "acme/maint");
253+
assert.equal(
254+
first(outsider).warnings.some((w) => /in-progress\/internal/i.test(w)),
255+
false,
256+
);
257+
});
258+
259+
test("buildIssueQualityReport: issue.linkedPrs back-references resolve, unknown ones warn from cache", () => {
260+
const r = laneRepo("acme/backref", 1);
261+
// The PR does not declare the issue; the issue declares the PR → resolveLinkedPullRequests adds it.
262+
const backref = build(r, [issue("acme/backref", 50, "Backref", { linkedPrs: [77] })], [pr("acme/backref", 77, { linkedIssues: [] })], "acme/backref");
263+
assert.equal(first(backref).status, "do_not_use");
264+
assert.ok(first(backref).warnings.some((w) => /active PR/i.test(w)));
265+
266+
// linkedPrs points at a PR absent from the fetched list → cached-metadata warning only.
267+
const cachedOnly = build(r, [issue("acme/backref", 51, "Cached", { linkedPrs: [999] })], [], "acme/backref");
268+
assert.ok(first(cachedOnly).warnings.some((w) => /already references PR\(s\): #999/.test(w)));
269+
270+
// Merged-PR back-reference travels the same path through the recent-merged index.
271+
const mergedBackref = build(r, [issue("acme/backref", 52, "Merged backref", { linkedPrs: [78] })], [], "acme/backref", [], undefined, [
272+
merged("acme/backref", 78, { linkedIssues: [] }),
273+
]);
274+
assert.equal(first(mergedBackref).status, "do_not_use");
275+
});
276+
277+
test("buildIssueQualityReport: high-risk collision clusters block, lower risk only warns", () => {
278+
const r = laneRepo("acme/collide", 1);
279+
const cluster = (risk: string): Json => ({
280+
repoFullName: "acme/collide",
281+
generatedAt: now(),
282+
summary: { clusterCount: 1, highRiskCount: risk === "high" ? 1 : 0, itemsReviewed: 2 },
283+
clusters: [
284+
{
285+
id: "c1",
286+
risk,
287+
reason: "overlap",
288+
items: [
289+
{ type: "issue", number: 60, title: "A" },
290+
{ type: "pull_request", number: 1, title: "B" },
291+
],
292+
},
293+
],
294+
});
295+
296+
const high = build(r, [issue("acme/collide", 60, "A")], [], "acme/collide", [], cluster("high"));
297+
assert.equal(first(high).status, "do_not_use");
298+
299+
const medium = build(r, [issue("acme/collide", 60, "A")], [], "acme/collide", [], cluster("medium"));
300+
assert.ok(first(medium).warnings.some((w) => /duplicate or overlapping/i.test(w)));
301+
assert.notEqual(first(medium).status, "do_not_use");
302+
});
303+
304+
test("buildIssueQualityReport: results sort by score desc then issue number asc", () => {
305+
const r = laneRepo("acme/sort", 1);
306+
const report = build(
307+
r,
308+
[issue("acme/sort", 2, "Thin", { body: "Short." }), issue("acme/sort", 1, "Ready"), issue("acme/sort", 3, "Ready too")],
309+
[],
310+
"acme/sort",
311+
[],
312+
emptyCollisions("acme/sort"),
313+
);
314+
assert.deepEqual(
315+
report.issues.map((i) => i.number),
316+
[1, 3, 2],
317+
);
318+
});
319+
320+
test("buildIssueQualityReport: a null repo, absent dates and a blank bounty status degrade safely", () => {
321+
const report = build(null, [issue("acme/null", 70, "No dates", { updatedAt: null, createdAt: null, body: null })], [], "acme/null", [
322+
bounty("acme/null", 70, " "),
323+
]);
324+
assert.equal(report.lane.lane, "unknown");
325+
assert.equal(first(report).status, "needs_proof");
326+
327+
// Unparseable timestamps normalize to age 0 rather than throwing or reading as ancient.
328+
const r = laneRepo("acme/null", 1);
329+
const badDate = build(r, [issue("acme/null", 71, "Bad date", { updatedAt: "not-a-date", createdAt: "also-bad" })], [], "acme/null");
330+
assert.equal(first(badDate).status, "ready");
331+
});
332+
333+
test("buildIssueQualityReport: repeated linked-issue references collapse into one PR bucket", () => {
334+
const r = laneRepo("acme/multi", 1);
335+
const multi = build(
336+
r,
337+
[issue("acme/multi", 91, "Crowded")],
338+
[pr("acme/multi", 1, { linkedIssues: [91] }), pr("acme/multi", 2, { linkedIssues: [91, 91] })],
339+
"acme/multi",
340+
);
341+
assert.equal(first(multi).status, "do_not_use");
342+
assert.ok(first(multi).warnings.some((w) => /^2 active PR/.test(w)));
343+
});
344+
345+
test("buildIssueQualityReport: an open issue past the lifecycle cap is still classified (#6141)", () => {
346+
const r = laneRepo("acme/cap", 1);
347+
const filler = Array.from({ length: 300 }, (_, i) => issue("acme/cap", i + 1, `Closed ${i + 1}`, { state: "closed" }));
348+
const beyondCap = issue("acme/cap", 301, "Beyond cap duplicate", { labels: ["duplicate"] });
349+
const report = build(r, [...filler, beyondCap], [], "acme/cap");
350+
assert.equal(report.issues.length, 1);
351+
assert.equal(first(report).number, 301);
352+
assert.equal(first(report).status, "do_not_use");
353+
});
354+
355+
test("buildIssueQualityReport: the report caps at 100 issues", () => {
356+
const r = laneRepo("acme/cap100", 1);
357+
const many = Array.from({ length: 140 }, (_, i) => issue("acme/cap100", i + 1, `Open ${i + 1}`));
358+
const report = build(r, many, [], "acme/cap100");
359+
assert.equal(report.issues.length, 100);
360+
});

0 commit comments

Comments
 (0)