Skip to content

Commit efaa30d

Browse files
KubrickCodeKubrickCode
committed
docs: add ADR-10 TestStatus data contract
Document cross-service TestStatus enum alignment decision across core, collector, web. Collector previously defined only 3 statuses causing focused→active, xfail→todo data loss. Enforced 1:1 mapping of all 5 status values to ensure data integrity.
1 parent 19371ae commit efaa30d

File tree

7 files changed

+503
-0
lines changed

7 files changed

+503
-0
lines changed

‎docs/.vitepress/config.mts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,10 @@ export default defineConfig({
8080
text: "GitHub App Integration",
8181
link: "/en/adr/09-github-app-integration",
8282
},
83+
{
84+
text: "TestStatus Data Contract",
85+
link: "/en/adr/10-test-status-data-contract",
86+
},
8387
{
8488
text: "Core",
8589
collapsed: true,
@@ -194,6 +198,7 @@ export default defineConfig({
194198
{ text: "공유 인프라", link: "/ko/adr/07-shared-infrastructure" },
195199
{ text: "External Repo ID 무결성", link: "/ko/adr/08-external-repo-id-integrity" },
196200
{ text: "GitHub App 통합", link: "/ko/adr/09-github-app-integration" },
201+
{ text: "TestStatus 데이터 계약", link: "/ko/adr/10-test-status-data-contract" },
197202
{
198203
text: "Core",
199204
collapsed: true,

‎docs/en/adr/10-test-status-data-contract.md‎

Lines changed: 247 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,247 @@
1+
---
2+
title: TestStatus Data Contract
3+
description: ADR on cross-service TestStatus enum alignment for data integrity
4+
---
5+
6+
# ADR-10: TestStatus Data Contract
7+
8+
> 🇰🇷 [한국어 버전](/ko/adr/10-test-status-data-contract)
9+
10+
| Date | Author | Repos |
11+
| ---------- | ------------ | -------------------- |
12+
| 2024-12-29 | @KubrickCode | core, collector, web |
13+
14+
## Context
15+
16+
### The Data Flow Problem
17+
18+
Specvital processes test metadata through a multi-service pipeline:
19+
20+
```
21+
Core Parser → Collector → Database → Web API → Frontend
22+
```
23+
24+
Each service defines its own `TestStatus` type, creating potential for:
25+
26+
- **Data loss**: Status values dropped during transformation
27+
- **Semantic drift**: Same enum value meaning different things
28+
- **Silent failures**: Mapping errors going unnoticed
29+
30+
### The Incident
31+
32+
During development, a critical data integrity issue was discovered:
33+
34+
```
35+
Core defines: active, focused, skipped, todo, xfail (5 statuses)
36+
Collector had: active, skipped, todo (3 statuses)
37+
```
38+
39+
**Impact**:
40+
41+
- `focused` tests were incorrectly mapped to `active`
42+
- `xfail` tests were incorrectly mapped to `todo`
43+
- Users saw inaccurate test counts and missing status indicators
44+
45+
### Why This Matters
46+
47+
| Status | Semantic Meaning | User Impact if Lost |
48+
| ------- | ------------------------------- | ------------------------------ |
49+
| active | Normal test, will run | Baseline, no impact |
50+
| focused | Debug-only test (.only, fit) | CI warning not triggered |
51+
| skipped | Intentionally excluded | Wrong skip count |
52+
| todo | Placeholder, not implemented | Missing TODO tracking |
53+
| xfail | Expected to fail (pytest xfail) | Incorrect failure expectations |
54+
55+
## Decision
56+
57+
**Enforce 1:1 TestStatus mapping across all services with no lossy transformations.**
58+
59+
### Canonical Status Definition
60+
61+
All services MUST support exactly these 5 statuses:
62+
63+
```go
64+
// Canonical TestStatus enum (source of truth: core)
65+
type TestStatus string
66+
67+
const (
68+
TestStatusActive TestStatus = "active" // Normal test
69+
TestStatusFocused TestStatus = "focused" // .only, fit - debug mode
70+
TestStatusSkipped TestStatus = "skipped" // .skip, xit - excluded
71+
TestStatusTodo TestStatus = "todo" // Placeholder test
72+
TestStatusXfail TestStatus = "xfail" // Expected failure
73+
)
74+
```
75+
76+
### Service Alignment
77+
78+
| Service | Location | Status |
79+
| --------- | --------------------------------------- | --------------------- |
80+
| Core | `pkg/domain/status.go` | Source of truth |
81+
| Collector | `internal/domain/analysis/inventory.go` | 1:1 mapping from Core |
82+
| Database | `test_status` ENUM in schema.sql | 1:1 mapping |
83+
| Web API | OpenAPI `TestStatus` schema | 1:1 mapping |
84+
85+
## Options Considered
86+
87+
### Option A: String Pass-Through (Rejected)
88+
89+
- Pass status as raw string without enum validation
90+
- **Rejected**: No compile-time safety, typos cause silent failures
91+
92+
### Option B: Subset Mapping (Previous State)
93+
94+
- Collector uses simplified 3-status model
95+
- Map `focused → active`, `xfail → todo`
96+
- **Rejected**: Data loss, semantic corruption
97+
98+
### Option C: Strict 1:1 Mapping (Selected)
99+
100+
- Every service defines identical enum values
101+
- Explicit switch statement with all cases
102+
- Unknown values panic/error (fail-fast)
103+
104+
## Implementation
105+
106+
### Core (Source of Truth)
107+
108+
```go
109+
// pkg/domain/status.go
110+
type TestStatus string
111+
112+
const (
113+
TestStatusActive TestStatus = "active"
114+
TestStatusSkipped TestStatus = "skipped"
115+
TestStatusTodo TestStatus = "todo"
116+
TestStatusFocused TestStatus = "focused"
117+
TestStatusXfail TestStatus = "xfail"
118+
)
119+
```
120+
121+
### Collector (Consumer)
122+
123+
```go
124+
// internal/domain/analysis/inventory.go
125+
type TestStatus string
126+
127+
const (
128+
TestStatusActive TestStatus = "active"
129+
TestStatusFocused TestStatus = "focused"
130+
TestStatusSkipped TestStatus = "skipped"
131+
TestStatusTodo TestStatus = "todo"
132+
TestStatusXfail TestStatus = "xfail"
133+
)
134+
```
135+
136+
### Mapping Layer
137+
138+
```go
139+
// internal/adapter/mapping/core_domain.go
140+
func convertCoreTestStatus(coreStatus domain.TestStatus) analysis.TestStatus {
141+
switch coreStatus {
142+
case domain.TestStatusFocused:
143+
return analysis.TestStatusFocused
144+
case domain.TestStatusSkipped:
145+
return analysis.TestStatusSkipped
146+
case domain.TestStatusTodo:
147+
return analysis.TestStatusTodo
148+
case domain.TestStatusXfail:
149+
return analysis.TestStatusXfail
150+
default:
151+
return analysis.TestStatusActive
152+
}
153+
}
154+
```
155+
156+
### Database Schema
157+
158+
```sql
159+
CREATE TYPE public.test_status AS ENUM (
160+
'active',
161+
'skipped',
162+
'todo',
163+
'focused',
164+
'xfail'
165+
);
166+
```
167+
168+
### Web API (OpenAPI)
169+
170+
```yaml
171+
TestStatus:
172+
type: string
173+
enum:
174+
- active
175+
- focused
176+
- skipped
177+
- todo
178+
- xfail
179+
description: |
180+
Test status indicator:
181+
- active: Normal test that will run
182+
- focused: Test marked to run exclusively (e.g., it.only)
183+
- skipped: Test marked to be skipped (e.g., it.skip)
184+
- todo: Placeholder test to be implemented
185+
- xfail: Expected to fail (pytest xfail)
186+
```
187+
188+
## Consequences
189+
190+
### Positive
191+
192+
**Data Integrity**:
193+
194+
- No information loss in the pipeline
195+
- Accurate test counts for all status types
196+
- Reliable CI warnings for focused tests
197+
198+
**Type Safety**:
199+
200+
- Compile-time validation with typed enums
201+
- Explicit mapping prevents silent failures
202+
- IDE autocomplete for status values
203+
204+
**API Clarity**:
205+
206+
- Frontend receives accurate status information
207+
- Consistent behavior across all endpoints
208+
- Self-documenting enum values
209+
210+
### Negative
211+
212+
**Coordination Overhead**:
213+
214+
- Adding new status requires changes in all 4 locations:
215+
- Core: `pkg/domain/status.go`
216+
- Collector: `internal/domain/analysis/inventory.go`
217+
- Database: Migration for ENUM alteration
218+
- Web: OpenAPI schema update
219+
- Risk of version mismatch during deployment
220+
221+
**Schema Evolution**:
222+
223+
- PostgreSQL ENUM alteration requires migration
224+
- Cannot easily remove status values (only deprecate)
225+
- Order of values in ENUM affects storage
226+
227+
## Guidelines for Future Changes
228+
229+
### Adding a New Status
230+
231+
1. Add to Core `pkg/domain/status.go` first
232+
2. Add to Collector domain and mapping layer
233+
3. Create database migration for ENUM addition
234+
4. Update OpenAPI schema
235+
5. Deploy in order: Database → Collector → Web → Core
236+
237+
### Deprecating a Status
238+
239+
1. Mark as deprecated in documentation
240+
2. Map deprecated status to replacement in Collector
241+
3. Update parsers to stop emitting deprecated status
242+
4. Remove from OpenAPI after migration period
243+
244+
## References
245+
246+
- [PostgreSQL ENUM Types](https://www.postgresql.org/docs/current/datatype-enum.html)
247+
- [OpenAPI Enum Best Practices](https://swagger.io/docs/specification/data-models/enums/)

‎docs/en/adr/index.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ XX-brief-decision-title.md
6666
| 07 | [Shared Infrastructure Strategy](./07-shared-infrastructure.md) | Infrastructure | 2024-12-17 |
6767
| 08 | [External Repository ID-Based Data Integrity](./08-external-repo-id-integrity.md) | Data Integrity | 2024-12-22 |
6868
| 09 | [GitHub App Integration Strategy](./09-github-app-integration.md) | Authentication | 2024-12-29 |
69+
| 10 | [TestStatus Data Contract](./10-test-status-data-contract.md) | Data Integrity | 2024-12-29 |
6970

7071
### Core Repository
7172

‎docs/en/index.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ Documentation of architectural decisions made during the development of Specvita
3939
- [Shared Infrastructure](./adr/07-shared-infrastructure.md)
4040
- [External Repo ID Integrity](./adr/08-external-repo-id-integrity.md)
4141
- [GitHub App Integration](./adr/09-github-app-integration.md)
42+
- [TestStatus Data Contract](./adr/10-test-status-data-contract.md)
4243

4344
**[Core](./adr/core/)**
4445

0 commit comments

Comments
 (0)