Refactor reverse proxy tests to improve path rewriting assertions. Updated reverse_test.go to introduce a captureTransport for validating outgoing requests and added new tests for director path rewrites, enhancing test reliability and coverage for nested API routes.

internal/proxy/reverse_test.go

@@ -8,72 +8,132 @@ import (
 	"testing"
 )
 
-func TestReverseProxyPathRewrite(t *testing.T) {
+// captureTransport records the outgoing request and returns 200 without dialing.
+// httptest.ResponseRecorder + ReverseProxy + real httptest.Server can yield flaky
+// or environment-dependent failures (proxy env, request-line quirks); we assert
+// Director output directly instead.
+type captureTransport struct {
+	got *http.Request
+}
+
+func (c *captureTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	c.got = req.Clone(req.Context())
+	return &http.Response{
+		StatusCode: http.StatusOK,
+		Body:       http.NoBody,
+		Header:     make(http.Header),
+	}, nil
+}
+
+func TestDirectorRewritesPath(t *testing.T) {
+	target, err := url.Parse("http://127.0.0.1:9")
+	if err != nil {
+		t.Fatal(err)
+	}
+	cap := &captureTransport{}
+	rp := NewReverseProxy(target, "/api/main_srv", "/v1", "")
+	rp.Transport = cap
+
+	req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "http://127.0.0.1:9/api/main_srv/health", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	rp.ServeHTTP(httptest.NewRecorder(), req)
+
+	if cap.got == nil {
+		t.Fatal("no outgoing request captured")
+	}
+	want, err := url.Parse("http://127.0.0.1:9/v1/health")
+	if err != nil {
+		t.Fatal(err)
+	}
+	assertSameURL(t, cap.got.URL, want)
+}
+
+func TestDirectorRewritesPathWithAPIBasePath(t *testing.T) {
+	target, err := url.Parse("http://127.0.0.1:9/api/")
+	if err != nil {
+		t.Fatal(err)
+	}
+	cap := &captureTransport{}
+	rp := NewReverseProxy(target, "/api/main_srv", "/v1", "")
+	rp.Transport = cap
+
+	req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "http://127.0.0.1:9/api/main_srv/health", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	rp.ServeHTTP(httptest.NewRecorder(), req)
+
+	if cap.got == nil {
+		t.Fatal("no outgoing request captured")
+	}
+	want, err := url.Parse("http://127.0.0.1:9/api/v1/health")
+	if err != nil {
+		t.Fatal(err)
+	}
+	assertSameURL(t, cap.got.URL, want)
+}
+
+func TestDirectorRewritesNestedStatsUsers(t *testing.T) {
+	target, err := url.Parse("http://127.0.0.1:9/api/")
+	if err != nil {
+		t.Fatal(err)
+	}
+	cap := &captureTransport{}
+	rp := NewReverseProxy(target, "/api/gt2", "/v1", "")
+	rp.Transport = cap
+
+	req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "http://127.0.0.1:9/api/gt2/stats/users", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	rp.ServeHTTP(httptest.NewRecorder(), req)
+
+	if cap.got == nil {
+		t.Fatal("no outgoing request captured")
+	}
+	want, err := url.Parse("http://127.0.0.1:9/api/v1/stats/users")
+	if err != nil {
+		t.Fatal(err)
+	}
+	assertSameURL(t, cap.got.URL, want)
+}
+
+// TestBuildUpstreamURLRoundTrip checks that a URL built like the Director does is
+// accepted by net/http against httptest (no ReverseProxy). Isolates JoinPath + server.
+func TestBuildUpstreamURLRoundTrip(t *testing.T) {
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if r.URL.Path != "/v1/health" {
-			t.Fatalf("path %q", r.URL.Path)
+			http.Error(w, "bad path", http.StatusBadRequest)
+			return
 		}
 		w.WriteHeader(http.StatusOK)
 	}))
 	defer srv.Close()
-	up, _ := url.Parse(srv.URL)
+
-	rp := NewReverseProxy(up, "/api/main_srv", "/v1", "")
+	base, err := url.Parse(srv.URL)
-	// Client-style request (no RequestURI / server quirks from httptest.NewRequest).
-	req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, srv.URL+"/api/main_srv/health", nil)
 	if err != nil {
 		t.Fatal(err)
 	}
-	rec := httptest.NewRecorder()
+	joined := buildUpstreamURL(base, "/v1", "health")
-	rp.ServeHTTP(rec, req)
+	req, err := http.NewRequest(http.MethodGet, joined.String(), nil)
-	if rec.Code != http.StatusOK {
+	if err != nil {
-		t.Fatalf("status %d", rec.Code)
+		t.Fatal(err)
+	}
+	resp, err := DirectTransport().RoundTrip(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		t.Fatalf("status %d", resp.StatusCode)
 	}
 }
 
-func TestReverseProxyPathRewriteWithAPIBasePath(t *testing.T) {
+func assertSameURL(t *testing.T, got, want *url.URL) {
-	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	t.Helper()
-		if r.URL.Path != "/api/v1/health" {
+	if got.Scheme != want.Scheme || got.Host != want.Host || got.Path != want.Path || got.RawQuery != want.RawQuery {
-			t.Fatalf("path %q", r.URL.Path)
+		t.Fatalf("URL mismatch\ngot  %q\nwant %q", got.String(), want.String())
-		}
-		w.WriteHeader(http.StatusOK)
-	}))
-	defer srv.Close()
-	up, err := url.Parse(srv.URL + "/api/")
-	if err != nil {
-		t.Fatal(err)
-	}
-	rp := NewReverseProxy(up, "/api/main_srv", "/v1", "")
-	req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, srv.URL+"/api/main_srv/health", nil)
-	if err != nil {
-		t.Fatal(err)
-	}
-	rec := httptest.NewRecorder()
-	rp.ServeHTTP(rec, req)
-	if rec.Code != http.StatusOK {
-		t.Fatalf("status %d", rec.Code)
-	}
-}
-
-func TestReverseProxyPathNestedStatsUsers(t *testing.T) {
-	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if r.URL.Path != "/api/v1/stats/users" {
-			t.Fatalf("path %q", r.URL.Path)
-		}
-		w.WriteHeader(http.StatusOK)
-	}))
-	defer srv.Close()
-	up, err := url.Parse(srv.URL + "/api/")
-	if err != nil {
-		t.Fatal(err)
-	}
-	rp := NewReverseProxy(up, "/api/gt2", "/v1", "")
-	req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, srv.URL+"/api/gt2/stats/users", nil)
-	if err != nil {
-		t.Fatal(err)
-	}
-	rec := httptest.NewRecorder()
-	rp.ServeHTTP(rec, req)
-	if rec.Code != http.StatusOK {
-		t.Fatalf("status %d", rec.Code)
 	}
 }