mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 21:10:59 +00:00
test(guards): stop the slowest guards repeating their own work
Each of these asked the same question several times or did the expensive part before the cheap one: the threshold guard parsed every client file once per rule and twice per run, the i18n reverse check rebuilt its scan three times and compared every leaf with every literal key, the step-up caller guard stripped comments and searched every route path in every file per case, the plausibility guard ran its writer sweep three times, and the cycle verdict guard parsed every client file to find out whether it mentions a cycle. Each now works a result out once, and a file is parsed or searched only when a substring says it can match. A planted violation still fails the threshold, plausibility, cycle verdict and backup plan guards.
This commit is contained in:
1 parent
4dc3bd46bf
commit
743a1e742f
8 files changed
+153
-47
No files matched your search
@@ -271,10 +271,17 @@ function delegateName(model: string): string {
|
||||
* route outright: the guard stayed green on all twelve checks, because the
|
||||
* cycle file was answering a question about a different table.
|
||||
*/
|
||||
const parentsCache = new Map<
|
||||
string,
|
||||
Array<{ field: string; parent: string }>
|
||||
>();
|
||||
|
||||
function relationParents(model: string): Array<{
|
||||
field: string;
|
||||
parent: string;
|
||||
}> {
|
||||
const cached = parentsCache.get(model);
|
||||
if (cached) return cached;
|
||||
const found: Array<{ field: string; parent: string }> = [];
|
||||
for (const [parent, body] of modelBodies()) {
|
||||
for (const line of body.split("\n")) {
|
||||
@@ -284,6 +291,7 @@ function relationParents(model: string): Array<{
|
||||
}
|
||||
}
|
||||
}
|
||||
parentsCache.set(model, found);
|
||||
return found;
|
||||
}
|
||||
|
||||
@@ -295,6 +303,8 @@ const WRITE_RELATION_OPS = /\{\s*create\b/;
|
||||
/** Does `source` call a matching operation on `model`'s own Prisma delegate? */
|
||||
function callsDelegate(source: string, model: string, ops: RegExp): boolean {
|
||||
const delegate = delegateName(model);
|
||||
// The expression below names the delegate; a file without it cannot match.
|
||||
if (!source.includes(delegate)) return false;
|
||||
for (const match of source.matchAll(
|
||||
new RegExp(`\\.${delegate}\\s*\\.\\s*(\\w+)`, "g"),
|
||||
)) {
|
||||
@@ -304,7 +314,11 @@ function callsDelegate(source: string, model: string, ops: RegExp): boolean {
|
||||
}
|
||||
|
||||
/** Every model a relation field of this name points at, across the schema. */
|
||||
const targetsCache = new Map<string, Set<string>>();
|
||||
|
||||
function targetsOfRelationField(field: string): Set<string> {
|
||||
const cached = targetsCache.get(field);
|
||||
if (cached) return cached;
|
||||
const targets = new Set<string>();
|
||||
for (const [, body] of modelBodies()) {
|
||||
for (const line of body.split("\n")) {
|
||||
@@ -312,6 +326,7 @@ function targetsOfRelationField(field: string): Set<string> {
|
||||
if (match && match[1] === field) targets.add(match[2]);
|
||||
}
|
||||
}
|
||||
targetsCache.set(field, targets);
|
||||
return targets;
|
||||
}
|
||||
|
||||
@@ -337,6 +352,7 @@ function touches(
|
||||
const source = flat(file);
|
||||
if (callsDelegate(source, model, delegateOps)) return true;
|
||||
return relationParents(model).some(({ field, parent }) => {
|
||||
if (!source.includes(field)) return false;
|
||||
if (
|
||||
!new RegExp(`\\b${field}\\s*:\\s*(?:${relationOps.source})`).test(
|
||||
source,
|
||||
|
||||
@@ -33,13 +33,16 @@
|
||||
* from a variable that happens to hold a literal is not seen; review catches
|
||||
* that.
|
||||
*/
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
|
||||
import ts from "typescript";
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { stripComments, walkSourceFiles } from "./helpers/source-files";
|
||||
import {
|
||||
readSourceText,
|
||||
stripComments,
|
||||
walkSourceFiles,
|
||||
} from "./helpers/source-files";
|
||||
|
||||
const ROOT = process.cwd();
|
||||
|
||||
@@ -67,7 +70,7 @@ function clientFiles(): ClientFile[] {
|
||||
const rel = `${dir}/${file}`;
|
||||
if (rel.startsWith("src/app/api/")) continue;
|
||||
if (rel.includes("/__tests__/") || /\.test\.tsx?$/.test(rel)) continue;
|
||||
const source = readFileSync(join(ROOT, rel), "utf8");
|
||||
const source = readSourceText(join(ROOT, rel));
|
||||
if (!isClientModule(rel, source)) continue;
|
||||
out.push({ rel, source });
|
||||
}
|
||||
@@ -75,14 +78,23 @@ function clientFiles(): ClientFile[] {
|
||||
return out;
|
||||
}
|
||||
|
||||
const parsed = new Map<string, ts.SourceFile>();
|
||||
|
||||
/** One parse per file and source, however many rules read it. */
|
||||
function parse(name: string, source: string): ts.SourceFile {
|
||||
return ts.createSourceFile(
|
||||
name,
|
||||
source,
|
||||
ts.ScriptTarget.Latest,
|
||||
true,
|
||||
name.endsWith("x") ? ts.ScriptKind.TSX : ts.ScriptKind.TS,
|
||||
);
|
||||
const key = `${name}\0${source}`;
|
||||
let file = parsed.get(key);
|
||||
if (file === undefined) {
|
||||
file = ts.createSourceFile(
|
||||
name,
|
||||
source,
|
||||
ts.ScriptTarget.Latest,
|
||||
true,
|
||||
name.endsWith("x") ? ts.ScriptKind.TSX : ts.ScriptKind.TS,
|
||||
);
|
||||
parsed.set(key, file);
|
||||
}
|
||||
return file;
|
||||
}
|
||||
|
||||
/** The literal text a JSX attribute initialiser carries, if it is one. */
|
||||
@@ -218,6 +230,11 @@ function resolverImports(name: string, source: string): string[] {
|
||||
interface Rule {
|
||||
name: string;
|
||||
find: (name: string, source: string) => string[];
|
||||
/**
|
||||
* Text a file must contain for `find` to have anything to report, tested
|
||||
* before any parse. Every shape the rule matches carries one of these.
|
||||
*/
|
||||
markers: readonly string[];
|
||||
/** Sources that must produce at least one hit, proving the matcher. */
|
||||
samples: Array<{ name: string; source: string }>;
|
||||
/** File → exact hit count it may have, with the reason. */
|
||||
@@ -228,6 +245,7 @@ const RULES: Rule[] = [
|
||||
{
|
||||
name: "a literal directionSentiment",
|
||||
find: literalDirections,
|
||||
markers: ["directionSentiment"],
|
||||
samples: [
|
||||
{
|
||||
name: "sample.tsx",
|
||||
@@ -249,6 +267,7 @@ const RULES: Rule[] = [
|
||||
{
|
||||
name: "a reference line at a literal",
|
||||
find: literalLines,
|
||||
markers: [...LINE_TAGS],
|
||||
samples: [
|
||||
{
|
||||
name: "sample.tsx",
|
||||
@@ -280,12 +299,14 @@ const RULES: Rule[] = [
|
||||
{
|
||||
name: "a polarity table",
|
||||
find: polarityTables,
|
||||
markers: ["up-good", "up-bad", "UP_GOOD_", "UP_BAD_", "Sentiment"],
|
||||
samples: POLARITY_SAMPLES.map((source) => ({ name: "sample.ts", source })),
|
||||
allow: {},
|
||||
},
|
||||
{
|
||||
name: "a server resolver imported into the browser",
|
||||
find: resolverImports,
|
||||
markers: SERVER_RESOLVERS,
|
||||
samples: [
|
||||
{
|
||||
name: "sample.ts",
|
||||
@@ -299,12 +320,19 @@ const RULES: Rule[] = [
|
||||
|
||||
const FILES = clientFiles();
|
||||
|
||||
const ruleHits = new Map<Rule, Map<string, string[]>>();
|
||||
|
||||
/** Each rule's hits across the client tree, found once per run. */
|
||||
function hitsFor(rule: Rule): Map<string, string[]> {
|
||||
const out = new Map<string, string[]>();
|
||||
let out = ruleHits.get(rule);
|
||||
if (out !== undefined) return out;
|
||||
out = new Map<string, string[]>();
|
||||
for (const { rel, source } of FILES) {
|
||||
if (!rule.markers.some((marker) => source.includes(marker))) continue;
|
||||
const hits = rule.find(rel, source);
|
||||
if (hits.length > 0) out.set(rel, hits);
|
||||
}
|
||||
ruleHits.set(rule, out);
|
||||
return out;
|
||||
}
|
||||
|
||||
@@ -321,6 +349,12 @@ describe("client threshold literal guard", () => {
|
||||
rule.find(sample.name, sample.source).length,
|
||||
`no hit on: ${sample.source}`,
|
||||
).toBeGreaterThan(0);
|
||||
// The pre-filter has to let every sample through, or it would hide
|
||||
// the shape before the matcher saw it.
|
||||
expect(
|
||||
rule.markers.some((marker) => sample.source.includes(marker)),
|
||||
`the markers skip: ${sample.source}`,
|
||||
).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -13,7 +13,7 @@
|
||||
* nothing in the client tree may derive the verdict, AND the surfaces that
|
||||
* show it must still be reading the published field.
|
||||
*/
|
||||
import { readFileSync, readdirSync, statSync } from "node:fs";
|
||||
import { readdirSync, statSync } from "node:fs";
|
||||
import { join, relative as relativeTo } from "node:path";
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
@@ -21,8 +21,10 @@ import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
accessPaths,
|
||||
derivationFindings,
|
||||
mayBeCycleContext,
|
||||
parse,
|
||||
} from "./helpers/cycle-verdict-derivation";
|
||||
import { readSourceText } from "./helpers/source-files";
|
||||
|
||||
const ROOT = process.cwd();
|
||||
|
||||
@@ -61,7 +63,7 @@ function clientFiles(): string[] {
|
||||
}
|
||||
|
||||
function parseFile(relative: string) {
|
||||
return parse(relative, readFileSync(join(ROOT, relative), "utf8"));
|
||||
return parse(relative, readSourceText(join(ROOT, relative)));
|
||||
}
|
||||
|
||||
describe("the cycle verdict is decided in one place", () => {
|
||||
@@ -70,9 +72,11 @@ describe("the cycle verdict is decided in one place", () => {
|
||||
// A guard over an empty file list proves nothing.
|
||||
expect(files.length).toBeGreaterThan(100);
|
||||
|
||||
const findings = files.flatMap((relative) =>
|
||||
derivationFindings(parseFile(relative)),
|
||||
);
|
||||
const findings = files
|
||||
.filter((relative) =>
|
||||
mayBeCycleContext(readSourceText(join(ROOT, relative))),
|
||||
)
|
||||
.flatMap((relative) => derivationFindings(parseFile(relative)));
|
||||
|
||||
expect(
|
||||
findings.map((f) => `${f.file}:${f.line} ${f.kind} — ${f.where}`),
|
||||
@@ -83,7 +87,7 @@ describe("the cycle verdict is decided in one place", () => {
|
||||
// The specific constant that was copied. Named here so its return is loud
|
||||
// rather than merely structural.
|
||||
const offenders = clientFiles().filter((relative) =>
|
||||
readFileSync(join(ROOT, relative), "utf8").includes("OVERDUE_GRACE_DAYS"),
|
||||
readSourceText(join(ROOT, relative)).includes("OVERDUE_GRACE_DAYS"),
|
||||
);
|
||||
expect(offenders).toEqual([]);
|
||||
});
|
||||
@@ -106,9 +110,8 @@ describe("the surfaces that show the verdict actually read it", () => {
|
||||
// The BBT chart used to walk the calendar itself to find the cycle start,
|
||||
// which was a second copy of the same walk. It now takes the resolved
|
||||
// value as a prop, and the cycle page feeds it from the verdict.
|
||||
const chart = readFileSync(
|
||||
const chart = readSourceText(
|
||||
join(ROOT, "src/components/cycle/bbt-chart.tsx"),
|
||||
"utf8",
|
||||
);
|
||||
expect(chart).toContain("cycleStartDate");
|
||||
|
||||
@@ -119,10 +122,7 @@ describe("the surfaces that show the verdict actually read it", () => {
|
||||
it("the calendar response type declares the verdict as always present", () => {
|
||||
// An optional verdict is a plausible default that lets a client fall back
|
||||
// to deriving its own.
|
||||
const types = readFileSync(
|
||||
join(ROOT, "src/components/cycle/types.ts"),
|
||||
"utf8",
|
||||
);
|
||||
const types = readSourceText(join(ROOT, "src/components/cycle/types.ts"));
|
||||
expect(types).toContain("verdict: CycleVerdict;");
|
||||
expect(types).not.toContain("verdict?:");
|
||||
});
|
||||
|
||||
@@ -51,7 +51,7 @@ const DAY_COUNT_NAME = /day.?of.?cycle|cycleday|cyclerun|runlength/i;
|
||||
* calendar's types, its phase names, or the verdict's own fields, so nothing
|
||||
* that could actually commit the defect escapes by renaming one identifier.
|
||||
*/
|
||||
const CYCLE_MARKERS = new Set([
|
||||
export const CYCLE_MARKERS: ReadonlySet<string> = new Set([
|
||||
"CalendarDay",
|
||||
"CalendarResponse",
|
||||
"CyclePhase",
|
||||
@@ -247,6 +247,16 @@ function isSumLike(
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a file's text could speak the cycle's vocabulary at all: every
|
||||
* marker the parse below looks for is also a substring of the text, so a
|
||||
* file holding none of them needs no parse.
|
||||
*/
|
||||
export function mayBeCycleContext(text: string): boolean {
|
||||
for (const marker of CYCLE_MARKERS) if (text.includes(marker)) return true;
|
||||
return false;
|
||||
}
|
||||
|
||||
/** Whether the file speaks the cycle's vocabulary at all. */
|
||||
export function isCycleContext(file: ts.SourceFile): boolean {
|
||||
let found = false;
|
||||
|
||||
@@ -305,11 +305,19 @@ function suffixBaseMatches(base: string, ref: Reference): boolean {
|
||||
|
||||
export function isReferenced(leaf: string, ref: Reference): boolean {
|
||||
if (ref.literalKeys.has(leaf)) return true;
|
||||
for (const ck of ref.literalKeys) {
|
||||
if (leaf.startsWith(`${ck}.`)) return true;
|
||||
}
|
||||
for (const base of ref.dottedBases) {
|
||||
if (leaf === base || leaf.startsWith(`${base}.`)) return true;
|
||||
// A literal key or a dotted base covers `leaf` when it IS one of the
|
||||
// leaf's dotted ancestors (or, for a base, the leaf itself), so the leaf's
|
||||
// own ancestors are looked up rather than every key in the sets scanned.
|
||||
if (ref.dottedBases.has(leaf)) return true;
|
||||
for (
|
||||
let dot = leaf.indexOf(".");
|
||||
dot !== -1;
|
||||
dot = leaf.indexOf(".", dot + 1)
|
||||
) {
|
||||
const ancestor = leaf.slice(0, dot);
|
||||
if (ref.literalKeys.has(ancestor) || ref.dottedBases.has(ancestor)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
for (const pre of ref.concatPrefixes) {
|
||||
if (leaf.startsWith(pre)) return true;
|
||||
|
||||
@@ -26,10 +26,11 @@
|
||||
* its keys report as orphans.
|
||||
*/
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { readFileSync, readdirSync, statSync } from "node:fs";
|
||||
import { readdirSync, statSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
|
||||
import { collectReferences, isReferenced } from "./helpers/i18n-reference";
|
||||
import { readSourceText } from "./helpers/source-files";
|
||||
|
||||
const ROOT = join(__dirname, "../..");
|
||||
const SRC = join(ROOT, "src");
|
||||
@@ -142,8 +143,16 @@ function isAllowlisted(leaf: string): boolean {
|
||||
);
|
||||
}
|
||||
|
||||
let scanned: ReturnType<typeof scanOnce> | null = null;
|
||||
|
||||
/** The bundle and the tree's references, worked out once per run. */
|
||||
function scan() {
|
||||
const en = JSON.parse(readFileSync(EN_BUNDLE_PATH, "utf8"));
|
||||
scanned ??= scanOnce();
|
||||
return scanned;
|
||||
}
|
||||
|
||||
function scanOnce() {
|
||||
const en = JSON.parse(readSourceText(EN_BUNDLE_PATH));
|
||||
const leaves: string[] = [];
|
||||
leafPaths(en, "", leaves);
|
||||
|
||||
@@ -152,7 +161,7 @@ function scan() {
|
||||
const topNamespaces = new Set<string>(Object.keys(en));
|
||||
|
||||
const allText = walk(SRC)
|
||||
.map((f) => readFileSync(f, "utf8"))
|
||||
.map((f) => readSourceText(f))
|
||||
.join("\n");
|
||||
|
||||
return { leaves, ...collectReferences(allText, allNodes, topNamespaces) };
|
||||
|
||||
@@ -19,10 +19,9 @@
|
||||
* test substitutes for that review.
|
||||
*/
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
|
||||
import { walkSourceFiles } from "./helpers/source-files";
|
||||
import { readSourceText, walkSourceFiles } from "./helpers/source-files";
|
||||
|
||||
const SRC = join(process.cwd(), "src");
|
||||
|
||||
@@ -164,14 +163,25 @@ function sourceFiles(): string[] {
|
||||
}
|
||||
|
||||
function read(rel: string): string {
|
||||
return readFileSync(join(SRC, rel), "utf8");
|
||||
return readSourceText(join(SRC, rel));
|
||||
}
|
||||
|
||||
let writers: string[] | null = null;
|
||||
|
||||
/**
|
||||
* Every file that writes a measurement, found once per run. A write names
|
||||
* the `measurement` delegate or the bulk insert's `MeasurementRows`, so a
|
||||
* file holding neither word is skipped before the expressions run.
|
||||
*/
|
||||
function discoveredWriters(): string[] {
|
||||
return sourceFiles().filter((rel) => {
|
||||
writers ??= sourceFiles().filter((rel) => {
|
||||
const text = read(rel);
|
||||
if (!text.includes("measurement") && !text.includes("MeasurementRows")) {
|
||||
return false;
|
||||
}
|
||||
return MEASUREMENT_WRITE.test(text) || BULK_INSERT_CALL.test(text);
|
||||
});
|
||||
return writers;
|
||||
}
|
||||
|
||||
describe("every measurement writer declares how it meets the plausibility domain", () => {
|
||||
@@ -195,9 +205,10 @@ describe("every measurement writer declares how it meets the plausibility domain
|
||||
});
|
||||
|
||||
it("raw SQL inserts measurements in the bulk-insert module and nowhere else", () => {
|
||||
const raw = sourceFiles().filter((rel) =>
|
||||
RAW_MEASUREMENT_INSERT.test(read(rel)),
|
||||
);
|
||||
const raw = sourceFiles().filter((rel) => {
|
||||
const text = read(rel);
|
||||
return /insert/i.test(text) && RAW_MEASUREMENT_INSERT.test(text);
|
||||
});
|
||||
expect(raw).toEqual([BULK_INSERT_MODULE]);
|
||||
});
|
||||
|
||||
|
||||
@@ -32,27 +32,33 @@
|
||||
* with every list.
|
||||
*/
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
|
||||
import { walkSourceFiles } from "./helpers/source-files";
|
||||
import { readSourceText, walkSourceFiles } from "./helpers/source-files";
|
||||
|
||||
const SRC = join(process.cwd(), "src");
|
||||
|
||||
function read(rel: string): string {
|
||||
return readFileSync(join(SRC, rel), "utf8");
|
||||
return readSourceText(join(SRC, rel));
|
||||
}
|
||||
|
||||
/**
|
||||
* Source with comments removed, so a route that only DESCRIBES a gate in its
|
||||
* doc comment is not counted, and a path quoted in a comment is not a call.
|
||||
*/
|
||||
const codeCache = new Map<string, string>();
|
||||
|
||||
function code(rel: string): string {
|
||||
return read(rel)
|
||||
.replace(/\/\*[\s\S]*?\*\//g, "")
|
||||
.split("\n")
|
||||
.map((line) => line.replace(/(^|[^:"'`\\])\/\/.*$/, "$1"))
|
||||
.join("\n");
|
||||
let stripped = codeCache.get(rel);
|
||||
if (stripped === undefined) {
|
||||
stripped = read(rel)
|
||||
.replace(/\/\*[\s\S]*?\*\//g, "")
|
||||
.split("\n")
|
||||
.map((line) => line.replace(/(^|[^:"'`\\])\/\/.*$/, "$1"))
|
||||
.join("\n");
|
||||
codeCache.set(rel, stripped);
|
||||
}
|
||||
return stripped;
|
||||
}
|
||||
|
||||
const METHODS = ["GET", "POST", "PUT", "PATCH", "DELETE"] as const;
|
||||
@@ -252,7 +258,15 @@ function hookCallersWrap(
|
||||
return callers > 0;
|
||||
}
|
||||
|
||||
let gatedCalls: GatedCall[] | null = null;
|
||||
|
||||
/** Every web call of a gated route, found once per run. */
|
||||
function gatedWebCalls(): GatedCall[] {
|
||||
gatedCalls ??= findGatedWebCalls();
|
||||
return gatedCalls;
|
||||
}
|
||||
|
||||
function findGatedWebCalls(): GatedCall[] {
|
||||
const calls: GatedCall[] = [];
|
||||
const files = webFiles();
|
||||
const sources = new Map(files.map((f) => [f, code(f)]));
|
||||
@@ -260,8 +274,12 @@ function gatedWebCalls(): GatedCall[] {
|
||||
if (GATED_ROUTES[route] !== "web") continue;
|
||||
const [method, path] = route.split(" ");
|
||||
const re = pathPattern(path);
|
||||
// Every match spells the path's static head out literally, up to the
|
||||
// first `[param]`; a file without it cannot match and is not searched.
|
||||
const head = path.split("/[")[0];
|
||||
for (const file of files) {
|
||||
const src = sources.get(file) ?? "";
|
||||
if (!src.includes(head)) continue;
|
||||
for (const m of src.matchAll(re)) {
|
||||
const index = m.index ?? 0;
|
||||
// The path handed to a component as a prop (`endpoint="/api/…"`):
|
||||
|
||||
Reference in new issue
Block a user