157 lines
8.8 KiB
Markdown
157 lines
8.8 KiB
Markdown
# Roadmap: synchronize delivery test fixtures
|
||
|
||
Status: complete; all audited corrections implemented and verified in `059a795`.
|
||
|
||
The [concurrency audit](audit.md) records the wider review, additional findings,
|
||
corrections, and final verification evidence.
|
||
|
||
## Objective and evidence
|
||
|
||
Make the delivery HTTP fixtures safe for concurrent access by test goroutines
|
||
and request handlers while preserving their existing behavioral assertions.
|
||
The [PromptKit bug report](promptkit-bug-report.md) supplies the original
|
||
diagnostics and consumer context.
|
||
|
||
Upstream investigation reproduced
|
||
`TestReleaseKitSourceDeliveryWorkflowFromTaggedCheckout` under the race detector
|
||
with Go 1.26.8. The diagnostics identified `commit`, `remoteCommit`, and
|
||
`releaseCreates` accesses shared between the workflow test and HTTP handlers.
|
||
The workflow test and relevant unsynchronized accesses predate v0.4.0.
|
||
Subprocess completion supplies no synchronization for this shared Go state.
|
||
|
||
The ordinary Woodpecker and multi-module workflow tests passed their focused
|
||
race-enabled runs during investigation. Source inspection nevertheless found
|
||
the same unsynchronized state pattern in their fixtures. A passing run alone
|
||
does not establish safe access under other scheduling.
|
||
|
||
## Scope
|
||
|
||
The correction belongs in test infrastructure:
|
||
|
||
- The `deliveryServerState`, `newDeliveryServer`, and `writeDeliveryRelease`
|
||
helpers in [main_test.go](../../cmd/releasekit/main_test.go), including all
|
||
tests using that fixture.
|
||
- The [source-delivery workflow test](../../cmd/releasekit/releasekit_delivery_workflow_test.go)
|
||
and [ordinary Woodpecker workflow tests](../../cmd/releasekit/woodpecker_test.go).
|
||
- The `mappedDeliveryFixture`, `newMappedDeliveryFixture`, and their consumers
|
||
in [multi_module_woodpecker_test.go](../../cmd/releasekit/multi_module_woodpecker_test.go).
|
||
- The public delivery fixture in [delivery_test.go](../../delivery_test.go),
|
||
including repository-resolution and partial-clone test consumers, plus server
|
||
startup in the delivery benchmark and artifact-read regression test.
|
||
- Shared observations, HTTP assertion helpers, and server cleanup in the Gitea
|
||
[release tests](../../internal/gitea/release_test.go) and
|
||
[asset tests](../../internal/gitea/assets_test.go).
|
||
|
||
Production behavior, public API contracts, and release requirements retain their
|
||
current definitions. Use the standard library and small fixture-local helpers,
|
||
following the [agent](../policy/agents.md) and
|
||
[testing](../policy/testing.md) policies.
|
||
|
||
## Implementation
|
||
|
||
1. **Synchronize the shared delivery fixture.** Give mutable fixture state a
|
||
`sync.Mutex`. Protect handler reads and writes, test-side configuration
|
||
changes, assertion reads, counters, page state, note content, and asset-map
|
||
operations with the same mutex. Initialize immutable configuration before
|
||
serving requests where practical; synchronize later initialization such as
|
||
the source commit assigned after profile creation. Review captured handler
|
||
values and response helpers as part of this boundary.
|
||
|
||
2. **Update every shared-fixture consumer.** Use simple locked setters and
|
||
snapshot accessors for test configuration and assertions. Snapshot plain
|
||
values separately from the mutex, copying maps and mutable byte slices when
|
||
they will be inspected outside the lock. Include failure diagnostics that
|
||
format fixture state. Keep lock ownership clear in handler helpers and
|
||
release locks before executing CLI workflows or closing the server.
|
||
|
||
3. **Apply the same discipline to the mapped fixture.** Protect its commit,
|
||
request and creation counters, page fields, uploads, and asset map, together
|
||
with all corresponding test accesses. Keep each fixture's synchronization
|
||
local and proportionate to its existing responsibilities.
|
||
|
||
4. **Synchronize the public delivery fixture.** Protect request counts,
|
||
selected tags and notes, release-page maps, and asset maps with a fixture-local
|
||
mutex. Use locked updates and independent snapshots in every consumer,
|
||
including repository-resolution and partial-clone tests. Update related tag
|
||
and note fields together. Initialize server URLs before serving requests in
|
||
the public fixture, benchmark, and artifact-read regression fixture. Reuse
|
||
captured snapshots within binary assertion groups.
|
||
|
||
5. **Synchronize Gitea scalar observations.** Use atomic counters and flags for
|
||
independent creation counts, lookup counts, creation state, cancellation
|
||
observations, and external attachment download counts. Update handler
|
||
accesses, test assertions, and diagnostic reads together. Retain exact
|
||
request-count, reconciliation, and credential assertions.
|
||
|
||
6. **Keep fatal assertions in the test goroutine.** Change HTTP handlers and
|
||
their assertion helpers to report failures through `Error` or `Errorf` and
|
||
return from the affected handler or helper. Preserve failure reporting for
|
||
unexpected requests, malformed payloads, and incorrect credentials.
|
||
|
||
7. **Give release-server cleanup a single owner.** Register `server.Close` with
|
||
`t.Cleanup` inside `newReleaseServer` so all consumers shut down the listener
|
||
and serving goroutines when their test finishes.
|
||
|
||
8. **Preserve behavioral coverage.** Retain rejection of mismatched hosted tags
|
||
before release creation, exact source-tag preservation, complete note bodies,
|
||
idempotent page and asset delivery, expected asset counts, mapped module
|
||
identity, retries using tagged configuration, and existing failure outcomes.
|
||
Use these workflows as the regression coverage for the reported race. Add
|
||
further coverage only for a concrete gap discovered during the correction.
|
||
|
||
## Audit finding coverage
|
||
|
||
All five finding groups are corrected in `059a795`. The audit owns their
|
||
diagnostics and review evidence; this roadmap records the corresponding work.
|
||
|
||
| Audit finding | Roadmap work | Status |
|
||
| --- | --- | --- |
|
||
| [1. CLI and mapped fixture state](audit.md#1-cli-delivery-fixtures-share-mutable-state-without-synchronization) | Steps 1–3: synchronized handlers, configuration, snapshots, and consumers. | Complete. |
|
||
| [2. Public delivery fixture state](audit.md#2-the-public-delivery-fixture-has-the-same-unsynchronized-ownership) | Step 4: synchronized public fixture and safe server startup. | Complete. |
|
||
| [3. Gitea counters and flags](audit.md#3-gitea-adapter-fixtures-expose-unsynchronized-counters-and-flags) | Step 5: atomic observations and diagnostic reads. | Complete. |
|
||
| [4. Fatal HTTP assertions](audit.md#4-http-assertions-invoke-fatal-methods-from-request-goroutines) | Step 6: failure reporting and returns from handlers. | Complete. |
|
||
| [5. Server lifetime cleanup](audit.md#5-a-release-fixture-omits-server-lifetime-cleanup) | Step 7: cleanup owned by the release-server helper. | Complete. |
|
||
|
||
## Verification and completion
|
||
|
||
With dependencies prepared, run the affected workflows from the repository root
|
||
using the local toolchain and offline module settings:
|
||
|
||
```sh
|
||
GOTOOLCHAIN=local GOWORK=off GOPROXY=off GOSUMDB=off \
|
||
go test -race -mod=readonly -count=1 ./cmd/releasekit \
|
||
-run '^(TestReleaseKitSourceDeliveryWorkflowFromTaggedCheckout|TestWoodpeckerTagDeliveryTemplate|TestMultiModuleWoodpeckerDelivery)$'
|
||
```
|
||
|
||
Then run the full race-enabled suite once to verify the other shared-fixture
|
||
users and catch remaining cross-goroutine access:
|
||
|
||
```sh
|
||
GOTOOLCHAIN=local GOWORK=off GOPROXY=off GOSUMDB=off \
|
||
go test -race -mod=readonly -count=1 ./...
|
||
```
|
||
|
||
Review the fixture accesses as well as the test results: successful execution
|
||
complements the synchronization review. Keep local Git repositories, HTTP
|
||
servers, and synthetic module caches isolated as described in
|
||
[development](../development.md#testing-synthetic-modules).
|
||
|
||
Completion means every audited fixture and its consumers use synchronized state,
|
||
HTTP assertions and server cleanup follow their goroutine ownership, the existing
|
||
assertions remain effective, and the focused and full race-enabled suites pass.
|
||
Record completion and verification here and update the bug report's status with
|
||
the remediation outcome. Commit the completed correction under the repository's
|
||
ordinary change workflow.
|
||
|
||
This verification addresses the reported defect. Future release validation
|
||
continues to follow the [release procedure](../release.md). The correction is
|
||
suitable for a subsequent patch release; preparation and publication are separate
|
||
work under that procedure, preserving the published v0.4.0 contents.
|
||
|
||
**Completed:** CLI, mapped, and public delivery fixtures and their consumers use
|
||
mutex-protected updates and independent snapshots. Existing mismatch, note,
|
||
asset, retry, and source-preservation assertions remain in place. Focused workflow checks and the
|
||
full offline race-enabled suite passed with Go 1.26.8; the compiled API/dependency
|
||
check passed. The audit also corrected public delivery fixture ownership,
|
||
Gitea scalar observations, HTTP assertion handling, and server cleanup.
|