Blame · Line-by-line history

`issue-counts.test.ts`

Each line is annotated with the commit that last touched it. Click any SHA to jump to that commit and see the surrounding change.

issue-counts.test.tsBlame141 lines · 1 contributor

Normal view Raw

dc32319ccantynz-alt	1	/**
	2	* Regression guard: the dashboard's "Open Issues" figure must reflect issues
	3	* that are actually open.
	4	*
	5	* repositories.issueCount is incremented in eight places — the issues route,
	6	* the MCP write surface, voice-to-pr, the dependency scanner (twice),
	7	* ai-auto-issues, ai-incident and incident-hooks — and decremented in NONE.
	8	* Closing an issue updates issues.state and never touches it. So the column
	9	* is an issues-ever-created total that can only go up, and the dashboard
	10	* rendered it under the label "Open Issues": close every issue in every
	11	* repository and the number does not move. The REST repo listing returns the
	12	* same column, and the API docs present it beside starCount and forkCount
	13	* where a consumer reads it as GitHub's open_issues_count does.
	14	*
	15	* Fixed by computing at read time instead of maintaining the counter. The
	16	* counter has eight independent write sites and has already drifted; making
	17	* it correct would mean adding decrements to close, reopen and delete plus a
	18	* backfill, and leaving eight future chances to drift again. One GROUP BY
	19	* over the issues_repo_state index cannot skew.
	20	*
	21	* The stored column stays and is still written — it is part of the
	22	* documented REST response shape, so removing it would be a contract change
	23	* rather than a bug fix.
	24	*/
	25
	26	import { describe, it, expect } from "bun:test";
	27	import { readFileSync } from "node:fs";
	28	import { join } from "node:path";
	29	import { countRowsToMap } from "../lib/issue-counts";
	30
	31	const REPO_A = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa";
	32	const REPO_B = "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb";
	33
	34	describe("countRowsToMap", () => {
	35	it("coerces bigint counts that arrive as strings", () => {
	36	// Postgres count() is bigint and the driver returns it as a STRING. This
	37	// is the whole hazard: drizzle's sql<number> is a compile-time cast only.
	38	const map = countRowsToMap([
	39	{ repositoryId: REPO_A, n: "3" },
	40	{ repositoryId: REPO_B, n: "5" },
	41	]);
	42	expect(map.get(REPO_A)).toBe(3);
	43	expect(map.get(REPO_B)).toBe(5);
	44	});
	45
	46	it("produces totals that ADD rather than concatenate", () => {
	47	// The failure mode if the coercion is dropped: "3" + "5" === "35".
	48	const map = countRowsToMap([
	49	{ repositoryId: REPO_A, n: "3" },
	50	{ repositoryId: REPO_B, n: "5" },
	51	]);
	52	let total = 0;
	53	for (const n of map.values()) total += n;
	54	expect(total).toBe(8);
	55	expect(total).not.toBe("35");
	56	});
	57
	58	it("accepts counts that already arrive as numbers", () => {
	59	const map = countRowsToMap([{ repositoryId: REPO_A, n: 7 }]);
	60	expect(map.get(REPO_A)).toBe(7);
	61	});
	62
	63	it("never yields NaN, which would poison a sum", () => {
	64	const map = countRowsToMap([
	65	{ repositoryId: REPO_A, n: null },
	66	{ repositoryId: REPO_B, n: "not-a-number" },
	67	]);
	68	expect(map.get(REPO_A)).toBe(0);
	69	expect(map.get(REPO_B)).toBe(0);
	70	let total = 0;
	71	for (const n of map.values()) total += n;
	72	expect(Number.isNaN(total)).toBe(false);
	73	expect(total).toBe(0);
	74	});
	75
	76	it("skips rows with no repository", () => {
	77	const map = countRowsToMap([{ repositoryId: null, n: "4" }]);
	78	expect(map.size).toBe(0);
	79	});
	80
	81	it("returns an empty map for no rows", () => {
	82	expect(countRowsToMap([]).size).toBe(0);
	83	});
	84	});
	85
	86	// --- the readers must use the computed value -------------------------------
	87	//
	88	// Assert the CALL form, not the bare identifier: `void totalOpenIssues;`
	89	// mentions it while computing nothing. Strip ONLY line comments.
	90
	91	function source(...rel: string[]): string {
	92	const raw = readFileSync(join(import.meta.dir, "..", ...rel), "utf8");
	93	const stripped = raw
	94	.split("\n")
	95	.filter((l) => !l.trim().startsWith("//") && !l.trim().startsWith("*"))
	96	.join("\n");
	97	expect(stripped.length).toBeGreaterThan(0);
	98	return stripped;
	99	}
	100
	101	describe("the dashboard shows computed open issues", () => {
	102	const src = source("routes", "dashboard.tsx");
	103
	104	it("calls totalOpenIssues", () => {
	105	expect(src).toContain("totalOpenIssues(");
	106	});
	107
	108	it("no longer sums the stored counter into the stat card", () => {
	109	// The exact expression that produced the wrong number.
	110	expect(src).not.toContain("value={String(repos.reduce((s, r) => s + r.issueCount, 0))}");
	111	});
	112
	113	it("still labels the card Open Issues, so the fix targets the right stat", () => {
	114	// Guards against the assertion above passing because the card was
	115	// renamed or deleted rather than corrected.
	116	expect(src).toContain('label="Open Issues"');
	117	expect(src).toContain("value={String(openIssuesTotal)}");
	118	});
	119	});
	120
	121	describe("the REST repo listing serves computed open issues", () => {
	122	const src = source("routes", "api.ts");
	123
	124	it("calls openIssueCountsByRepo", () => {
	125	expect(src).toContain("openIssueCountsByRepo(");
	126	});
	127
	128	it("overwrites issueCount on the serialized rows", () => {
	129	expect(src).toContain("row.issueCount = counts.get(");
	130	});
	131
	132	it("keeps serving the field when the count query fails", () => {
	133	// Dropping the key would change the documented response shape; a stale
	134	// number is better than a missing one.
	135	const at = src.indexOf("openIssueCountsByRepo(");
	136	expect(at).toBeGreaterThan(-1);
	137	const after = src.slice(at, at + 600);
	138	expect(after.length).toBeGreaterThan(0);
	139	expect(after).toContain("catch");
	140	});
	141	});