[MEDIUM] Add vitest suite for createCoreFetch + createI18n; wire into prepare

uwe.admin commented

2026-04-25 13:39:55 +02:00

Owner

From audit #1.

Problem

No tests at all in webapp-scaffold — the largest risk multiplier on every other audit finding.

Remediation

Add vitest suite covering:

createCoreFetch: each hook, each response shape, 401/409/network paths.
createI18n: resolver chain, miss cache, subscribe/notify.

Wire npm test into the prepare script so publish is gated on tests passing.

Acceptance

npm test runs on prepare.
Coverage includes both createCoreFetch hook variants and createI18n happy + miss paths.

Effort: medium.

From audit #1. ## Problem No tests at all in `webapp-scaffold` — the largest risk multiplier on every other audit finding. ## Remediation Add vitest suite covering: - `createCoreFetch`: each hook, each response shape, 401/409/network paths. - `createI18n`: resolver chain, miss cache, subscribe/notify. Wire `npm test` into the `prepare` script so publish is gated on tests passing. ## Acceptance - `npm test` runs on `prepare`. - Coverage includes both `createCoreFetch` hook variants and `createI18n` happy + miss paths. Effort: medium.

uwe.admin added the

new

label 2026-04-25 13:39:55 +02:00

uwe.admin referenced this issue

2026-04-25 13:40:05 +02:00

Code Audit Report — 2026-04-23 #1

uwe.admin referenced this issue

2026-04-25 13:42:41 +02:00

[MEDIUM] fetch-openapi.sh: pass $OUT via sys.argv, not f-string in heredoc #2

uwe.admin commented

2026-04-25 13:46:08 +02:00

Author

Owner

Agent Evaluation

Feasibility: Medium. webapp-scaffold currently has zero test infrastructure — no vitest, jsdom, or test setup. Bootstrapping it means adding vitest, @vitest/ui (optional), jsdom, and possibly msw for HTTP fixtures, plus a minimal vitest.config.ts. The library has three substantive units to cover:

core-fetch.ts (~180 LOC) — createCoreFetch factory, hook wiring (onUnauthorized, onConflict, response-shape branches, network-error path).
i18n.ts (~80 LOC) — resolver chain, miss-cache, subscribe/notify.
i18n-react.ts — small wrapper, can piggy-back on i18n.ts tests.

Surface area is small but the test count to be "useful" is meaningful — easily 25–35 cases.

Impact: Direct risk-reduction multiplier on every other audit finding. Once the harness exists, future fixes (#2, #3, #5) ship with regression guards instead of "manual smoke-test on the consumer".

Effort: Medium.

Recommendation: Accept.

Implementation plan

Bootstrap vitest — add vitest, jsdom (for i18n-react.ts's React deps) to devDependencies. Minimal vitest.config.ts setting environment: 'jsdom'.
src/core-fetch.test.ts — mock globalThis.fetch (vitest's vi.fn); test:
- happy path returns body for responseShape: 'body' and {response, body} for 'full'.
- 401 → onUnauthorized invoked with the Response; promise rejects (per #1's documentation note).
- 409 → onConflict invoked, error includes the parsed body.
- network error → rejection bubbles, no hook spuriously fires.
- non-JSON 2xx → currently silently returns text; test pins that behaviour and is updated when #5's LOW-finding fix lands.
src/i18n.test.ts — synchronous resolver chain (lookup hits first dict), miss path returns key + caches, subscribe/notify fires once per setLocale.
src/i18n-react.test.tsx — @testing-library/react (add to devDeps), one test that the hook re-renders on locale change.
package.json — add "test": "vitest run", "test:watch": "vitest". Wire into prepare: "prepare": "tsc && vitest run" (drop the || true per audit-#1 LOW item).
README — one paragraph: how to run, how to add a test.

Decision needed

Check one (edit this comment):

Option A — Full coverage as specced — all four files, jsdom, react-testing-library; 25–35 cases.
Option B — Minimal coverage on core-fetch.ts only — boot vitest, cover only createCoreFetch (the highest-risk surface); leave i18n for a later issue. Smaller PR, addresses the audit's most concrete "untested code path" finding.

## Agent Evaluation **Feasibility:** Medium. webapp-scaffold currently has zero test infrastructure — no vitest, jsdom, or test setup. Bootstrapping it means adding `vitest`, `@vitest/ui` (optional), `jsdom`, and possibly `msw` for HTTP fixtures, plus a minimal `vitest.config.ts`. The library has three substantive units to cover: - `core-fetch.ts` (~180 LOC) — `createCoreFetch` factory, hook wiring (`onUnauthorized`, `onConflict`, response-shape branches, network-error path). - `i18n.ts` (~80 LOC) — resolver chain, miss-cache, subscribe/notify. - `i18n-react.ts` — small wrapper, can piggy-back on `i18n.ts` tests. Surface area is small but the test count to be "useful" is meaningful — easily 25–35 cases. **Impact:** Direct risk-reduction multiplier on every other audit finding. Once the harness exists, future fixes (#2, #3, #5) ship with regression guards instead of "manual smoke-test on the consumer". **Effort:** Medium. **Recommendation:** Accept. ### Implementation plan 1. **Bootstrap vitest** — add `vitest`, `jsdom` (for `i18n-react.ts`'s React deps) to devDependencies. Minimal `vitest.config.ts` setting `environment: 'jsdom'`. 2. **`src/core-fetch.test.ts`** — mock `globalThis.fetch` (vitest's `vi.fn`); test: - happy path returns body for `responseShape: 'body'` and `{response, body}` for `'full'`. - 401 → `onUnauthorized` invoked with the Response; promise rejects (per #1's documentation note). - 409 → `onConflict` invoked, error includes the parsed body. - network error → rejection bubbles, no hook spuriously fires. - non-JSON 2xx → currently silently returns text; test pins that behaviour and is updated when #5's LOW-finding fix lands. 3. **`src/i18n.test.ts`** — synchronous resolver chain (lookup hits first dict), miss path returns key + caches, subscribe/notify fires once per `setLocale`. 4. **`src/i18n-react.test.tsx`** — `@testing-library/react` (add to devDeps), one test that the hook re-renders on locale change. 5. **`package.json`** — add `"test": "vitest run"`, `"test:watch": "vitest"`. Wire into `prepare`: `"prepare": "tsc && vitest run"` (drop the `|| true` per audit-#1 LOW item). 6. **README** — one paragraph: how to run, how to add a test. ### Decision needed Check one (edit this comment): - [ ] **Option A — Full coverage as specced** — all four files, jsdom, react-testing-library; 25–35 cases. - [ ] **Option B — Minimal coverage on `core-fetch.ts` only** — boot vitest, cover only `createCoreFetch` (the highest-risk surface); leave i18n for a later issue. Smaller PR, addresses the audit's most concrete "untested code path" finding.

uwe.admin added the

effort:medium

evaluated

labels 2026-04-25 13:46:14 +02:00

uwe.admin commented

2026-04-25 13:46:14 +02:00

Author

Owner

Evaluated #4 — Medium, recommend accept; decision: full coverage vs core-fetch-only.

u.schuster commented

2026-04-25 21:25:30 +02:00

Implement Option A

uwe.admin commented

2026-04-25 21:27:50 +02:00

Author

Owner

Re-evaluation

Owner selected Option A — Full coverage as specced.

Acknowledged. Implementation will follow the original plan verbatim:

Bootstrap vitest + jsdom (+ @testing-library/react for i18n-react.test.tsx).
src/core-fetch.test.ts — happy path (both response shapes), 401 → onUnauthorized, 409 → onConflict, network error, non-JSON 2xx pin.
src/i18n.test.ts — resolver chain, miss-cache, subscribe/notify.
src/i18n-react.test.tsx — re-render on locale change.
package.json — add test / test:watch; wire prepare = tsc && vitest run (drop || true).
README — short "running tests" paragraph.

Apply the accepted label to trigger implementation.

## Re-evaluation Owner selected **Option A — Full coverage as specced**. **Acknowledged.** Implementation will follow the original plan verbatim: 1. Bootstrap vitest + jsdom (+ `@testing-library/react` for `i18n-react.test.tsx`). 2. `src/core-fetch.test.ts` — happy path (both response shapes), 401 → `onUnauthorized`, 409 → `onConflict`, network error, non-JSON 2xx pin. 3. `src/i18n.test.ts` — resolver chain, miss-cache, subscribe/notify. 4. `src/i18n-react.test.tsx` — re-render on locale change. 5. `package.json` — add `test` / `test:watch`; wire `prepare = tsc && vitest run` (drop `|| true`). 6. README — short "running tests" paragraph. Apply the `accepted` label to trigger implementation.

uwe.admin commented

2026-04-25 21:27:53 +02:00

Author

Owner

Re-evaluated #4 — owner picked Option A (full coverage); awaiting accepted label.

uwe.admin added the

accepted

label 2026-04-25 21:32:07 +02:00

uwe.admin referenced this issue from a commit

2026-04-25 21:45:32 +02:00

#4: vitest harness + suites for createCoreFetch / createI18n / useI18nStore

uwe.admin closed this issue

2026-04-25 21:45:32 +02:00

uwe.admin commented

2026-04-25 21:45:37 +02:00

Author

Owner

Implemented #4 → commit 90c5ca2, version 0.3.7 (Option A: full coverage; 25 tests, vitest+jsdom+@testing-library/react, prepare gated on tests passing).

Implemented #4 → commit 90c5ca2, version 0.3.7 (Option A: full coverage; 25 tests, vitest+jsdom+@testing-library/react, prepare gated on tests passing).

uwe.admin referenced this issue

2026-04-25 22:24:11 +02:00

Code Audit Report — 2026-04-23 #1

Rows
Columns

[MEDIUM] Add vitest suite for createCoreFetch + createI18n; wire into prepare #4

Problem

Remediation

Acceptance

Agent Evaluation

Implementation plan

Decision needed

Re-evaluation