Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request re-enables the Dgraph integration tests in the Cloud Build CI pipeline and configures them to run conditionally based on file changes. To support this, the Dgraph integration tests are updated to spin up an ephemeral Dgraph container using testcontainers-go when the DGRAPH_URL environment variable is not set. Feedback on the changes suggests improving the robustness of the container health check by decoding the JSON response instead of performing a fragile substring match on the raw response body.
| WithResponseMatcher(func(body io.Reader) bool { | ||
| b, err := io.ReadAll(body) | ||
| return err == nil && strings.Contains(string(b), `"status":"healthy"`) | ||
| }). |
There was a problem hiding this comment.
Using strings.Contains with a hardcoded JSON substring like "status":"healthy" is fragile because it is sensitive to whitespace formatting (e.g., spaces after colons like "status": "healthy"). Additionally, simply checking for the substring "healthy" can be problematic if the status is "unhealthy" (which contains the substring "healthy").
To make this check robust and immune to formatting differences, decode the JSON response and inspect the status field directly.
WithResponseMatcher(func(body io.Reader) bool {
var result []struct {
Status string
}
if json.NewDecoder(body).Decode(&result) != nil {
return false
}
for _, info := range result {
if info.Status == "healthy" {
return true
}
}
return false
}).0e937e9 to
1d00917
Compare
Run the tests against an ephemeral dgraph/standalone container instead. Setting DGRAPH_URL still runs them against an external instance.
1d00917 to
6979127
Compare
Description
Migrates the Dgraph integration test to testcontainers and re-enables its CI shard.
PR Checklist
CONTRIBUTING.md
bug/issue
before writing your code! That way we can discuss the change, evaluate
designs, and agree on the general idea
review
!if this involve a breaking change