base-layer/promptkit
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 2 Wiki Activity

Record test coverage and effectiveness audit findings

Browse Source
This commit is contained in:
Eric Rakestraw
2026-11-04 15:11:58 +00:00
parent f7085d76f6
commit 31f1e6a89d
1 changed files with 284 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

284
docs/roadmap/audit/08-testing.md Normal file
View File

@@ -0,0 +1,284 @@
# Test Coverage And Effectiveness Audit
Reviewed 2026-11-04 against commit `f7085d7` and the current worktree. Existing
audit-document moves were preserved. No production code or tests were changed.
## Assessment And Method
The suite broadly follows the [testing policy](../../policy/testing.md): real
ConfigKit composition, filesystems, rendering, validation, and engine assembly
are exercised alongside controlled model clients and HTTP transports. Internal
tests own focused rules and coordination, external-package root tests protect
the public facade, and the three maintained examples have Go workflow tests.
Capacity, prepared-handle state transitions, ownership, cancellation, retries,
repair, observations, and configuration inspection have substantial behavioral
coverage. This is a targeted improvement list, not a recommendation to replace
the test architecture or pursue a percentage threshold.
Both required analyses were performed: review of missing behavioral protection,
and review of assertion/fixture cost and coupling. Evidence includes source and
call-graph review, existing GoDoc and integration contracts, full-suite coverage,
repeated race-enabled execution, and reuse of the earlier error audit's temporary
offline diagnostic. No live provider, paid API, new repository test, production
mutation, or external mutation-testing service was needed.
Validation used the local Go 1.27.0 toolchain:
```sh
go test -count=1 -covermode=atomic -coverpkg=./... -coverprofile=/tmp/promptkit-testing-audit-LHAyNm/coverage.out ./...
go tool cover -func=/tmp/promptkit-testing-audit-LHAyNm/coverage.out
go test -race -vet=off -shuffle=42 -count=2 ./...
```
Both suite runs passed, including the examples. The race run exercised two
shuffled passes; this is evidence against readily reproducible order/race
problems, not proof that no flakes exist. Coverage output remains outside the
repository. The commands are audit evidence, not additional release gates.
Combined coverage is **86.4%**, or 4,782 of 5,536 instrumented statements.
Selected package totals below combine hits across all test binaries. The
per-binary percentages printed by `-coverpkg=./...` are not individual-package
coverage percentages; repeated profile spans were combined before calculating
these totals.
| Component | Combined statement coverage |
| --- | ---: |
| Public facade | 81.0% |
| Configuration | 91.0% |
| Domain | 98.0% |
| Capacity | 98.0% |
| Generation coordination | 91.2% |
| HTTP model client | 94.2% |
| Execution use cases | 90.5% |
| Rendering and requirement inspection | 78.4% |
| Output/schema validation | 75.8% |
Coverage located candidates; the findings below concern specific risks and
assertions, not the percentages themselves. Testing recommendations associated
with already recorded implementation defects should accompany those fixes,
rather than becoming duplicate remediation projects.
## 1. RunRequest's Privacy Formatting Contract Is Not Exercised
**Priority:** P2. Missing protection for a documented confidentiality boundary.
**Relevant code:** [RunRequest formatting](../../../formatting.go:5) and the
analogous existing [SmokeTestRequest privacy test](../../../smoke_test_contract_test.go:135).
**Deficiency:** `RunRequest.String`, `GoString`, and their shared formatter all
have 0% combined statement coverage. The suite protects formatting of generated
client requests, smoke-test requests, and opaque prepared handles, but does not
exercise the ordinary run request's own formatting contract.
**Unverified behavior:** A change that prints the direct key, input bodies, or
variable values from `RunRequest` can pass the complete suite. This is an
assertion gap, not evidence that current formatting actually leaks those values.
**Potential improvement:** Add one small public-boundary test with distinct
synthetic private markers and exercise the supported String/GoString formatting
paths on values and pointers. Assert absence of the data covered by the GoDoc;
do not snapshot the complete summary or freeze incidental field order/count
formatting. If adding a JSON check, assert only the documented direct-key
exclusion, not a stable JSON shape or omission of ordinary request data.
Reflection-based disclosure is explicitly outside this formatter guarantee.
## 2. The Public Single-Schema-File Option Has No Execution Coverage
**Priority:** P2. An exported source-selection path is entirely unprotected.
**Relevant code:** [WithSchemaFile](../../../engine.go:212),
[SetSchemaFile](../../../internal/engine/engine.go:75), and
[fileSource](../../../internal/engine/engine.go:211).
**Deficiency:** All three functions have 0% combined coverage. The existing
[single-file validator test](../../../internal/validate/standard_validator_test.go:596)
constructs `FSValidator` directly with `fstest.MapFS`; it does not test the public
option, host-file selection, or its engine wiring.
**Unverified behavior:** The public option could ignore its argument, fail to
replace `SchemaDir`, select the wrong root/base name, or mishandle invalid files
without this validator test noticing. Direct validator success does not establish
that a consumer can reach that behavior through the supported API.
**Potential improvement:** Use `t.TempDir` and a real schema file in a small
public integration test. Exercise construction plus preparation/validation
through that file's base name, and representative invalid selections such as a
missing file or directory. Include source-option replacement where it protects
the documented last-valid-option behavior. Assert public error identity and
observable schema selection, not private helper calls. This does not require a
helper-by-helper test matrix or model-provider access.
## 3. Programmatic Provider-Value Conversion Lacks Representative Value Cases
**Priority:** P2. Conversion and ownership coverage misses known data-integrity
and deferred-error risks.
**Relevant tests:** [programmatic source ownership/order](../../../sources_contract_test.go:14)
and [programmatic construction](../../../construction_contract_test.go:226).
**Implementation:** [providerValueForMapping](../../../provider_values.go:31).
**Deficiency:** The converter has only 11.9% combined coverage. The principal
source-ownership test mutates a flat string-valued map. It does not establish
preservation and ownership for representative nested/typed containers or the
`json.Number` branch, nor deferred failure for unsupported provider values.
The more extensive `internal/jsonvalue` tests protect a different traversal and
do not execute this pre-ConfigKit convenience conversion.
**Unverified behavior:** The suite passes with the exact-number rounding defect
already recorded in [the correctness audit](01-correctness.md#1-provider-source-helpers-silently-change-exact-json-numbers).
It also would not detect many regressions in nested source capture or the
documented source-creation-versus-loading failure boundary.
**Potential improvement:** Exercise both `ProfileSource` and `BackendSource`
through `NewEngine` with a compact set of representative supported nested
values, mutations after source creation, and unsupported/cyclic values that
should fail during loading. Add the numeric regression when addressing the
existing precision finding. Compare meaningful final values or encoding, not Go
container identity, numeric spelling, or a required float representation.
Keep tests at the final handoff/public source-helper boundary; do not duplicate
ConfigKit's raw-source validation or its entire value-limit suite. Work-bound
coverage for the separately identified shared-graph issue belongs with that fix.
## 4. Error Tests Miss Composed Causes And Pre-Dispatch Diagnostic Failures
**Priority:** P2. Covered code paths lack important boundary-case assertions.
**Relevant tests:** [public generation failures](../../../generation_error_contract_test.go:111),
[direct mapper cases](../../../errors_internal_test.go:13),
[body cancellation](../../../internal/llm/prepared_generation_test.go:161), and
[inference diagnostic cases](../../../internal/usecase/smoke_inference_test.go:14).
**Deficiency and demonstrated blind spots:**
- Injected failures are mostly plain marker errors or a single explicit
category. They do not check a real nested PromptKit error returned through a
different operation. The suite passes although an injected client/reader can
lose the outer public generation/artifact category.
- Tests cover `http.Client.Do` failures and cancellation during successful-body
reading, but not an ordinary non-EOF read error during initial decoding or
final EOF verification. Such errors currently lose their cause and become
malformed-response failures.
- Diagnostic tests cover invalid selection/settings and concluded generation
failures separately, but miss a runtime setting rejected by built-in request
construction before dispatch. A reserved `ExtraParams["model"]` override
currently returns a completed report with nil Go error.
These are the three implementation findings in the error-handling audit, not
new duplicate defects. Re-running its temporary offline public-API diagnostic
against the current tree confirmed all three while the complete suite passed.
**Potential improvement:** Add narrowly owned regressions with those fixes:
real nested public causes through the injection boundary; a controlled body
reader failing at each decode/framing phase; and a public `SmokeTest` call with
a reserved runtime field and counted transport. Assert relevant error identities,
cause preservation, factual failure classification, and whether dispatch occurred.
Avoid reproducing the internal error topology by hand or matching complete
messages. Preserve intentional nil-error diagnostic findings, cancellation
precedence, best-effort non-2xx detail parsing, and status-only retry policy.
## 5. Multi-Resource Schema Tests Do Not Establish A Usable Provider Constraint
**Priority:** P2. Missing assembled validation-to-wire assertion.
**Relevant tests:** [prepared execution source freezing](../../../prepared_execution_contract_test.go:19),
its [referenced schema fixture](../../../prepared_execution_contract_test.go:809),
and [local reference validation](../../../internal/validate/standard_validator_test.go:627).
**Deficiency:** Local tests establish that referenced schemas compile and
validate. The prepared-execution test checks that the injected client's schema
matches frozen details. Its opaque model client accepts that schema without
interpreting references. Equality between those two values does not establish
that the provider-facing constraint contains the resources it needs.
**Unverified behavior:** Both sides can contain the same unresolved `value.json`
reference while the test passes. This is the packaging gap already identified
in [the correctness audit](01-correctness.md#2-external-schema-resources-do-not-reach-the-provider-constraint).
The freezing test's fake is appropriate for its stated ownership purpose; the
missing protection is a separate built-in-path integration case.
**Potential improvement:** Once the schema-packaging policy is selected, add one
offline assembled test with a root and child schema, capture the actual built-in
HTTP payload, and assess the chosen provider-facing representation independently
of `Details`. For a bundled native schema, compile/evaluate the captured
constraint without granting access to PromptKit's original local files; if the
chosen policy supports a local-only fallback, assert that behavior instead.
Keep the existing freezing test. Do not require live provider acceptance or
promise support for arbitrary provider schema dialect features.
## 6. Artifact Tests And Benchmarks Pin An Explicitly Opaque Hash Algorithm
**Priority:** P3. Remove or rewrite change-detector assertions, retaining the
substantive file-reading tests.
**Relevant assertions:** [size-hint test](../../../internal/artifact/reader_test.go:287)
and [benchmark correctness check](../../../internal/artifact/reader_benchmark_test.go:156).
**Deficiency:** Both independently calculate a hex SHA-256 digest and require
that exact value. The [public Artifact.Hash contract](../../../types.go:290)
expressly leaves its format and algorithm unspecified. A legitimate algorithm
or encoding change would fail tests unrelated to the requested behavior.
**Potential improvement:** Replace those algorithm-specific checks with the
opaque equality relationships needed for each test. For the under/overreported
size cases, comparing with the inline reader for the same complete body still
detects hashing only a reported-size prefix. The existing
[source-parity test](../../../internal/artifact/reader_test.go:27) already owns
repeatability, same-content equality, and changed-content inequality. Retain
body, actual-size, metadata, and read-boundary assertions. The benchmark may
reuse an opaque expected hash prepared outside measurement or rely on the
owning behavioral test; it need not maintain its own SHA-256 contract.
## 7. Bundled Catalog Expectations Have Multiple Test Owners
**Priority:** P3. Consolidation/removal candidate for duplicated assertions.
**Relevant tests:** [internal complete catalog table](../../../internal/configuration/bundled_test.go:12),
[public option/inventory table](../../../bundled_openrouter_contract_test.go:54),
[HTTP payload matrix](../../../bundled_execution_contract_test.go:51), and
[bundled smoke defaults](../../../bundled_smoke_contract_test.go:16).
**Deficiency:** The current count of 17, complete ID inventory, selected
ID-to-model mappings, tier-plugin data, and diagnostic default model recur
across configuration, facade, wire, and diagnostic tests. A catalog update can
require expectation edits in several layers even when loading, option handling,
routing, and serialization are unchanged. The internal plugin assertion also
searches `fmt.Sprint` output instead of comparing the plugin's structure.
**Potential improvement:** Choose one owner for the complete maintained asset
inventory and expected model/plugin/default data. Remove duplicate full-catalog
counts/lists from option permutations and keep those tests focused on enable,
disable, last-option selection, isolation, and override behavior. Retain a small
assembled wire/diagnostic set for distinct integration risks; controlled
overrides can isolate routing assertions from a changing catalog default.
Use semantic structural assertions for plugin data. Keep independent expected
asset values in the selected owner rather than deriving every expectation
from the production loader, which could conceal an omitted or incorrect asset.
This is consolidation, not a proposal to stop checking bundled configuration.
## Findings Not Supported And Work To Preserve
- No general test-flakiness, default-suite live-provider dependency, or race
failure was reproduced. Bounded timeout guards and channel synchronization
are not classified as flaky merely because a test involves concurrency.
- The public API/type-boundary checks and their checker fixtures protect an
explicit architectural rule; they are not automatically obsolete because
they inspect structure. Stable JSON shape tests protect selected contracts.
- Fakes at model/HTTP boundaries, counted paid-effect calls, and no-work-after-
rejection checks are appropriate. A fake is not deficient merely because it
does not model unrelated provider behavior.
- Exhaustive domain-rule tables plus representative boundary wiring tests are
generally sound. No wholesale deletion of error, cancellation, capacity,
ownership, or prepared-lifecycle coverage is recommended.
- Low coverage of thin wrappers, example `main` functions, defensive impossible
branches, and unexercised reflection variants is not independently a finding.
Interface-only packages do not need artificial tests to obtain a package
percentage. Conservative requirement inspection is not obliged to fully
evaluate unsupported template expressions just to cover every AST branch.
- The standalone release-wiring fixture remains scoped to local disposable
repositories under the documented testing model. This audit does not propose
moving it into every normal test run or retesting ReleaseKit's algorithms.
Prioritize the missing public privacy/source-option protection and regressions
for the already identified defects. Make the two assertion-cleanup changes as
focused rewrites, retaining their useful behavioral coverage. None of these
recommendations requires new public guarantees or a coverage-percentage gate.