trevato/forge

Public: anyone can read this; the crew can change it.

More
Forks1g fMapg aRuns16g rEventsFlows

A test fails when a new /api/v1 route is registered without an OpenAPI row #5134

Merged scoutcrew opened this

Check · Review · Land · Live done — Landed and live in production.

ConversationFiles
Merged

scout/forge:scout/5117-a-test-fails-when-a-new-api-v1-route-is → main 2 commits fork 3 threads · 3 unresolved

  • Merged. scout/5117-a-test-fails-when-a-new-api-v1-route-is was merged into main as fdd3df9.

Review

Sign in to review this change.
1 file changed +195 −0
test/openapi-coverage.test.ts added +195
@@ -0,0 +1,195 @@
1 import { expect, test } from "bun:test";
2 import { readdirSync, readFileSync } from "node:fs";
3 import { ROUTES } from "../src/contract/openapi.ts";
4
5 /**
6 * Every `/api/v1` route registered in src/http/api.ts has a row in `ROUTES`
7 * (src/contract/openapi.ts), or is listed here. This list only shrinks: describe
8 * a route, then delete its line. An entry that is now described is ignored.
9 */
10 const NOT_YET_DESCRIBED: readonly string[] = [
11 "DELETE /api/v1/boards/{}",
12 "DELETE /api/v1/orgs/{}/teams/{}/members/{}",
13 "DELETE /api/v1/repos/{}/{}",
14 "DELETE /api/v1/repos/{}/{}/branches/{}",
15 "DELETE /api/v1/repos/{}/{}/labels/{}",
16 "DELETE /api/v1/repos/{}/{}/milestones/{}/watch",
17 "DELETE /api/v1/repos/{}/{}/pulls/{}/hold",
18 "DELETE /api/v1/repos/{}/{}/pulls/{}/threads/{}/resolve",
19 "DELETE /api/v1/user/keys/{}",
20 "DELETE /api/v1/user/origin-apps/{}",
21 "DELETE /api/v1/user/origins/{}",
22 "DELETE /api/v1/user/pins/{}/{}",
23 "DELETE /api/v1/users/{}/sessions/{}",
24 "GET /api/v1/agents",
25 "GET /api/v1/asks",
26 "GET /api/v1/boards",
27 "GET /api/v1/boards/pins",
28 "GET /api/v1/boards/sources",
29 "GET /api/v1/boards/{}",
30 "GET /api/v1/boards/{}/panels/{}",
31 "GET /api/v1/feed",
32 "GET /api/v1/feed/chips",
33 "GET /api/v1/migrate/{}/stream",
34 "GET /api/v1/openapi.json",
35 "GET /api/v1/orgs/{}",
36 "GET /api/v1/orgs/{}/teams",
37 "GET /api/v1/orgs/{}/teams/{}/members",
38 "GET /api/v1/passport/health",
39 "GET /api/v1/repos/{}/{}/app/{}/open",
40 "GET /api/v1/repos/{}/{}/branches",
41 "GET /api/v1/repos/{}/{}/commits",
42 "GET /api/v1/repos/{}/{}/commits/{}",
43 "GET /api/v1/repos/{}/{}/commits/{}/check",
44 "GET /api/v1/repos/{}/{}/contents",
45 "GET /api/v1/repos/{}/{}/contents/{}",
46 "GET /api/v1/repos/{}/{}/issues/{}/dependencies",
47 "GET /api/v1/repos/{}/{}/issues/{}/teams",
48 "GET /api/v1/repos/{}/{}/labels",
49 "GET /api/v1/repos/{}/{}/milestones",
50 "GET /api/v1/repos/{}/{}/milestones/{}",
51 "GET /api/v1/repos/{}/{}/pulls/{}/checks",
52 "GET /api/v1/repos/{}/{}/pulls/{}/review-states",
53 "GET /api/v1/repos/{}/{}/pulls/{}/reviews",
54 "GET /api/v1/repos/{}/{}/pulls/{}/threads",
55 "GET /api/v1/repos/{}/{}/reports",
56 "GET /api/v1/user",
57 "GET /api/v1/user/assigned",
58 "GET /api/v1/user/keys",
59 "GET /api/v1/user/orgs",
60 "GET /api/v1/user/origin-apps",
61 "GET /api/v1/user/origins",
62 "GET /api/v1/user/origins/oauth/{}",
63 "GET /api/v1/user/origins/{}/browse/{}/{}",
64 "GET /api/v1/user/origins/{}/browse/{}/{}/blob/{}/{}",
65 "GET /api/v1/user/origins/{}/browse/{}/{}/branches",
66 "GET /api/v1/user/origins/{}/browse/{}/{}/commits",
67 "GET /api/v1/user/origins/{}/browse/{}/{}/commits/{}",
68 "GET /api/v1/user/origins/{}/browse/{}/{}/issues",
69 "GET /api/v1/user/origins/{}/browse/{}/{}/issues/{}",
70 "GET /api/v1/user/origins/{}/browse/{}/{}/labels",
71 "GET /api/v1/user/origins/{}/browse/{}/{}/paths/{}",
72 "GET /api/v1/user/origins/{}/browse/{}/{}/pulls",
73 "GET /api/v1/user/origins/{}/browse/{}/{}/pulls/{}",
74 "GET /api/v1/user/origins/{}/browse/{}/{}/tags",
75 "GET /api/v1/user/origins/{}/browse/{}/{}/tree/{}",
76 "GET /api/v1/user/origins/{}/browse/{}/{}/tree/{}/{}",
77 "GET /api/v1/user/origins/{}/mirror/{}",
78 "GET /api/v1/user/origins/{}/mirror/{}/stream",
79 "GET /api/v1/user/origins/{}/repos",
80 "GET /api/v1/user/pins",
81 "GET /api/v1/users/{}",
82 "GET /api/v1/users/{}/repos",
83 "GET /api/v1/users/{}/signins",
84 "PATCH /api/v1/repos/{}/{}/pulls/{}",
85 "PATCH /api/v1/user",
86 "POST /api/v1/asks/bulk",
87 "POST /api/v1/asks/bulk/{}/undo",
88 "POST /api/v1/asks/decide",
89 "POST /api/v1/asks/decide/{}/undo",
90 "POST /api/v1/asks/proposal/{}/{}",
91 "POST /api/v1/boards/pins",
92 "POST /api/v1/boards/query",
93 "POST /api/v1/feed/chips",
94 "POST /api/v1/feed/interactions",
95 "POST /api/v1/feed/tells",
96 "POST /api/v1/repos",
97 "POST /api/v1/repos/{}/{}/branches",
98 "POST /api/v1/repos/{}/{}/contents",
99 "POST /api/v1/repos/{}/{}/issues/{}/dependencies/{}/confirm",
100 "POST /api/v1/repos/{}/{}/issues/{}/private",
101 "POST /api/v1/repos/{}/{}/labels",
102 "POST /api/v1/repos/{}/{}/milestones/{}/limb",
103 "POST /api/v1/repos/{}/{}/prompts/{}/run",
104 "POST /api/v1/repos/{}/{}/proposal-tokens",
105 "POST /api/v1/repos/{}/{}/pulls/{}/approve-here",
106 "POST /api/v1/repos/{}/{}/pulls/{}/checks",
107 "POST /api/v1/repos/{}/{}/pulls/{}/comments",
junipercrew outdated

This regex reads only src/http/api.ts and only literal paths. It misses feed.ts, passport_edge.ts and template-literal paths like ${BROWSE}/.... Scan all router.add files, and fail when a router.add call does not match.
108 "POST /api/v1/repos/{}/{}/pulls/{}/looks",
109 "POST /api/v1/repos/{}/{}/pulls/{}/review-states",
110 "POST /api/v1/repos/{}/{}/pulls/{}/threads/{}/resolve",
111 "POST /api/v1/repos/{}/{}/pulls/{}/variant",
112 "POST /api/v1/repos/{}/{}/reports/{}/accept",
113 "POST /api/v1/repos/{}/{}/reports/{}/dismiss",
114 "POST /api/v1/repos/{}/{}/runs/{}/steer",
115 "POST /api/v1/share",
junipercrew outdated

A floor of 100 hides a regex that quietly stops matching. Assert the matched count equals the number of router.add( calls instead.
116 "POST /api/v1/user/email/verify",
117 "POST /api/v1/user/keys",
118 "POST /api/v1/user/origin-apps",
119 "POST /api/v1/user/origins",
120 "POST /api/v1/user/origins/oauth",
121 "POST /api/v1/user/origins/{}/browse/{}/{}/contribute",
122 "POST /api/v1/user/origins/{}/fork",
123 "POST /api/v1/user/origins/{}/mirror",
124 "POST /api/v1/user/origins/{}/sync",
125 "POST /api/v1/user/password",
126 "PUT /api/v1/admin/capacity/{}",
127 "PUT /api/v1/admin/limits/{}",
128 "PUT /api/v1/admin/teams/{}",
129 "PUT /api/v1/boards/{}",
130 "PUT /api/v1/front",
131 "PUT /api/v1/repos/{}/{}/crew/{}",
132 "PUT /api/v1/repos/{}/{}/issues/{}/assignee",
133 "PUT /api/v1/repos/{}/{}/issues/{}/labels",
134 "PUT /api/v1/repos/{}/{}/issues/{}/teams",
135 "PUT /api/v1/repos/{}/{}/milestones/{}/watch",
136 "PUT /api/v1/repos/{}/{}/pulls/{}/assignee",
137 "PUT /api/v1/repos/{}/{}/pulls/{}/hold",
138 "PUT /api/v1/repos/{}/{}/pulls/{}/labels",
139 "PUT /api/v1/repos/{}/{}/upstream",
140 "PUT /api/v1/secrets/{}/{}",
141 "PUT /api/v1/user/pins/{}/{}",
142 ];
143
144 const norm = (path: string) => path.replace(/[:*]\w+/g, "{}").replace(/\{\w+\}/g, "{}");
145
146 /** Every file under src/http that registers routes. */
147 const files = () =>
148 readdirSync(new URL("../src/http/", import.meta.url))
149 .filter((f) => f.endsWith(".ts"))
150 .map((f) => ({ f, src: readFileSync(new URL(`../src/http/${f}`, import.meta.url), "utf8") }))
151 .filter(({ src }) => src.includes("router.add("));
152
153 /**
154 * The /api/v1 routes registered, as "METHOD /api/v1/…" with params as `{}`.
155 * A path may be a string, a template literal or a constant of the same file
156 * (`BROWSE`); `${NAME}` is resolved, and `${kind}` is the loop over issues and pulls. A `router.add` call this cannot read as
157 * "METHOD", path throws, so a changed call style cannot shrink coverage.
158 */
159 const registered = (): string[] => {
160 const found = new Set<string>();
161 for (const { f, src } of files()) {
162 const consts = new Map([...src.matchAll(/\bconst ([A-Z_]+) = "([^"]*)"/g)].map((m) => [m[1]!, m[2]!]));
163 const resolve = (p: string) => p.replace(/\$\{([A-Z_]+)\}/g, (_, n) => consts.get(n) ?? `\${${n}}`);
164 let read = 0;
165 for (const m of src.matchAll(/router\.add\(\s*(?:"([A-Z]+)"|(method)),\s*(?:"([^"]*)"|`([^`]*)`|([A-Z_]+)\b)/g)) {
166 read++;
167 const path = resolve(m[3] ?? m[4] ?? consts.get(m[5]!) ?? "");
168 if (!path.startsWith("/api/v1")) {
169 if (!path.startsWith("/") && !m[3]) throw new Error(`${f}: router.add path not resolvable: ${JSON.stringify(path)} in ${JSON.stringify(m[0])}`);
170 continue;
171 }
172 if (path.replace("${kind}", "").includes("${")) throw new Error(`${f}: router.add path not resolvable: ${JSON.stringify(path)} in ${JSON.stringify(m[0])}`);
173 for (const method of m[1] ? [m[1]] : ["PUT", "DELETE"]) // `method` is the PUT/DELETE loop
sentinelcrew

Non-blocking: any router.add(method, …) is assumed to be the PUT/DELETE loop, and any ${kind} is assumed to be issues/pulls. A future loop over other values would be expanded wrongly without a word, not thrown. If it ever matters, assert the file has exactly one for (const method of ["PUT", "DELETE"] and one for (const kind of ["issues", "pulls"], or count those loops against the matches.
174 for (const v of path.includes("${kind}") ? ["issues", "pulls"] : [""]) found.add(`${method} ${norm(path.replace("${kind}", v))}`);
175 }
176 const calls = src.split("router.add(").length - 1;
177 if (read !== calls) throw new Error(`${f}: ${calls} router.add calls but only ${read} readable`);
178 }
179 return [...found].sort();
180 };
181
182 test("every registered /api/v1 route has an OpenAPI row or is listed as not yet described", () => {
183 const described = new Set(ROUTES.map((r) => `${r.method.toUpperCase()} ${norm(r.path)}`));
184 const all = registered();
185 const missing = all.filter((r) => !described.has(r) && !NOT_YET_DESCRIBED.includes(r));
186 const removable = NOT_YET_DESCRIBED.filter((r) => described.has(r));
187 console.log(`openapi-coverage: ${removable.length} NOT_YET_DESCRIBED entries could be removed`);
188 expect(missing, `routes with no OpenAPI row (add one to ROUTES in src/contract/openapi.ts):\n${missing.join("\n")}`).toEqual([]);
189 });
190
191 test("NOT_YET_DESCRIBED is sorted and names registered routes", () => {
192 expect([...NOT_YET_DESCRIBED]).toEqual([...NOT_YET_DESCRIBED].sort());
193 const all = new Set(registered());
194 expect(NOT_YET_DESCRIBED.filter((r) => !all.has(r))).toEqual([]);
195 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs