Skip to content

Commit 4a30654

Browse files
authored
Add focused test coverage roadmap (#180)
1 parent f08303c commit 4a30654

5 files changed

Lines changed: 232 additions & 0 deletions

File tree

CONTRIBUTING.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ Dependencies are managed through normal Go module resolution. Review dependency
2525

2626
DB-backed integration tests skip when no Postgres connection is configured. To run them, provide libpq environment variables or `READINESS_TEST_DSN`. Tests create throwaway schemas and drop them during cleanup.
2727

28+
See `docs/TEST_COVERAGE.md` for the current focused coverage roadmap and fixture rules.
29+
2830
### CI and race detector policy
2931

3032
The default required-looking CI job is `Build, Vet & Test`. Keep that job name stable unless repository branch protection is updated at the same time.

docs/TEST_COVERAGE.md

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# Focused Test Coverage Roadmap
2+
3+
This roadmap tracks high-value Health Dashboard test coverage without turning the suite into a slow infrastructure exercise.
4+
5+
## Constraints
6+
7+
- Default CI must stay pure and fast under `go test ./...`.
8+
- Fixtures must be synthetic or anonymized. Do not commit personal health exports, screenshots, logs, or raw metric dumps.
9+
- DB-backed tests should skip cleanly unless a Postgres connection is configured.
10+
- Coverage thresholds are intentionally deferred until the active product contracts have a useful baseline.
11+
12+
## Current Coverage Map
13+
14+
| Area | Current state | Next useful coverage |
15+
|---|---|---|
16+
| `internal/health` formulas | Strong coverage for readiness, evidence payloads, EnergyBank v2, stress, i18n labels, and anomaly helpers. | Add tests only when formula or evidence contracts change. |
17+
| Apple Health import | Synthetic XML fixtures cover mapped records, empty input, malformed input, sleep-stage mapping, and focused parser edge cases. | Add fixtures for real bug classes after reducing them to minimal synthetic XML. |
18+
| UI and admin APIs | Contract tests cover admin pages, auth/session behavior, webhook dispatch, dashboard sections, and tenant scope. | Pin response-shape changes before frontend code starts depending on them. |
19+
| Storage writers | Readiness redesign, EnergyBank, sleep gates, freshness, calibration, and tenant helpers have focused tests, with DB-backed tests gated. | Prefer bug-driven storage contract tests over broad repository-level sweeps. |
20+
| Notifications | Morning/evening report, freshness banners, smart retry, Telegram webhook, and proactive framework tests cover key behavior. | Add regression tests when notification timing or skip conditions change. |
21+
| CI policy | Default CI runs build, vet, and pure tests; race detector is manual and documented. | Keep race detector separate from default PR checks unless the runtime cost becomes acceptable. |
22+
23+
## Delivered First Batch
24+
25+
Issue #149 starts with a small Apple Health import safety batch:
26+
27+
- `internal/applehealth/testdata/focused_edge_export.xml` is a synthetic fixture with no personal data.
28+
- `TestParseXMLFocusedEdgeFixturePinsImportSafety` checks percent normalization boundaries, duration-derived category metrics, stand-hour mapping, unknown quantity fallback, invalid duration rejection, and unsupported correlation skipping.
29+
30+
## Candidate Follow-Ups
31+
32+
- Dashboard API response stability tests for fields consumed by `static/app.js` and `static/charts.js`.
33+
- Import ZIP wrapper tests around missing `export.xml`, progress status, and parse error propagation.
34+
- Storage contract tests for freshness/readiness behavior when a bug or product change proves the need.
35+
- One informational `go test -cover` baseline after the focused lanes are less sparse, without enforcing a threshold.
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
<!doctype html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="utf-8">
5+
<title>Implementation Plan: Issue #149 Focused Test Coverage Roadmap</title>
6+
<meta name="viewport" content="width=device-width, initial-scale=1">
7+
<style>
8+
body { margin: 0; font-family: system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", sans-serif; background: #f6f7f9; color: #1f2937; line-height: 1.5; }
9+
main { max-width: 1100px; margin: 0 auto; padding: 32px; }
10+
section { background: #fff; border: 1px solid #e5e7eb; border-radius: 8px; padding: 20px; margin: 16px 0; }
11+
h1, h2 { line-height: 1.2; margin-top: 0; }
12+
code { background: #f3f4f6; padding: 2px 5px; border-radius: 4px; }
13+
.badge { display: inline-block; padding: 4px 10px; border-radius: 999px; background: #dcfce7; color: #166534; font-size: 12px; font-weight: 700; }
14+
.risk { border-left: 4px solid #dc2626; }
15+
.approval { border-left: 4px solid #d97706; }
16+
</style>
17+
</head>
18+
<body>
19+
<main>
20+
<h1>Implementation Plan: Issue #149 Focused Test Coverage Roadmap</h1>
21+
<p class="badge">Approved and implemented</p>
22+
23+
<section>
24+
<h2>Task Summary</h2>
25+
<p>Create a current, focused test coverage roadmap for active Health Dashboard behavior. This should update the issue/doc roadmap and add only high-value missing tests, not introduce broad coverage thresholds or slow infrastructure.</p>
26+
</section>
27+
28+
<section>
29+
<h2>Current Behavior</h2>
30+
<ul>
31+
<li>The repository currently has 78 Go test files under <code>internal</code> and related packages.</li>
32+
<li>Existing tests already cover major active areas: health formulas, readiness evidence, EnergyBank v2, stress validation, Apple Health XML parser fixtures, UI admin contracts, tenant scope, webhook dispatch, and storage writers.</li>
33+
<li><code>internal/applehealth/testdata</code> already has synthetic, malformed, and empty XML fixtures.</li>
34+
<li><code>CONTRIBUTING.md</code> states DB-backed tests skip when no Postgres connection is configured and warns against real personal health data in fixtures.</li>
35+
<li>CI currently runs the full pure test suite under <code>go test ./...</code>.</li>
36+
</ul>
37+
</section>
38+
39+
<section>
40+
<h2>Desired Behavior</h2>
41+
<ul>
42+
<li>The repo has a short, current roadmap that identifies remaining high-value test gaps instead of implying the test suite is empty.</li>
43+
<li>New tests target product contracts that change often: readiness freshness, EnergyBank v1/v2 compatibility boundaries, import conflict resolution, and API response stability.</li>
44+
<li>All tests added in this lane run under the existing default <code>go test ./...</code> path unless explicitly DB-gated and justified.</li>
45+
<li>No real personal health data is committed.</li>
46+
</ul>
47+
</section>
48+
49+
<section>
50+
<h2>Assumptions And Unknowns</h2>
51+
<ul>
52+
<li>Assumption: this issue is best handled as a roadmap plus a small first batch of tests, not a large test sweep.</li>
53+
<li>Assumption: issue #166 should own readiness freshness contract tests if implemented first.</li>
54+
<li>Unknown: whether the user wants child GitHub issues created for each coverage lane or a single living doc section.</li>
55+
<li>Unknown: whether coverage measurement should be introduced now; the issue says threshold should wait until there is a meaningful baseline.</li>
56+
</ul>
57+
</section>
58+
59+
<section>
60+
<h2>Files Likely To Change</h2>
61+
<ul>
62+
<li><code>docs/ARCHITECTURE.md</code>, <code>README.md</code>, or <code>CONTRIBUTING.md</code> - add the current test coverage roadmap.</li>
63+
<li><code>internal/health/*_test.go</code> - add focused invariant tests only where gaps remain.</li>
64+
<li><code>internal/applehealth/testdata/*</code> and <code>internal/applehealth/parse_test.go</code> - add a small anonymized fixture only if current fixtures do not cover a real parser risk.</li>
65+
<li><code>internal/ui/*_test.go</code> - add API response contract tests if readiness/AI/dashboard shapes are changed.</li>
66+
<li>GitHub issue comments or child issues - optional, if the roadmap should live in GitHub rather than docs.</li>
67+
</ul>
68+
</section>
69+
70+
<section>
71+
<h2>Implementation Steps</h2>
72+
<ol>
73+
<li>Produce a compact test inventory grouped by package and behavior.</li>
74+
<li>Map the issue's candidate lanes to current coverage and mark each as covered, partial, or missing.</li>
75+
<li>Choose a first small batch of tests that protects active product behavior without DB/network requirements.</li>
76+
<li>Add or update docs with the roadmap and "do not commit personal health data" fixture rule.</li>
77+
<li>If GitHub child issues are desired, create them after user approval with clear scopes and dependencies.</li>
78+
<li>Run the default test suite and vet.</li>
79+
</ol>
80+
</section>
81+
82+
<section>
83+
<h2>Impact</h2>
84+
<ul>
85+
<li><strong>CI:</strong> should remain fast; no coverage threshold or race job in this issue.</li>
86+
<li><strong>Data:</strong> no production DB changes; fixtures must stay synthetic/anonymized.</li>
87+
<li><strong>Product behavior:</strong> tests should pin existing behavior unless paired with an approved product change.</li>
88+
<li><strong>GitHub process:</strong> optional child issues can make the roadmap easier to execute incrementally.</li>
89+
</ul>
90+
</section>
91+
92+
<section>
93+
<h2>Test Plan</h2>
94+
<ul>
95+
<li><code>go test ./internal/health ./internal/applehealth ./internal/ui ./internal/storage -count=1</code></li>
96+
<li><code>go test ./...</code></li>
97+
<li><code>go vet ./...</code></li>
98+
<li>If fixtures are added: manually inspect fixture content for personal data before commit.</li>
99+
</ul>
100+
</section>
101+
102+
<section class="risk">
103+
<h2>Risks And Edge Cases</h2>
104+
<ul>
105+
<li>A generic coverage push can create brittle tests that slow useful development.</li>
106+
<li>DB-backed tests can become flaky if they assume a developer has a local database; keep them opt-in unless necessary.</li>
107+
<li>Fixtures can accidentally leak personal data; prefer minimal synthetic XML.</li>
108+
<li>This issue overlaps with #166 and #152; keep readiness contract tests and race policy in their respective issues where possible.</li>
109+
</ul>
110+
</section>
111+
112+
<section>
113+
<h2>Rollback Plan</h2>
114+
<p>Revert roadmap docs and any added tests. If child issues are created incorrectly, close or relabel them rather than deleting historical context.</p>
115+
</section>
116+
117+
<section>
118+
<h2>Open Questions</h2>
119+
<ul>
120+
<li>Should the roadmap live in docs, GitHub child issues, or both?</li>
121+
<li>Which first coverage batch is most valuable after #166: import safety, EnergyBank compatibility, or dashboard API stability?</li>
122+
<li>Should coverage metrics be measured once as an informational baseline, without enforcing a threshold?</li>
123+
</ul>
124+
</section>
125+
126+
<section class="approval">
127+
<h2>Approval Gate</h2>
128+
<p>Approved by the user on 2026-06-06 before production code changes.</p>
129+
</section>
130+
131+
<section>
132+
<h2>Implementation Result</h2>
133+
<ul>
134+
<li>Added <code>docs/TEST_COVERAGE.md</code> as the living focused coverage roadmap.</li>
135+
<li>Linked the roadmap from <code>CONTRIBUTING.md</code>.</li>
136+
<li>Added a synthetic Apple Health edge fixture at <code>internal/applehealth/testdata/focused_edge_export.xml</code>.</li>
137+
<li>Added <code>TestParseXMLFocusedEdgeFixturePinsImportSafety</code> to pin percent normalization boundaries, duration-derived category metrics, stand-hour mapping, unknown quantity fallback, invalid duration rejection, and unsupported correlation skipping.</li>
138+
<li>No real personal health data was added.</li>
139+
</ul>
140+
</section>
141+
142+
<section>
143+
<h2>Verification Run</h2>
144+
<ul>
145+
<li><code>go test ./internal/health ./internal/applehealth ./internal/ui ./internal/storage -count=1</code></li>
146+
<li><code>go test ./...</code></li>
147+
<li><code>go vet ./...</code></li>
148+
<li><code>git diff --check</code></li>
149+
</ul>
150+
</section>
151+
152+
<section>
153+
<h2>Known Limitations And Follow-Up</h2>
154+
<ul>
155+
<li>This PR intentionally does not add a coverage threshold.</li>
156+
<li>DB-backed coverage remains opt-in unless a product bug requires a focused storage contract test.</li>
157+
<li>Candidate follow-ups are tracked in <code>docs/TEST_COVERAGE.md</code> rather than created as child issues in this first pass.</li>
158+
</ul>
159+
</section>
160+
</main>
161+
</body>
162+
</html>

internal/applehealth/parse_test.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,21 @@ func TestParseXMLSyntheticFixtureMapsHealthKitRecords(t *testing.T) {
2626
assertNoPoint(t, points, "sleep_total", "2026-01-02 02:00:00 +0000")
2727
}
2828

29+
func TestParseXMLFocusedEdgeFixturePinsImportSafety(t *testing.T) {
30+
points := collectXMLFixturePoints(t, "testdata/focused_edge_export.xml")
31+
32+
if got, want := len(points), 4; got != want {
33+
t.Fatalf("point count = %d, want %d: %+v", got, want, points)
34+
}
35+
assertPoint(t, points, "blood_oxygen_saturation", "%", "2026-02-01 06:00:00 +0100", "Synthetic Watch", 98)
36+
assertPoint(t, points, "mindful_minutes", "min", "2026-02-01 09:00:00 +0100", "Synthetic Phone", 15)
37+
assertPoint(t, points, "apple_stand_hour", "count", "2026-02-01 10:00:00 +0100", "Synthetic Watch", 1)
38+
assertPoint(t, points, "some_new_metric", "count", "2026-02-01 13:00:00 +0100", "Synthetic Device", 12.5)
39+
40+
assertMetricCount(t, points, "mindful_minutes", 1)
41+
assertNoMetric(t, points, "blood_pressure")
42+
}
43+
2944
func TestParseXMLEmptyAndMalformedInputs(t *testing.T) {
3045
var emitted bool
3146
if err := parseXMLFixture("testdata/empty_export.xml", func(points []storage.MetricPoint) {
@@ -87,6 +102,15 @@ func assertNoPoint(t *testing.T, points []storage.MetricPoint, metric, date stri
87102
}
88103
}
89104

105+
func assertNoMetric(t *testing.T, points []storage.MetricPoint, metric string) {
106+
t.Helper()
107+
for _, p := range points {
108+
if p.MetricName == metric {
109+
t.Fatalf("unexpected metric=%s point: %+v", metric, p)
110+
}
111+
}
112+
}
113+
90114
func assertMetricCount(t *testing.T, points []storage.MetricPoint, metric string, want int) {
91115
t.Helper()
92116
got := 0
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<HealthData locale="en_US">
3+
<Record type="HKQuantityTypeIdentifierOxygenSaturation" sourceName="Synthetic Watch" unit="%" creationDate="2026-02-01 06:00:00 +0100" startDate="2026-02-01 06:00:00 +0100" endDate="2026-02-01 06:01:00 +0100" value="98"/>
4+
<Record type="HKCategoryTypeIdentifierMindfulSession" sourceName="Synthetic Phone" creationDate="2026-02-01 09:15:00 +0100" startDate="2026-02-01 09:00:00 +0100" endDate="2026-02-01 09:15:00 +0100" value="HKCategoryValueNotApplicable"/>
5+
<Record type="HKCategoryTypeIdentifierAppleStandHour" sourceName="Synthetic Watch" creationDate="2026-02-01 10:00:00 +0100" startDate="2026-02-01 10:00:00 +0100" endDate="2026-02-01 11:00:00 +0100" value="HKCategoryValueAppleStandHourStood"/>
6+
<Record type="HKCategoryTypeIdentifierMindfulSession" sourceName="Synthetic Phone" creationDate="2026-02-01 11:00:00 +0100" startDate="2026-02-01 11:00:00 +0100" endDate="2026-02-01 10:59:00 +0100" value="HKCategoryValueNotApplicable"/>
7+
<Record type="HKCorrelationTypeIdentifierBloodPressure" sourceName="Synthetic Cuff" creationDate="2026-02-01 12:00:00 +0100" startDate="2026-02-01 12:00:00 +0100" endDate="2026-02-01 12:01:00 +0100" value="ignored"/>
8+
<Record type="HKQuantityTypeIdentifierSomeNewMetric" sourceName="Synthetic Device" unit="count" creationDate="2026-02-01 13:00:00 +0100" startDate="2026-02-01 13:00:00 +0100" endDate="2026-02-01 13:01:00 +0100" value="12.5"/>
9+
</HealthData>

0 commit comments

Comments
 (0)