test(mongodb): add live integration tests for the MongoDB proxy#104
test(mongodb): add live integration tests for the MongoDB proxy#104bharathnr1 wants to merge 2 commits into
Conversation
The existing proxy_test.go only covers URI building and the not-configured
paths; the handlers that run real commands (serverStatus / replSetGetStatus /
currentOp / find / aggregate) are never exercised against a live MongoDB.
Add build-tagged (`//go:build integration`) integration tests that, given a
reachable MongoDB via FORAGER_MONGO_TEST_HOST, configure the proxy and assert
the read-only diagnostics return a 200 with the expected document shape. They
skip cleanly when no database is configured, so `go test ./...` is unaffected.
Run:
docker run -d -p 27017:27017 mongo:7 --replSet rs0 --bind_ip_all
docker exec <id> mongosh --quiet --eval "rs.initiate({_id:'rs0',members:[{_id:0,host:'localhost:27017'}]})"
FORAGER_MONGO_TEST_HOST=localhost FORAGER_MONGO_TEST_REPLICA_SET=rs0 \
go test -tags integration -run TestIntegration -v ./pkg/proxy/mongodb/
Signed-off-by: Bharath Nallapeta <bnallapeta@mirantis.com>
|
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces integration tests for the MongoDB proxy in proxy_integration_test.go, which run against a live MongoDB instance. The feedback suggests adding a defensive nil check for the response object in the decodeData helper function to prevent potential nil pointer dereference panics during test execution.
| func decodeData(t *testing.T, resp *proxy.ActionResponse, err error, action string) map[string]any { | ||
| t.Helper() | ||
| if err != nil { | ||
| t.Fatalf("%s: unexpected error: %v", action, err) | ||
| } | ||
| if resp.StatusCode != 200 { | ||
| t.Fatalf("%s: expected status 200, got %d (data=%s)", action, resp.StatusCode, resp.Data) | ||
| } |
There was a problem hiding this comment.
If err is nil but resp is also nil (for example, if a handler incorrectly returns nil, nil), accessing resp.StatusCode will cause a panic. Adding a defensive check to ensure resp is not nil before accessing its fields will allow the test to fail gracefully with a clear message.
func decodeData(t *testing.T, resp *proxy.ActionResponse, err error, action string) map[string]any {
t.Helper()
if err != nil {
t.Fatalf("%s: unexpected error: %v", action, err)
}
if resp == nil {
t.Fatalf("%s: response is nil", action)
}
if resp.StatusCode != 200 {
t.Fatalf("%s: expected status 200, got %d (data=%s)", action, resp.StatusCode, resp.Data)
}There was a problem hiding this comment.
Good call — added the resp == nil guard in decodeData before dereferencing, so a handler returning (nil, nil) fails with a clear message instead of panicking. Pushed in 280a392; gofmt/go vet -tags integration clean.
Addresses review: guard against a handler returning (nil, nil) so the test fails with a clear message instead of panicking on resp.StatusCode. Signed-off-by: Bharath Nallapeta <bnallapeta@mirantis.com>
|
Have signed the CLA twice. But its not recognizing it. |
2 participants
Description
pkg/proxy/mongodb/proxy_test.gocovers URI building and the not-configured paths, but the handlers that actually run commands —serverStatus/replSetGetStatus/currentOp/find/aggregate— are never exercised against a live MongoDB (the unit test even notes "skip since it requires real mongo").This adds build-tagged (
//go:build integration) integration tests for the read-only diagnostics. Given a reachable MongoDB viaFORAGER_MONGO_TEST_HOST, they configure the proxy and assert each diagnostic returns a200with the expected document shape:mongo_server_status→ok/connections/uptimemongo_current_ops→inprogmongo_repl_status→set/members(whenFORAGER_MONGO_TEST_REPLICA_SETis set; otherwise asserts the handler surfaces the error rather than panicking)They skip cleanly when no database is configured, so
go test ./...(andmake test) are unaffected — this is opt-in and adds no dependencies.How Has This Been Tested?
Verified against MongoDB 7.0 (single-node replica set). Without
FORAGER_MONGO_TEST_HOST, the tests skip.Context
Came up while validating the MongoDB AI-troubleshooting tool in nudgebee/nudgebee#385, which relies on these forager diagnostics — this gives that path reproducible coverage on the forager side.
🤖 Generated with Claude Code