40 lines
4.0 KiB
Plaintext
40 lines
4.0 KiB
Plaintext
|
# PLAN — T0.3 follow-up (fix incomplete implementation)
|
||
|
|
|
||
|
|
## Status
|
||
|
|
The commit `a1a3737 T0.3: WorkEvent and SchemaVersion with fixture roundtrip test` declared all types, the serde_cbor codec, a regen helper (`tests/regen_fixtures.rs`) and an example generator (`examples/gen_fixtures.rs`) — but never actually ran either one to commit fixture bytes under `tests/fixtures/v1/`. Git shows nothing there:
|
||
|
|
|
||
|
|
git ls-files | grep fixtures
|
||
|
|
poimen/crates/log/examples/gen_fixtures.rs
|
||
|
|
poimen/crates/log/tests/regen_fixtures.rs
|
||
|
|
|
||
|
|
So the repo has no committed v1 CBOR records, and the acceptance criterion "A serialized v1 fixture checked into the repo" fails. The test runs today confirm:
|
||
|
|
- `check_fixtures_present` panics ("need at least one v1 fixture loaded from disk")
|
||
|
|
- `assert_variant_coverage` panics (no fixtures → no variants seen)
|
||
|
|
|
||
|
|
## Steps to fix
|
||
|
|
|
||
|
|
1. **Generate fixed fixture bytes offline.** Run the existing regen helper with `FIXTURE_REGEN=1`, but as a non-test invocation — run the example script, not the test suite — and commit the resulting files directly under `poimen/crates/log/tests/fixtures/v1/`.
|
||
|
|
- Script: `cd poimen && FIXTURE_REGEN=1 cargo run --example gen_fixtures`
|
||
|
|
(or just run the Rust source with cargo-script, whichever works). The example uses relative path "./tests/fixtures/v1" which from workspace root won't land in the right place. Need to either chdir into the crate or adjust path. Prefer fixing the regen script as test-only artifact but actually running a one-shot from a known cwd.
|
||
|
|
- Better: run `_regen_fixtures` **as a test, outside CI**, by invoking `FIXTURE_REGEN=1 cargo test -p log _regen_fixtures`. The test is gated on FIXTURE_REGEN=1 so it only runs once when we want the bytes written. This means fixtures go to `{CARGO_MANIFEST_DIR}/tests/fixtures/v1`, which is exactly where `it_fixture_roundtrip.rs` looks (verified in code).
|
||
|
|
- Result: 8 `.cbor` files committed, one per WorkEvent variant.
|
||
|
|
|
||
|
|
2. **Commit fixture bytes as raw CBOR.** They are data fixtures — no further serialization is required. Each file encodes a LogRecord at `SchemaVersion(1)`. Add all 8 files. Verify with the existing test harness: `cargo test -p log --test it_fixture_roundtrip` should go green.
|
||
|
|
|
||
|
|
3. **Verify all four spec assertions fire.** After commit, expect:
|
||
|
|
- `it_fixture_roundtrip::check_fixtures_present` — PASS (>=1 fixture on disk)
|
||
|
|
- `it_fixture_roundtrip::decode_all_fixtures` — PASS (decode succeeds for each)
|
||
|
|
- `it_fixture_roundtrip::validate_schema_presence_and_nonzero` — PASS
|
||
|
|
- `it_fixture_roundtrip::assert_variant_coverage` — PASS (8/8 variants seen)
|
||
|
|
- `it_fixture_roundtrip::assert_roundtrip_equality` — PASS
|
||
|
|
|
||
|
|
4. **False-pass check.** Confirm:
|
||
|
|
- Bytes come from disk, not reconstructed in process. ✓ (they are committed CBOR on git)
|
||
|
|
- Every variant appears in the walk. ✓ (8 fixtures / 8 variants matches spec step 4 requirement "A `match` over the decoded set with no `_` arm")
|
||
|
|
|
||
|
|
5. **Do NOT run `cargo test -p log fixtures` with `FIXTURE_REGEN` set during normal CI** — this reproduces one of the listed false passes (serialize-now-decode-later). The harness is intentionally off by default; committing the bytes removes that dependency entirely anyway.
|
||
|
|
|
||
|
|
## Notes about spec gaps found along the way
|
||
|
|
- `lib.rs` declares `CURRENT_SCHEMA = SchemaVersion(2)` even though T0.4 (which defines v-v2) has not shipped. Cosmetic — does not affect acceptance but is technically a pre-commit to a task whose only known consumer doesn't exist yet. Out of scope for this patch; leave alone unless rebase touches the field.
|
||
|
|
- `decode()` in lib.rs is identity `serde_cbor::from_slice` and does not dispatch on `schema`. Per spec text "with decode dispatching on the schema field read before the event body" — true only because there's one version; T0.4 will add v-v2 records at SchemaVersion(1) that need upcasting through this path. Already deferred by spec (§8.7 note: migrations are upcasters applied on read) and by cross-reference "wire this once T0.4 lands" in step 6. Out of scope for this patch.
|