Copilot commented on code in PR #93:
URL:
https://github.com/apache/dubbo-go-pixiu-samples/pull/93#discussion_r2312740960
##########
dubbogo/simple/prometheus/test/pixiu_test.go:
##########
@@ -30,41 +30,64 @@ import (
)
type _testMetric struct {
- buf string
+ metricChan chan string
+ buf string
}
func (tt *_testMetric) ServeHTTP(writer http.ResponseWriter, request
*http.Request) {
body, _ := io.ReadAll(request.Body)
tt.buf = string(body)
//logger.Info("read (%d, content-length: %d) => %s\n", len(tt.buf),
request.ContentLength, string(tt.buf))
writer.WriteHeader(200)
+
+ select {
+ case tt.metricChan <- tt.buf:
+ default:
+ }
+}
+
+func waitForMetric(t *testing.T, metricServer *_testMetric, expectedSubstring
string) {
+ timeout := time.After(2 * time.Second)
+ for {
+ select {
+ case receivedMetric := <-metricServer.metricChan:
+ if strings.Contains(receivedMetric, expectedSubstring) {
+ return
+ }
+ case <-timeout:
+ t.Fatalf("timed out waiting for metric. last received:
%q; expected to contain: %q", metricServer.buf, expectedSubstring)
+ }
+ }
Review Comment:
The function creates a new timeout channel on each call but continues
reading from the metric channel in an infinite loop without resetting the
timeout. If multiple metrics are received that don't contain the expected
substring, the function may timeout even if the correct metric arrives later.
Consider using a context with timeout or resetting the timeout for each
iteration.
##########
dubbogo/simple/prometheus/test/pixiu_test.go:
##########
@@ -30,41 +30,64 @@ import (
)
type _testMetric struct {
- buf string
+ metricChan chan string
+ buf string
}
func (tt *_testMetric) ServeHTTP(writer http.ResponseWriter, request
*http.Request) {
body, _ := io.ReadAll(request.Body)
tt.buf = string(body)
//logger.Info("read (%d, content-length: %d) => %s\n", len(tt.buf),
request.ContentLength, string(tt.buf))
writer.WriteHeader(200)
+
+ select {
+ case tt.metricChan <- tt.buf:
+ default:
+ }
+}
+
+func waitForMetric(t *testing.T, metricServer *_testMetric, expectedSubstring
string) {
+ timeout := time.After(2 * time.Second)
+ for {
+ select {
+ case receivedMetric := <-metricServer.metricChan:
+ if strings.Contains(receivedMetric, expectedSubstring) {
+ return
+ }
+ case <-timeout:
+ t.Fatalf("timed out waiting for metric. last received:
%q; expected to contain: %q", metricServer.buf, expectedSubstring)
+ }
+ }
}
func TestLocal(t *testing.T) {
metricServer := &_testMetric{
- buf: "",
+ buf: "",
+ metricChan: make(chan string, 10),
}
- http.Handle("/metrics", metricServer)
- go http.ListenAndServe(":9091", metricServer)
+
+ go func() {
+ server := &http.Server{Addr: ":9091", Handler: metricServer}
+ defer server.Close()
+ server.ListenAndServe()
Review Comment:
The `defer server.Close()` statement will not execute as intended because
`server.ListenAndServe()` blocks indefinitely. The deferred function will only
run when the goroutine exits, which may never happen in normal test execution.
Consider using `server.Shutdown(context.Background())` in a proper cleanup
mechanism or test teardown.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org
For additional commands, e-mail: notifications-h...@dubbo.apache.org
