Skip to content

Commit e39ff2c

Browse files
justin-layervclaude
andcommitted
test+script: address cr round 5 — actionable diagnostics
cr round 5 (Approved, non-blocking) findings I acted on: - Completeness test: replace vitest's default set-diff with a custom remediation message. On drift, it names the offending method AND tells the contributor what to do: "add to SDK_PUBLIC_METHODS + add a case" OR "add to INTERNAL_HELPERS if not user-facing." Saves a future-dev the five minutes of figuring out where to update on first encounter. - Regen script: verify origin remote is qurl-service. Prevents the silent-foot-gun where someone with an unrelated repo at ../qurl-service that happens to contain api/openapi.yaml (or a fork) snapshots from the wrong source. The header SHA would otherwise be from the fork, not upstream. Skipped per cr's explicit guidance: - #2 (throw-vs-expect) — cr: "probably the right tradeoff" - #3 (second-page cursor) — cr: "no coverage gap … consistent with the CONTRIBUTING.md scope note" - #5 (mockOk in test-helpers) — cr: "not worth churning for" Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
1 parent 9e8bc68 commit e39ff2c

2 files changed

Lines changed: 33 additions & 1 deletion

File tree

scripts/update-openapi-snapshot.sh

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,18 @@ if [ ! -d "$QURL_SERVICE_DIR/.git" ]; then
2222
exit 2
2323
fi
2424

25+
# Verify the remote actually points at qurl-service. Without this, someone
26+
# with an unrelated repo at ../qurl-service that happens to contain
27+
# api/openapi.yaml could silently snapshot from the wrong source — the
28+
# header SHA would be from their fork, not upstream.
29+
REMOTE_URL=$(git -C "$QURL_SERVICE_DIR" remote get-url origin 2>/dev/null || echo "")
30+
if ! printf '%s' "$REMOTE_URL" | grep -q -E '[:/]qurl-service(\.git)?$'; then
31+
echo "error: $QURL_SERVICE_DIR origin remote does not look like qurl-service:" >&2
32+
echo " $REMOTE_URL" >&2
33+
echo " expected a URL ending in :qurl-service or /qurl-service(.git)" >&2
34+
exit 2
35+
fi
36+
2537
# Refresh remote refs so `origin/main` (the default) points at the current
2638
# upstream tip, not whatever was last fetched locally. Without this, a
2739
# stale checkout would silently pin the snapshot to an old SHA. Harmless

src/contract.test.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,27 @@ describe("OpenAPI contract", () => {
183183
const desc = Object.getOwnPropertyDescriptor(QURLClient.prototype, name);
184184
return typeof desc?.value === "function";
185185
});
186-
expect(new Set(prototypeMethods)).toEqual(SDK_PUBLIC_METHODS);
186+
const actual = new Set(prototypeMethods);
187+
const missingFromSet = [...actual].filter((m) => !SDK_PUBLIC_METHODS.has(m));
188+
const missingFromPrototype = [...SDK_PUBLIC_METHODS].filter((m) => !actual.has(m));
189+
// Custom remediation message — vitest's default `toEqual` diff on Sets
190+
// doesn't explain *what to do*. If this fires, the contributor needs
191+
// to either add the method to SDK_PUBLIC_METHODS + write a contract
192+
// case, or move it to INTERNAL_HELPERS if it's not a public API call.
193+
if (missingFromSet.length || missingFromPrototype.length) {
194+
throw new Error(
195+
`Contract set drift:\n` +
196+
(missingFromSet.length
197+
? ` New public method(s) on QURLClient.prototype: ${missingFromSet.join(", ")}\n` +
198+
` → add to SDK_PUBLIC_METHODS and add an it("…") case, ` +
199+
`OR add to INTERNAL_HELPERS if not a user-facing API call.\n`
200+
: "") +
201+
(missingFromPrototype.length
202+
? ` Listed in SDK_PUBLIC_METHODS but missing from prototype: ${missingFromPrototype.join(", ")}\n` +
203+
` → either the method was removed/renamed in client.ts, or the name here is wrong.\n`
204+
: ""),
205+
);
206+
}
187207
});
188208

189209
// Each SDK public method → one call → captured (verb, url) must match

0 commit comments

Comments
 (0)