Add REST handler integration tests - #380
Merged
Depo-dev merged 2 commits intoJul 31, 2026
Merged
Conversation
|
@deltron-fr Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Three conflicts, all additive rather than genuine disagreements. - ci.yml — kept both sides: this branch's REST handler coverage gate and dev's comment explaining the golangci-lint-action v8 bump. - internal/httputil/errors.go — took dev's comment on the codes.Unavailable case. The case itself is identical on both sides. - handlers/status_test.go — took dev's. This branch's copy is `package handlers` (internal) and calls InternalStatus() unqualified, while dev split that file in two: the external-package tests stayed in status_test.go and the internal ones moved to internal_status_test.go. Keeping this branch's version would have redeclared five functions. Checked the coverage rather than assuming: all six test functions on this branch already exist on dev — five in status_test.go and TestInternalStatus_UnsetKey_EmptyProvidedKey_StillRejected in internal_status_test.go. Nothing was lost by taking dev's side. The coverage gate this PR adds passes on the merged result: ListEvents 100.0%, GetEvent 90.5%, Health 90.3%, all above its 80% floor. Verified: gofmt, go vet, and go test across all ten services/api packages.
Depo-dev
added a commit
to Emrys02/Trident
that referenced
this pull request
Jul 31, 2026
…el-Labs#331/Telocel-Labs#353/Telocel-Labs#359/Telocel-Labs#378/Telocel-Labs#380) Six PRs landed on dev since the first merge, so this branch went dirty again. One conflict: services/api/handlers/health_test.go, add/add — both sides created it. The two files are not alternatives. dev's is `package handlers_test` with a single table-driven TestHealthHandler_TableDriven; this branch's is `package handlers` with six tests covering the /v1/health and /v1/ready split it introduces (Telocel-Labs#243). Kept this branch's file and dropped dev's, having checked what that costs. dev's four cases — all dependencies reachable, db down, grpc down, redis down — map one-to-one onto TestReady_AllHealthy_Returns200, TestReady_PostgresDown_Returns503, TestReady_GRPCDown_Returns503, and TestReady_RedisDown_Returns503. This branch adds a fifth (TestReady_NilDependencies_Returns503) and TestHealth_AlwaysReturns200. Keeping dev's file was not an option regardless: it asserts against handlers.HealthResponse, a type this branch deliberately split into LivenessResponse and ReadyResponse, and it expects /v1/health to return 503 when a dependency is down — the exact behaviour Telocel-Labs#243 changes, since a liveness probe must not fail because Postgres is unreachable. Verified: go vet and go test across all twelve services/api packages, cargo fmt --check, cargo clippy --workspace --all-targets -D warnings, and the REST handler coverage gate added by Telocel-Labs#380 still passes (ListEvents 100.0%, GetEvent 90.5%, Health 100.0%).
Depo-dev
pushed a commit
that referenced
this pull request
Jul 31, 2026
…it (#374) Four related pieces of API observability and operability work: - **Prometheus metrics** on a dedicated `METRICS_PORT` listener (issue #58), recorded by middleware wrapped outermost so requests shed by the global concurrency limiter are still counted. - **pgxpool saturation stats** polled into gauges every 15s (issue #238) — total/idle/acquired connections and acquire-wait time are the direct signal that slow queries are starving the pool. - **A liveness/readiness split** (issue #243). `GET /v1/health` is now dependency-free and always 200: restarting a pod never fixes an unreachable Postgres, so a liveness probe must not fail because a dependency is down. `GET /v1/ready` does the dependency checks and gates traffic routing; `fly/api.toml` now probes it. - Header and contract documentation in the OpenAPI spec. Maintainer changes on top of the original branch: Merged `dev` twice — once before the batch, once after six other PRs landed on the same files. The notable resolution is metrics. Both branches independently created `services/api/middleware/metrics.go`, but the two implementations are complementary rather than competing: dev's hand-rolled counters render into the public `GET /metrics` route and are called by `handlers/stats.go`, while this branch's registry-backed middleware serves the separate metrics port. They share no state. This branch's version moved to `metrics_registry.go` so the boundary stays visible. Also combined rather than picked: `main.go` keeps both this branch's pool poller and dev's usage-rollup loops; `openapi.yaml` keeps dev's clearer batch description with this branch's `X-RateLimit-*` headers; `docs/deployment.md` keeps this branch's `/v1/ready` endpoint alongside dev's gRPC TCP-check caveat. `go.mod` took this branch's newer set and was reconciled with `go mod tidy`. On the second pass, `health_test.go` conflicted add/add. Kept this branch's six tests: dev's four cases map one-to-one onto the `TestReady_*` tests, and dev's file could not survive regardless — it asserts against `handlers.HealthResponse`, the type this branch splits into `LivenessResponse` and `ReadyResponse`, and expects `/v1/health` to 503 on a dependency failure, which is exactly the behaviour #243 changes. Three stale references fixed, all pre-existing here and only reachable once the branch compiled against dev: `contract_test.go` still called `handlers.Health(nil, nil, nil)` after the zero-arg refactor; `TestContract_RouteParity` failed on `GET /v1/ready` being "documented but not registered" when it was registered — the test's route list is hand-maintained; and the OpenAPI spec needed the same route added. The REST handler coverage gate added by #380 still passes: ListEvents 100.0%, GetEvent 90.5%, Health 100.0%. All 20 checks pass.
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
2 participants
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.
closes #59
Add deterministic REST handler coverage for
/v1/events,/v1/events/{id}, and/v1/healthusing httptest and fake dependencies. Covers success paths, validation errors, pagination forwarding, gRPC error mapping, and degraded health responses. Also adds CI coverage enforcement for the target handlers.