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

Record test coverage audit findings

Browse Source
This commit is contained in:
Eric Rakestraw
2026-11-02 01:43:27 +00:00
parent 3d7a691338
commit b345bc0ec4
1 changed files with 97 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -0,0 +1,97 @@
# Test coverage and effectiveness audit
Reviewed baseline: `3d7a691` on `main` against the
[testing policy](../../policy/testing.md). The review considered public and
package contracts, failure and recovery paths, fixture realism, and the cost
of implementation-coupled assertions. Coverage is a way to locate questions,
not evidence that every exercised behavior is correct. No production code or
tests were modified for this audit.
The offline instrumented suite passed with
`GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off go test -coverpkg=./... -coverprofile=/tmp/releasekit-audit-coverage.out ./...`.
`go tool cover -func` reported 86.0%
aggregate statement coverage across instrumented packages. Child executables
and platform-specific paths are not necessarily represented by that aggregate;
it does not measure branch coverage or assertion strength.
## 1. Publication fixtures omit Git settings that change what is published
The public [publication success test](../../../publish_test.go) checks the
remote tag object, peeled commit, message, and unrelated refs using a local
bare remote. However, its ordinary Git configuration does not exercise
`push.recurseSubmodules=only`, under which the
[correctness audit](01-correctness.md#1-high-publication-can-report-success-while-leaving-the-tag-unpublished)
reproduced a successful result with no remote tag. The same test checks object
IDs and tag-message bytes, but its fixture has no Git replacement objects;
the [second correctness finding](01-correctness.md#2-high-git-replacement-objects-can-separate-validated-content-from-published-source)
demonstrates that the validated contents and remote contents can differ while
the recorded commit ID remains the same.
When these defects are addressed, add focused public-boundary regressions with
a real local bare remote. For the inherited push mode, assert that a successful
result corresponds to the exact remotely present tag object. For replacement
objects, compare the actual validated and remotely published source and note
bytes, or assert a contextual rejection before publication. Preserve the
existing ordinary success case; it protects different important behavior.
## 2. Cancellation fixtures do not cover inherited subprocess descriptors
[process cancellation tests](../../../internal/process/process_test.go) and
[candidate reader lifecycle tests](../../../internal/validation/tree_lifecycle_test.go)
exercise cancellation of a ready child and check error identity and cleanup.
Their blocked children do not leave a descendant holding an output descriptor
after the direct child is killed. The independent probes recorded in the
[concurrency audit](04-concurrency.md#1-medium-inherited-descriptors-can-keep-canceled-operations-and-reader-shutdown-blocked)
show that both buffered execution and batch-reader shutdown can then remain
blocked past cancellation, despite those tests passing.
Add controlled regressions when repairing the shared shutdown path: establish
that a descendant holds a descriptor, cancel only after readiness, and verify
bounded return, preserved context identity, and cleanup. Exercise both the
buffered subprocess boundary and the explicitly owned batch-reader path; retain
one public preparation or publication-recovery check if needed to show the
shared behavior reaches callers. Avoid treating arbitrary descendant-process
supervision as an additional contract.
## 3. Local tag creation lacks a post-write inspection failure case
The [publication tests](../../../publish_test.go) cover successful exact tag
publication, a rejected push with the local tag retained, and a changed tag
before push. They do not cover failure of the inspection commands immediately
after successful local tag creation. The isolated probe in the
[error-handling audit](06-error-handling.md#a-post-creation-inspection-failure-hides-a-retained-local-tag)
shows that the tag can remain while the returned object ID is empty and the
error does not identify that partial local state.
When the recovery behavior is selected and fixed, use a controlled Git wrapper
and a real temporary repository to fail post-creation inspection. Verify both
the actual local ref and the reported recovery evidence, the original error,
and absence of a remote push. Do not rely solely on a mocked return value.
## 4. Some assertions lock in incidental internal mechanisms
[TestMarkdownCacheSharesParsedDocumentsByBlobObjectID](../../../internal/validation/markdown_test.go)
requires two parsed document pointers to be identical. A correct cache that
returns independent immutable values, or another memory strategy, would fail
without changing link validation. The separate
[shared-document link test](../../../internal/validation/markdown_check_test.go)
already checks that identical content resolves relative to each source path.
Consider removing the pointer-identity assertions or the cache-specific test
after retaining any unique parsing checks at the behavior boundary.
[TestDeliverReadsLocalBinaryOnceForVerificationAndOnceForUpload](../../../delivery_reads_linux_test.go)
requires exactly two filesystem opens using Linux inotify. Verification before
hosted writes and correct uploaded bytes matter; the exact number of opens is
an implementation choice, and a harmless extra metadata open or a different
safe streaming approach would fail the test. Existing delivery and artifact
tests cover tampering and uploaded content. Consider removing this exact-count
test or moving a justified read-volume concern to a benchmark, while retaining
behavioral verification of artifact integrity and ordering.
The success case in [TestPublishPushesOnlyTheCapturedAnnotatedTag](../../../publish_test.go)
also asserts exactly three remote queries, including no post-success read-back.
That exact success-path count is not necessary to establish the public result,
and would reject an additional verification step even if it preserved the
contract. Consider removing that assertion while retaining the exact remote
tag, message, and unrelated-ref checks. The separately documented single
reconciliation query after a failed push remains a distinct contract.