chore: overhaul the test suite - #152
Merged
Merged
Conversation
Replace the per-test endure boilerplate with a Start helper and dial helpers for the three connection kinds the configs need: plaintext, TLS trusting the test CA, and mutual TLS presenting the client certificate. The 22 fixed sleeps are gone. The gzip file repeated every scenario from the main file with one extra dial option, 452 lines for four cases. It now reuses the same configs and helpers, so gzip is a dial option rather than a parallel suite. Add the negative half of mutual TLS: a client that presents no certificate must be refused. The suite only ever asserted the success path, so a server that ignored client_auth_type would have passed. Split the rest by theme: request/response, TLS, gzip, reflection and observability. The status test gains an unregistered plugin name, which must report nothing rather than matching. Fix the test-certs ignore rule. It read ./tests/test-certs/**, and the leading ./ makes the pattern match nothing, so generated certificates and their private keys were never actually ignored.
coverpkg named the root package only, so codec, parser and proxy went unmeasured, and the e2e step listed test files by name so a new file would not run. Drop -failfast, unfold the ee step from a single escaped line, and fail the codecov job when the merged summary maps to no plugin source.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #152 +/- ##
==========================================
+ Coverage 78.38% 78.41% +0.03%
==========================================
Files 7 10 +3
Lines 495 746 +251
==========================================
+ Hits 388 585 +197
- Misses 80 117 +37
- Partials 27 44 +17 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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, and the last of the infra group. 1893 lines of e2e across three files, 20 tests, 22 fixed sleeps.
The gzip file was a parallel copy of the suite. 452 lines repeating every scenario from the main file with one extra dial option. gzip is now a dial option rather than a second suite, so those four cases reuse the same configs and helpers.
Mutual TLS had no negative case. The suite asserted only that a properly certified client succeeds — a server that ignored
client_auth_type: require_and_verify_client_certwould have passed every test. Added the case where a client presents no certificate and must be refused.Dial helpers cover the three connection kinds the configs need: plaintext, TLS trusting the test CA, and mutual TLS with the client certificate. Each test now says which one it wants instead of rebuilding
tls.Configinline — that block appeared five times.Split by theme: request/response, TLS, gzip, reflection, observability (metrics, status, otel, otlp). The status test also gained an unregistered plugin name, which must report nothing rather than matching whatever it is asked for.
A gitignore rule that never worked.
.gitignoreread./tests/test-certs/**; a leading./makes the pattern match nothing, so the generated certificates — including private keys — were not actually ignored. Anyone running the suite locally could commit them. Nowtests/test-certs/.Local results: root unit 25.3%, e2e 68.1%, merged 81.9% against a current badge of 78%. Full suite including every TLS case runs in ~25s and is stable across 3 consecutive shuffled runs — verified here with mkcert-generated certificates, matching what CI builds.
Two corrections I made after reading the originals rather than assuming:
CI:
-coverpkgnamed only the root package, socodec,parserandproxywere never measured. The e2e step also listed test files by name, so a newly added file simply would not run — both now use./.... Dropped-failfast, unfolded the ee step from a single escaped line into a readable block, and added the coverage guard.