-
Notifications
You must be signed in to change notification settings - Fork 19
Description
The RestateDeployment controller's admin API code has zero test coverage. Changes to error handling can only be verified by manually deploying against a live cluster. The recent work to surface response bodies in status conditions (#100) was tested entirely by hand.
What needs coverage
- check_admin_response() - extracts response body on non-2xx status
- register_service_with_restate() and list_deployments() - the main admin API call sites
- Error handling arms in reconcile_status() - status condition messages, warn-level logging, K8s event emission
- Cleanup paths in replicaset.rs and knative.rs - force-delete with 404 tolerance
I did a bit of research for how it could be done, these are just research notes, not implementation requirements.
On mocking (wiremock/mockito/httpmock): reqwest::Response is sealed so you can't construct one in-memory - even "unit" tests need a local HTTP server. wiremock is lightweight (localhost on a random port) and the mock setup is reusable across tests. Good for: testing our error handling logic, fast, runs in CI. Bad for: doesn't catch response format drift from real Restate.
On integration tests with real Restate: run restatedev/restate in Docker and test against it. Good for: catches real error formats, tests the actual registration flow. Bad for: harder to trigger specific error cases (e.g. reliably causing a 409), slower. Could be a just test-integration target or testcontainers-rs (the latter handles port allocation and cleanup automatically, but for a single container the difference is marginal).
These are complementary: wiremock for fast logic tests in CI, real Restate for end-to-end validation. Start with whichever is more useful at the time.
Related: there are 5 pre-existing broken doctests in podidentityassociations.rs and quantity_parser.rs that should be fixed. CI currently only runs clippy, not tests.