chore: overhaul the test suite - #172
Merged
Merged
Conversation
Replace the per-test endure boilerplate with a Start helper that boots the container, waits on a readiness probe and tears down through t.Cleanup. Split the e2e into request/response header behaviour and CORS, and add a case proving the origin regex rejects an origin it does not match. Add root unit tests for Init: the two disabled paths, regex compilation and its error, the options_success_status default and override, and the header injection in both directions.
There was a problem hiding this comment.
Pull request overview
This PR overhauls the headers plugin test suite by extracting a shared RoadRunner boot helper, splitting e2e tests by theme (headers vs CORS), and adding new unit coverage for Plugin.Init and middleware behavior. It also updates CI to run root-module unit tests with coverage and upload both unit + e2e coverage outputs.
Changes:
- Introduced
tests/helpers.Startto centralize container boot + readiness probing and remove fixed sleeps. - Replaced the monolithic
tests/headers_plugin_test.gowith focused e2e tests (tests/headers_test.go,tests/cors_test.go) and added root-module unit tests (init_test.go) for init/error branches and middleware behavior. - Updated GitHub Actions workflow to run root-module unit tests with coverage and upload the full coverage directory as an artifact.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/helpers/rr.go | Adds a shared container boot helper with pluggable readiness probes and cleanup. |
| tests/headers_test.go | New focused e2e tests for boot + request/response header injection. |
| tests/cors_test.go | New focused e2e tests for wildcard and regex CORS behavior, including a rejecting case. |
| tests/headers_plugin_test.go | Removes the previous monolithic e2e test file in favor of themed tests + helper. |
| init_test.go | Adds unit tests covering Init disabled paths, regex compilation/error, CORS options default/override, and header injection. |
| .github/workflows/linux.yml | Adds root-module unit coverage tier and uploads the entire coverage-ci directory artifact. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wave 2 overhaul, same shape as gzip #244. No dependency bump needed — headers is already on
http beta.9.Simplification. 437 lines across one file, five tests, each opening with the same ~60 lines of
endure.New→RegisterAll→Init→Serve→signal.Notify→stopCh→ drain-goroutine. That is now aStart(t, cfg, plugins, opts...)helper, and the five fixed sleeps are gone in favour of a readiness probe.Split by theme.
headers_test.gocovers boot plus request/response header injection;cors_test.gocovers preflight and actual-request behaviour for both the wildcard and the regex config.New coverage. The old suite never checked that the origin regex rejects anything — it only asserted the matching case, so a regex that accepted everything would have passed. Added that case. Root unit tests now cover
Init: both disabled paths, regex compilation and its error branch, theoptions_success_statusdefault and override, and header injection in both directions.Local results: root unit 91.4%, e2e 81.0%, merged 98.3% against a current badge of 70%. Stable across 5 consecutive shuffled runs. Guard sees 88 blocks against a floor of 10.
CI gains the unit tier. The artifact now uploads the whole
coverage-cidirectory rather than a single file, so both tiers reach codecov.