From b345bc0ec443a1b75d18676fc659e40399f55c54 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 2 Nov 2026 01:43:27 +0000 Subject: [PATCH] Record test coverage audit findings --- docs/roadmap/audit/08-testing.md | 97 ++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 docs/roadmap/audit/08-testing.md diff --git a/docs/roadmap/audit/08-testing.md b/docs/roadmap/audit/08-testing.md new file mode 100644 index 0000000..6da7fa7 --- /dev/null +++ b/docs/roadmap/audit/08-testing.md @@ -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.