From 34026d22b4512ca90b71660ded408e7ccce1089b Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Thu, 20 Aug 2026 20:06:51 +0200 Subject: [PATCH] fix(logging): keep debug logging in a packaged debug build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit import.meta.env.DEV is true only under the vite dev server, but scripts/build-android.sh produces the debug APK with a plain `bun run build` — so the logger defaulted to warn there too and the debug package lost every frontend message from logcat. `bun run android:logs` is a documented workflow that depends on them. vite now defines __JT_DEBUG_BUILD__ from Tauri's TAURI_ENV_DEBUG, which the CLI sets while running beforeBuildCommand. The decision is split into a pure resolveDefaultLogLevel(isDevServer, isDebugBuild) because neither import.meta.env.DEV nor a vite define can be varied from inside a test. Also replaces the pinned requirement counts in extract-traces.test.ts with invariants. The pins guarded nothing the computeCoverage fixtures don't already cover, while forcing every branch that adds a requirement to edit the numbers — the comment above them had become a ledger of which branch contributed which row. TRACES: | DR-204 | UT-201 --- scripts/extract-traces.test.ts | 49 ++++++++++++++++++---------------- src/lib/utils/logger.test.ts | 26 ++++++++++++++++++ src/lib/utils/logger.ts | 30 ++++++++++++++++++++- vite.config.js | 8 ++++++ 4 files changed, 89 insertions(+), 24 deletions(-) diff --git a/scripts/extract-traces.test.ts b/scripts/extract-traces.test.ts index b984d920..d88fcced 100644 --- a/scripts/extract-traces.test.ts +++ b/scripts/extract-traces.test.ts @@ -322,34 +322,37 @@ describe("generated matrix file links", () => { }); describe("live requirements.md", () => { - it("parses the real file to the counts the CI gate must use", () => { - // Guards the specific regression: CI hardcoded UR/39, IR/24, DR/48, JA/3 - // (total 114) while the real file had grown to 211. Update these numbers - // deliberately when requirements are added — that edit is the signal the - // denominator is live rather than frozen. + it("parses the real file into a self-consistent denominator", () => { + // Guards the original regression: CI hardcoded UR/39, IR/24, DR/48, JA/3 + // (total 114) while the real file had grown past 200, so the gate compared + // live traces against a frozen denominator and reported 158% coverage. + // + // Deliberately asserts *invariants*, not exact totals. Pinning the counts + // was tried and turned this test into a merge-conflict magnet: every + // requirement added on any branch had to edit the numbers here too, and the + // comment above them grew into a ledger of which branch contributed which + // row. Worse, the pins never guarded the actual defect — a stale denominator + // is caught by the sum-consistency check below, and the >100% ratio it + // produced is covered directly by the computeCoverage tests, on fixtures. const md = fs.readFileSync( path.resolve(HERE, "../docs/requirements.md"), "utf-8" ); const defined = countDefinedRequirements(md); - expect(defined.UR).toBe(76); - expect(defined.IR).toBe(32); - // 192 = 187 + four requirements added independently on four audit branches, - // plus DR-201 (lockscreen skip resolution). Originally 191 = 187 + four - // that landed together: DR-189 (control-bar auto-hide), DR-198 (asset - // scope/CSP), DR-199 (webview mixed-content) and DR-200 (the - // POST_NOTIFICATIONS media-session exemption; renumbered from 198 on - // merge, where it collided). Each branch bumped for its own — merged, - // they sum. Resolve this by summing, never by taking one side. 193 adds - // DR-202 (video keeps the display awake), 194 DR-203 (the handoff - // transcode refusing the player's own load-error retry). 200 adds the - // six tooling/quality requirements DR-204..DR-209 (logging facade, lint - // gate, pinned toolchain, pre-commit hook, doc-link check, server-side - // library folder exclusion); UR rises to 76 with UR-076, which DR-209 - // serves. - expect(defined.DR).toBe(200); - expect(defined.JA).toBe(36); - expect(defined.total).toBe(344); + // The parser found real rows of every type: a section silently failing to + // parse would shrink the denominator and inflate coverage. + expect(defined.UR).toBeGreaterThan(0); + expect(defined.IR).toBeGreaterThan(0); + expect(defined.DR).toBeGreaterThan(0); + expect(defined.JA).toBeGreaterThan(0); + + // The denominator is the sum of its parts, and every counted id is unique — + // double-counting one section is the other way a ratio breaks. + expect(defined.total).toBe(defined.UR + defined.IR + defined.DR + defined.JA); + expect(defined.ids.size).toBe(defined.total); + + // The file is live, not frozen: it is well past the 114 the stale gate used. + expect(defined.total).toBeGreaterThan(200); }); }); diff --git a/src/lib/utils/logger.test.ts b/src/lib/utils/logger.test.ts index 0ccd14ea..40e4572f 100644 --- a/src/lib/utils/logger.test.ts +++ b/src/lib/utils/logger.test.ts @@ -8,6 +8,7 @@ import { parseLogLevel, readStoredLogLevel, resetLogLevel, + resolveDefaultLogLevel, setLogLevel, type LogLevel, } from "./logger"; @@ -316,3 +317,28 @@ describe("localStorage override", () => { expect(spies.error).toHaveBeenCalledWith("[Boot] still reaches the console"); }); }); + +/** + * A packaged *debug* build must still log at debug level. + * + * `import.meta.env.DEV` is true only under the vite dev server. Both the debug + * and the release APK are produced by a plain `vite build` + * (scripts/build-android.sh runs `bun run build`), so gating on DEV alone + * silences the debug APK too — and `bun run android:logs` is a documented + * workflow that depends on those messages reaching logcat. + * + * TRACES: | DR-204 | UT-201 + */ +describe("resolveDefaultLogLevel", () => { + it("logs at debug under the dev server", () => { + expect(resolveDefaultLogLevel(true, false)).toBe("debug"); + }); + + it("logs at debug in a packaged debug build", () => { + expect(resolveDefaultLogLevel(false, true)).toBe("debug"); + }); + + it("stays quiet in a packaged release build", () => { + expect(resolveDefaultLogLevel(false, false)).toBe("warn"); + }); +}); diff --git a/src/lib/utils/logger.ts b/src/lib/utils/logger.ts index e27a3dc4..de7dde48 100644 --- a/src/lib/utils/logger.ts +++ b/src/lib/utils/logger.ts @@ -115,9 +115,37 @@ export function parseLogLevel(raw: unknown): LogLevel | null { return normalised in LEVEL_RANK ? (normalised as LogLevel) : null; } +/** + * Injected by vite (see `vite.config.js`) from Tauri's `TAURI_ENV_DEBUG`, which + * the CLI sets while running `beforeBuildCommand`. Undefined outside a Tauri + * build — a bare `vite build`, or vitest — hence the `typeof` guard. + */ +declare const __JT_DEBUG_BUILD__: boolean | undefined; + +/** Is this bundle inside a *debug* Tauri package (a debug APK, say)? */ +function isDebugBuild(): boolean { + return typeof __JT_DEBUG_BUILD__ !== "undefined" && __JT_DEBUG_BUILD__ === true; +} + +/** + * The default level, as a pure function of the two build facts it depends on. + * + * Split out from {@link defaultLogLevel} so it can be tested — neither + * `import.meta.env.DEV` nor a vite `define` can be varied from inside a test. + * + * 🔴 `isDevServer` alone is not enough. `import.meta.env.DEV` is true only under + * the vite dev server, and `scripts/build-android.sh` produces the debug APK + * with a plain `bun run build` — so gating on it silences the debug package as + * thoroughly as the release one, and `bun run android:logs` stops showing + * anything from the frontend. + */ +export function resolveDefaultLogLevel(isDevServer: boolean, isDebugBuild: boolean): LogLevel { + return isDevServer || isDebugBuild ? "debug" : "warn"; +} + /** The level a build defaults to with no override present. */ export function defaultLogLevel(): LogLevel { - return import.meta.env?.DEV ? "debug" : "warn"; + return resolveDefaultLogLevel(Boolean(import.meta.env?.DEV), isDebugBuild()); } /** diff --git a/vite.config.js b/vite.config.js index 8c81e853..afec61ab 100644 --- a/vite.config.js +++ b/vite.config.js @@ -8,6 +8,14 @@ const host = process.env.TAURI_DEV_HOST; export default defineConfig(async () => ({ plugins: [sveltekit(), tailwindcss()], + // Tauri sets TAURI_ENV_DEBUG while it runs `beforeBuildCommand`, which is how + // the frontend can tell a debug package from a release one. `import.meta.env.DEV` + // cannot: it is true only under the dev server, so both APKs look identical to + // it and the debug build loses its logging (see logger.ts). + define: { + __JT_DEBUG_BUILD__: JSON.stringify(process.env.TAURI_ENV_DEBUG === "true"), + }, + // Vite options tailored for Tauri development and only applied in `tauri dev` or `tauri build` // // 1. prevent Vite from obscuring rust errors