mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-11 22:02:16 +00:00
test: guard tile eligibility against a missing null arm
The defect was invisible from the outside because every behavioural test in the suite seeded a tagged reading. Two checks close that: A structural guard that reads the dashboard surfaces and fails when a tile derives presence from a list of enum buckets with no null / unknown arm and no written reason, following one hop through a named constant. It carries its own positive and negative controls, so a refactor that silently stops matching anything fails here rather than going quiet. An integration test that seeds twenty readings with a genuinely NULL context against a real Postgres and reads the tile back through the same resolver the page renders with: eligible, latest value intact, and a tagged reading still filed under its own context. Refs #943
This commit is contained in:
1 parent
126383b352
commit
ad62ab6f27
2 files changed
+362
No files matched your search
@@ -0,0 +1,197 @@
|
||||
/**
|
||||
* Structural guard — a dashboard tile may not be gated on a closed enum
|
||||
* whose column is nullable.
|
||||
*
|
||||
* The Blood Glucose tile decided whether to paint by filtering the four
|
||||
* `GlucoseContext` members against a per-context summary record and asking
|
||||
* whether any of them held readings. Every named context is a legal value of
|
||||
* the column, so the list read as exhaustive — but `glucose_context` is
|
||||
* nullable, and a meter synced through Apple Health writes no meal-time
|
||||
* metadata at all. An account with one hundred percent untagged readings
|
||||
* therefore matched no bucket, and the tile was omitted with the module on,
|
||||
* the layout toggle on, and a reading from today. Nothing warned: every
|
||||
* behavioural test in the suite seeded a tagged reading, so the missing arm
|
||||
* was invisible from the outside (#943).
|
||||
*
|
||||
* What the guard asserts: wherever a dashboard tile surface derives presence
|
||||
* from a list of enum buckets, the list must carry a null / unknown arm — or
|
||||
* the call site must write down why it cannot. It matches on the list
|
||||
* EXPRESSION, following one hop through a named constant, because the fix for
|
||||
* this class is always the same: the untagged bucket joins the list the
|
||||
* eligibility loop already walks.
|
||||
*
|
||||
* Its limits, written down so a later reader does not over-trust it: it sees
|
||||
* presence tests spelled with `count`, and a bucket list that is either an
|
||||
* inline literal or a single named constant. An eligibility gate that counted
|
||||
* rows some other way, or assembled its bucket list at runtime, would slip
|
||||
* past. The `detect()` self-controls below exist so a refactor that silently
|
||||
* stops matching anything fails here rather than going quiet.
|
||||
*/
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { readFileSync, readdirSync, statSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
|
||||
/**
|
||||
* A bucket list that carries one of these tokens has an arm for rows the
|
||||
* enum cannot name. Matched against the list expression's source text, so a
|
||||
* shared constant (`GLUCOSE_CONTEXT_BUCKETS = [...NAMED, UNSPECIFIED]`)
|
||||
* satisfies it through its own name.
|
||||
*/
|
||||
const UNTAGGED_ARM = /UNSPECIFIED|UNKNOWN|UNTAGGED|OTHER|NONE/;
|
||||
|
||||
/** The written-reason escape hatch, with a non-empty reason after it. */
|
||||
const WRITTEN_REASON = /tile-eligibility-exempt:\s*\S/;
|
||||
|
||||
/** Dashboard surfaces that decide whether a tile paints. */
|
||||
const SURFACES = [
|
||||
"src/app/page-client.tsx",
|
||||
...listSourceFiles("src/components/dashboard"),
|
||||
...listSourceFiles("src/lib/dashboard"),
|
||||
];
|
||||
|
||||
function listSourceFiles(dir: string): string[] {
|
||||
const abs = join(process.cwd(), dir);
|
||||
return readdirSync(abs)
|
||||
.filter((name) => /\.tsx?$/.test(name))
|
||||
.map((name) => join(dir, name))
|
||||
.filter((rel) => statSync(join(process.cwd(), rel)).isFile());
|
||||
}
|
||||
|
||||
interface Finding {
|
||||
/** The bucket-list expression as written at the eligibility site. */
|
||||
list: string;
|
||||
/** 1-based line of the eligibility site. */
|
||||
line: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Iteration over a bucket list: `for (const x of LIST)` or
|
||||
* `LIST.filter(…)` / `.some(…)`. The list is either an inline array literal
|
||||
* or an identifier.
|
||||
*/
|
||||
const ITERATION =
|
||||
/(?:for\s*\(\s*(?:const|let)\s+[^)]*?\bof\s+(\[[^\]]*\]|[A-Za-z_$][\w$]*)\s*\))|(?:(\[[^\]]*\]|[A-Za-z_$][\w$]*)\s*\.\s*(?:filter|some|flatMap)\s*\()/g;
|
||||
|
||||
/** A presence test — "does this bucket hold any readings". */
|
||||
const PRESENCE = /\bcount\b/;
|
||||
|
||||
/** How far past the iteration head a presence test still belongs to it. */
|
||||
const BODY_WINDOW = 500;
|
||||
|
||||
/**
|
||||
* Every eligibility site in `source` whose bucket list has no untagged arm
|
||||
* and no written reason. `resolve` supplies the initializer text of a named
|
||||
* constant so the guard can follow one hop out of the file.
|
||||
*/
|
||||
export function detect(
|
||||
source: string,
|
||||
resolve: (name: string) => string | null,
|
||||
): Finding[] {
|
||||
const findings: Finding[] = [];
|
||||
ITERATION.lastIndex = 0;
|
||||
let match: RegExpExecArray | null;
|
||||
while ((match = ITERATION.exec(source)) !== null) {
|
||||
const list = match[1] ?? match[2];
|
||||
if (!list) continue;
|
||||
const body = source.slice(match.index, match.index + BODY_WINDOW);
|
||||
if (!PRESENCE.test(body)) continue;
|
||||
// Only enum-shaped lists are in scope: an inline literal of UPPER_SNAKE
|
||||
// members, or an UPPER_SNAKE constant name.
|
||||
const inline = list.startsWith("[");
|
||||
const members = inline ? list.match(/"[A-Z][A-Z0-9_]*"/g) : null;
|
||||
if (inline && (members?.length ?? 0) < 2) continue;
|
||||
if (!inline && !/^[A-Z][A-Z0-9_]*$/.test(list)) continue;
|
||||
const expression = inline ? list : (resolve(list) ?? list);
|
||||
if (UNTAGGED_ARM.test(expression)) continue;
|
||||
const before = source.slice(Math.max(0, match.index - 400), match.index);
|
||||
if (WRITTEN_REASON.test(before)) continue;
|
||||
findings.push({
|
||||
list,
|
||||
line: source.slice(0, match.index).split("\n").length,
|
||||
});
|
||||
}
|
||||
return findings;
|
||||
}
|
||||
|
||||
/** Initializer text of `export const NAME = …` / `const NAME = …` in `src/`. */
|
||||
function resolveConstant(name: string): string | null {
|
||||
for (const file of CONSTANT_SOURCES) {
|
||||
const source = readFileSync(join(process.cwd(), file), "utf8");
|
||||
const declaration = new RegExp(
|
||||
`(?:export\\s+)?const\\s+${name}\\s*(?::[^=]+)?=\\s*(\\[[\\s\\S]*?\\])`,
|
||||
).exec(source);
|
||||
if (declaration) return declaration[1];
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/** Files a dashboard surface may import a bucket list from. */
|
||||
const CONSTANT_SOURCES = ["src/lib/glucose.ts", ...SURFACES];
|
||||
|
||||
describe("detect() — the matcher itself", () => {
|
||||
it("flags the shape that shipped: four enum members, no null arm", () => {
|
||||
const shipped = `
|
||||
const contexts = ["FASTING", "POSTPRANDIAL", "RANDOM", "BEDTIME"];
|
||||
const present = ["FASTING", "POSTPRANDIAL", "RANDOM", "BEDTIME"].filter(
|
||||
(ctx) => (byContext[ctx]?.count ?? 0) > 0,
|
||||
);
|
||||
`;
|
||||
const findings = detect(shipped, () => null);
|
||||
expect(findings).toHaveLength(1);
|
||||
expect(findings[0].list).toContain("FASTING");
|
||||
});
|
||||
|
||||
it("accepts a list that carries an untagged arm", () => {
|
||||
const fixed = `
|
||||
for (const bucket of GLUCOSE_CONTEXT_BUCKETS) {
|
||||
const summary = byContext?.[bucket];
|
||||
if (!summary || summary.count <= 0) continue;
|
||||
}
|
||||
`;
|
||||
const resolve = () => '["FASTING", "BEDTIME", GLUCOSE_CONTEXT_UNSPECIFIED]';
|
||||
expect(detect(fixed, resolve)).toEqual([]);
|
||||
// …and rejects the same site when the constant has no arm, so the pass
|
||||
// above is the arm's doing and not the resolver's.
|
||||
expect(detect(fixed, () => '["FASTING", "BEDTIME"]')).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("accepts a written reason", () => {
|
||||
const exempt = `
|
||||
// tile-eligibility-exempt: sleep stages are never null on this path.
|
||||
for (const stage of ["DEEP", "REM"]) {
|
||||
if (byStage[stage].count > 0) return true;
|
||||
}
|
||||
`;
|
||||
expect(detect(exempt, () => null)).toEqual([]);
|
||||
});
|
||||
|
||||
it("ignores lists that are not presence tests", () => {
|
||||
const unrelated = `
|
||||
const labels = ["WEIGHT", "PULSE"].map((type) => LABELS[type]);
|
||||
`;
|
||||
expect(detect(unrelated, () => null)).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("dashboard tile eligibility", () => {
|
||||
it("scans the surfaces it claims to scan", () => {
|
||||
// An empty match set must fail rather than read as clean: the guard is
|
||||
// worthless the day its matchers stop finding the real call sites.
|
||||
expect(SURFACES.length).toBeGreaterThan(5);
|
||||
const sited = SURFACES.filter((file) =>
|
||||
PRESENCE.test(readFileSync(join(process.cwd(), file), "utf8")),
|
||||
);
|
||||
expect(sited.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("computes no tile's eligibility from an enum without a null arm", () => {
|
||||
const offenders: string[] = [];
|
||||
for (const file of SURFACES) {
|
||||
const source = readFileSync(join(process.cwd(), file), "utf8");
|
||||
for (const finding of detect(source, resolveConstant)) {
|
||||
offenders.push(`${file}:${finding.line} — ${finding.list}`);
|
||||
}
|
||||
}
|
||||
expect(offenders).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,165 @@
|
||||
/**
|
||||
* #943 — untagged glucose readings light the dashboard tile.
|
||||
*
|
||||
* A reporter's meter syncs through Apple Health and never writes HealthKit's
|
||||
* blood-glucose meal-time metadata, so all of their readings carry a NULL
|
||||
* `glucose_context`. The tile strip filtered the four named contexts against
|
||||
* the per-context summaries, matched none, and omitted the tile — module on,
|
||||
* layout toggle on, a reading from today, and no warning anywhere.
|
||||
*
|
||||
* A real Postgres is what makes this convincing: the summaries are built from
|
||||
* rows the database actually holds, with the column genuinely NULL, and the
|
||||
* tile gate is the same pure resolver the page renders through.
|
||||
*/
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
import { cookieJar, headerJar } from "./mock-next-headers";
|
||||
import { getPrismaClient, truncateAllTables } from "./setup";
|
||||
|
||||
import { resolveGlucoseTiles } from "@/components/dashboard/dashboard-gates";
|
||||
import type { DataSummary } from "@/lib/analytics/trends";
|
||||
|
||||
vi.mock("next/headers", async () => {
|
||||
const { cookieJar, headerJar } = await import("./mock-next-headers");
|
||||
return {
|
||||
headers: vi.fn(async () => ({
|
||||
get: (name: string) => headerJar.get(name.toLowerCase()) ?? null,
|
||||
})),
|
||||
cookies: vi.fn(async () => ({
|
||||
get: (name: string) => {
|
||||
const value = cookieJar.get(name);
|
||||
return value ? { name, value } : undefined;
|
||||
},
|
||||
set: (name: string, value: string) => {
|
||||
cookieJar.set(name, value);
|
||||
},
|
||||
delete: (name: string) => {
|
||||
cookieJar.delete(name);
|
||||
},
|
||||
})),
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("@/lib/db-compat", () => ({
|
||||
ensureDbCompatibility: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
interface AnalyticsEnvelope {
|
||||
data: {
|
||||
glucoseByContext: Record<string, DataSummary> | null;
|
||||
};
|
||||
error: null;
|
||||
}
|
||||
|
||||
const DAY_MS = 24 * 60 * 60 * 1000;
|
||||
const READINGS = 20;
|
||||
|
||||
beforeEach(async () => {
|
||||
await truncateAllTables(getPrismaClient());
|
||||
cookieJar.clear();
|
||||
headerJar.clear();
|
||||
});
|
||||
|
||||
async function seedUntaggedGlucose() {
|
||||
const prisma = getPrismaClient();
|
||||
const user = await prisma.user.create({
|
||||
data: {
|
||||
username: "glucose-untagged",
|
||||
email: "glucose-untagged@example.test",
|
||||
role: "USER",
|
||||
timezone: "Europe/Berlin",
|
||||
},
|
||||
});
|
||||
const session = await prisma.session.create({
|
||||
data: { userId: user.id, expiresAt: new Date(Date.now() + 60_000) },
|
||||
});
|
||||
cookieJar.set("healthlog_session", session.id);
|
||||
|
||||
// 20 readings, one per day, newest first at 104 mg/dL. Every one of them
|
||||
// arrives exactly as the reporter's sync writes them: no meal-time context.
|
||||
const now = Date.now();
|
||||
await prisma.measurement.createMany({
|
||||
data: Array.from({ length: READINGS }, (_, i) => ({
|
||||
userId: user.id,
|
||||
type: "BLOOD_GLUCOSE" as const,
|
||||
value: 104 + i,
|
||||
unit: "mg/dL",
|
||||
source: "APPLE_HEALTH" as const,
|
||||
measuredAt: new Date(now - i * DAY_MS),
|
||||
glucoseContext: null,
|
||||
})),
|
||||
});
|
||||
return user;
|
||||
}
|
||||
|
||||
async function readAnalytics(): Promise<AnalyticsEnvelope> {
|
||||
const { GET } = await import("@/app/api/analytics/route");
|
||||
const response = await (
|
||||
GET as unknown as (req: Request) => Promise<Response>
|
||||
)(new Request("http://localhost/api/analytics"));
|
||||
expect(response.status).toBe(200);
|
||||
return (await response.json()) as AnalyticsEnvelope;
|
||||
}
|
||||
|
||||
describe("dashboard glucose tile with untagged readings (#943)", () => {
|
||||
it("stores the readings with a NULL context", async () => {
|
||||
const user = await seedUntaggedGlucose();
|
||||
const rows = await getPrismaClient().measurement.findMany({
|
||||
where: { userId: user.id, type: "BLOOD_GLUCOSE" },
|
||||
select: { glucoseContext: true },
|
||||
});
|
||||
|
||||
expect(rows).toHaveLength(READINGS);
|
||||
expect(rows.every((row) => row.glucoseContext === null)).toBe(true);
|
||||
});
|
||||
|
||||
it("reports the tile as eligible and carries the latest value", async () => {
|
||||
await seedUntaggedGlucose();
|
||||
|
||||
const envelope = await readAnalytics();
|
||||
const byContext = envelope.data.glucoseByContext;
|
||||
expect(byContext).not.toBeNull();
|
||||
|
||||
// The summaries carry the untagged bucket, and nothing else.
|
||||
expect(Object.keys(byContext ?? {})).toEqual(["UNSPECIFIED"]);
|
||||
const summary = byContext!.UNSPECIFIED;
|
||||
expect(summary.count).toBe(READINGS);
|
||||
expect(summary.latest).toBe(104);
|
||||
expect(summary.avg30).not.toBeNull();
|
||||
|
||||
// …and the tile gate the dashboard renders through says "paint it".
|
||||
const tiles = resolveGlucoseTiles(byContext ?? undefined);
|
||||
expect(tiles).toHaveLength(1);
|
||||
expect(tiles[0]).toMatchObject({
|
||||
bucket: "UNSPECIFIED",
|
||||
labelKey: "targets.glucoseUnspecified",
|
||||
});
|
||||
expect(tiles[0].summary.latest).toBe(104);
|
||||
});
|
||||
|
||||
it("keeps a tagged reading in its own bucket beside the untagged ones", async () => {
|
||||
const user = await seedUntaggedGlucose();
|
||||
await getPrismaClient().measurement.create({
|
||||
data: {
|
||||
userId: user.id,
|
||||
type: "BLOOD_GLUCOSE",
|
||||
value: 92,
|
||||
unit: "mg/dL",
|
||||
source: "MANUAL",
|
||||
measuredAt: new Date(),
|
||||
glucoseContext: "FASTING",
|
||||
},
|
||||
});
|
||||
|
||||
const envelope = await readAnalytics();
|
||||
const byContext = envelope.data.glucoseByContext ?? {};
|
||||
|
||||
expect(Object.keys(byContext).sort()).toEqual(["FASTING", "UNSPECIFIED"]);
|
||||
expect(byContext.FASTING.count).toBe(1);
|
||||
expect(byContext.UNSPECIFIED.count).toBe(READINGS);
|
||||
expect(resolveGlucoseTiles(byContext).map((tile) => tile.bucket)).toEqual([
|
||||
"FASTING",
|
||||
"UNSPECIFIED",
|
||||
]);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user