Add tests for coverage gaps S1, S2, S3, S5, S6, S7 (285→346 tests, 88.9%→91.9% lines)
- S1 (error.rs): 5 tests for Display impl on all 4 AlkTypeError variants + Error::source(). error.rs 0%→100%. - S2 (validation.rs, macros.rs): 28 tests for validate() (Result-returning) method on every validator + factory rejection arms for all 12 keywords. validation.rs 85%→97.5% lines / 100% fns; macros.rs 75.7%→93.1% / 100% fns. - S3 (engine.rs): 3 tests for read_field on nested struct leaf fields, Bytes, and Timestamp. engine.rs fns 95%→95.3%. - S5 (layout_builder.rs): 2 tests for union 'variant must be Struct' error (byte + field discriminator). - S6 (schema.rs): 6 tests for as_str() round-trip (all 19 kinds), Display, needs_endian(), is_composite(), get_alktype_kind_enum(). schema.rs 91.2%→97.5% lines / 94.3% fns. - S7 (offset_map.rs): 1 test for nested-struct-without-properties schema error. Updated docs/reviews/001-coverage-analysis.md with resolution section and partial-resolved status. Remaining: S4 (sequential_reader error paths, medium effort) and S8 (overflow guards, recommended to skip for v1).
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
---
|
||||
status: open
|
||||
status: partially-resolved
|
||||
last_updated: 2026-08-02
|
||||
reviewed_artifacts:
|
||||
- src/lib.rs
|
||||
@@ -454,4 +454,87 @@ functions**. The remaining ~5% is the overflow-guard long tail.
|
||||
`data_access.rs` are unreachable by construction (they're match
|
||||
exhaustiveness guards on `AlkTypeError` variants in `assert!`
|
||||
matchers). They show as uncovered but are not test gaps — they're
|
||||
defensive code in test helpers.
|
||||
defensive code in test helpers.
|
||||
|
||||
---
|
||||
|
||||
## Resolution (2026-08-02)
|
||||
|
||||
The high-leverage, low-effort suggestions (S1, S2, S3, S5, S6, S7) were
|
||||
implemented in the same pass. 61 new tests added (285 → 346 passing).
|
||||
Workspace coverage rose from **88.9% → 91.9%** lines (4557/5125 →
|
||||
5118/5568) and **82.3% → 86.3%** functions (433/526 → 498/577). `cargo
|
||||
build`, `cargo test`, and `cargo clippy -- -D warnings` are all green
|
||||
(0 warnings).
|
||||
|
||||
Per-file deltas on the targeted files:
|
||||
|
||||
| File | Before | After |
|
||||
|------|-------:|------:|
|
||||
| src/error.rs | 0.0% | 100.0% |
|
||||
| src/validation.rs | 85.0% | 97.5% |
|
||||
| src/macros.rs | 75.7% | 93.1% |
|
||||
| src/schema.rs | 91.2% | 97.5% |
|
||||
| src/layout_builder.rs | 89.4% | 90.3% |
|
||||
| src/offset_map.rs | 92.6% | 93.3% |
|
||||
|
||||
What landed:
|
||||
|
||||
- **S1 (error.rs Display)**: 5 tests covering `Display` for all four
|
||||
`AlkTypeError` variants (`Schema`, `Offset`, `Access`, `Validation`)
|
||||
and `Error::source()` returning `None`. error.rs is now at 100%.
|
||||
- **S2 (validate() paths)**: 28 tests covering the `validate()` (Result-
|
||||
returning) method of every validator — `Int8/16/32`, `Uint8/16/32`,
|
||||
`Int64`, `Uint64`, `Float32/64`, `Boolean`, `String` (with maxLength
|
||||
exceeded), `Bytes` (with maxLength exceeded), `Enum`, `Timestamp`,
|
||||
`Struct`, `Union`, `Array`, `Record` — plus the factory rejection arms
|
||||
for all 12 keyword factories (rejecting non-true / non-bool-or-object
|
||||
values). validation.rs is now at 97.5% lines / 100% functions; macros.rs
|
||||
at 93.1% lines / 100% functions.
|
||||
- **S3 (engine reads)**: 3 tests covering `read_field` for nested struct
|
||||
leaf fields (`header.magic`, `header.version`), `Bytes` fields, and
|
||||
`Timestamp` fields (read as length-prefixed string). The
|
||||
`AlkTypeKind::Struct` arm in `read_field` remains uncovered — it
|
||||
requires an offset-map entry for a struct path, but the `OffsetMap`
|
||||
only records leaf fields. This is a design characteristic, not a test
|
||||
gap (see Notes below).
|
||||
- **S5 (layout_builder "variant must be Struct")**: 2 tests covering
|
||||
the byte-offset and field-name discriminator union variant-must-be-
|
||||
struct error paths.
|
||||
- **S6 (schema.rs accessors)**: 6 tests covering `AlkTypeKind::as_str()`
|
||||
round-trip for all 19 variants, `Display`, `needs_endian()`,
|
||||
`is_composite()`, and `get_alktype_kind_enum()` (strict bool-form
|
||||
variant). schema.rs is now at 97.5% lines / 94.3% functions. The
|
||||
`needs_endian()` and `is_composite()` methods are kept as public API
|
||||
(they're useful for consumers even though no internal caller uses
|
||||
them currently).
|
||||
- **S7 (offset_map missing-properties)**: 1 test covering the nested-
|
||||
struct-without-properties schema error path.
|
||||
|
||||
Remaining (deferred to follow-up sessions):
|
||||
|
||||
- **S4 (sequential_reader error paths)** — the largest remaining gap
|
||||
(~117 uncovered lines). Medium effort: union error paths, array error
|
||||
paths, discriminator helper Uint16/Uint32 arms, `discriminator_string_value`
|
||||
for U8/U16/U32/Enum, nested-union-as-variant, and the `schema()`
|
||||
accessor. The test patterns exist but need crafted schemas and
|
||||
buffers. Good candidate for a dedicated session.
|
||||
- **S8 (overflow guards)** — ~150 lines of `checked_add` overflow guards
|
||||
across all modules. Recommended to skip for v1 (correct-by-construction
|
||||
format strings, poor effort/value ratio). Revisit if the crate ever
|
||||
handles untrusted offsets from untrusted input.
|
||||
|
||||
### Notes on the `AlkTypeKind::Struct` arm in `engine.rs::read_field`
|
||||
|
||||
The `AlkTypeKind::Struct` arm (engine.rs:279) returns
|
||||
`FieldValue::Struct { start, end }` for a struct field. It's uncovered
|
||||
because the `OffsetMap` records only leaf fields (`header.magic`,
|
||||
`header.version`), not intermediate struct paths (`header`). A
|
||||
`read_field(&buf, "header")` call fails at the offset-map lookup, never
|
||||
reaching the `AlkTypeKind::Struct` arm. This is a design characteristic:
|
||||
the aligned-mode API is designed for reading individual leaf fields at
|
||||
known offsets, not for extracting nested-struct byte ranges (the
|
||||
consumer knows the range from the schema and the offset map's leaf
|
||||
positions). The arm exists for completeness but is unreachable through
|
||||
the current offset-map + schema-tree lookup path. Leaving it uncovered
|
||||
is correct; removing it would be a design decision for a separate pass.
|
||||
Reference in New Issue
Block a user