From 16f7d24ea6b228014618628fbcdd5d9638f7a7ba Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Feb 2026 20:53:47 +0000 Subject: [PATCH 1/3] test(interceptor): add comprehensive tests for interceptor chain builder Expand test coverage from 4 basic tests to 19 tests across 5 categories: - Options white-box tests (config application, independence, last-wins) - Builder black-box tests (table-driven with various option combos) - Ordering tests via reflect (verify exact interceptor sequence) - Behavioral integration tests (panic recovery, request ID propagation, deadline enforcement, error mapping, success path) - Options integration tests (custom deadline/requestID configs, order independence) https://claude.ai/code/session_011YbD59ctwdD2jMEbvBBTBN --- .../interceptor/interceptor_test.go | 553 +++++++++++++++++- 1 file changed, 548 insertions(+), 5 deletions(-) diff --git a/pkg/connectrpc/interceptor/interceptor_test.go b/pkg/connectrpc/interceptor/interceptor_test.go index 9e034ee..c7ec769 100644 --- a/pkg/connectrpc/interceptor/interceptor_test.go +++ b/pkg/connectrpc/interceptor/interceptor_test.go @@ -1,13 +1,151 @@ package interceptor import ( + "context" + "errors" + "net/http" + "reflect" + "strings" "testing" + "time" + + "connectrpc.com/connect" "github.com/deepworx/go-utils/pkg/connectrpc/deadline" "github.com/deepworx/go-utils/pkg/connectrpc/jwtauth" "github.com/deepworx/go-utils/pkg/connectrpc/requestid" + "github.com/deepworx/go-utils/pkg/ctxutil" ) +// --------------------------------------------------------------------------- +// Mock types +// --------------------------------------------------------------------------- + +type mockRequest struct { + connect.AnyRequest + procedure string + headers http.Header + isClient bool +} + +func (r *mockRequest) Any() any { return nil } +func (r *mockRequest) Spec() connect.Spec { return connect.Spec{Procedure: r.procedure, IsClient: r.isClient} } +func (r *mockRequest) Header() http.Header { return r.headers } +func (r *mockRequest) Peer() connect.Peer { return connect.Peer{} } + +type mockResponse struct { + connect.AnyResponse +} + +func (r *mockResponse) Any() any { return nil } +func (r *mockResponse) Header() http.Header { return http.Header{} } +func (r *mockResponse) Trailer() http.Header { return http.Header{} } + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +// interceptorPkgName extracts the short package name from a connect.Interceptor +// using reflection (e.g. "*recovery.interceptor" → "recovery"). +func interceptorPkgName(ic connect.Interceptor) string { + t := reflect.TypeOf(ic) + if t.Kind() == reflect.Ptr { + t = t.Elem() + } + parts := strings.Split(t.PkgPath(), "/") + return parts[len(parts)-1] +} + +// buildUnaryChain mimics how connect-go applies interceptors: +// the first interceptor in the slice is the outermost wrapper. +func buildUnaryChain(interceptors []connect.Interceptor, handler connect.UnaryFunc) connect.UnaryFunc { + for i := len(interceptors) - 1; i >= 0; i-- { + handler = interceptors[i].WrapUnary(handler) + } + return handler +} + +// --------------------------------------------------------------------------- +// A. Options white-box tests +// --------------------------------------------------------------------------- + +func TestWithDeadline_AppliesConfig(t *testing.T) { + t.Parallel() + + cfg := deadline.Config{ + DefaultTimeout: 45 * time.Second, + MaxTimeout: 180 * time.Second, + } + + o := &Options{} + WithDeadline(cfg)(o) + + if o.deadlineCfg == nil { + t.Fatal("deadlineCfg should not be nil after applying WithDeadline") + } + if o.deadlineCfg.DefaultTimeout != 45*time.Second { + t.Errorf("DefaultTimeout = %v, want %v", o.deadlineCfg.DefaultTimeout, 45*time.Second) + } + if o.deadlineCfg.MaxTimeout != 180*time.Second { + t.Errorf("MaxTimeout = %v, want %v", o.deadlineCfg.MaxTimeout, 180*time.Second) + } +} + +func TestWithRequestID_AppliesConfig(t *testing.T) { + t.Parallel() + + cfg := requestid.Config{HeaderName: "X-Correlation-ID"} + + o := &Options{} + WithRequestID(cfg)(o) + + if o.requestIDCfg == nil { + t.Fatal("requestIDCfg should not be nil after applying WithRequestID") + } + if o.requestIDCfg.HeaderName != "X-Correlation-ID" { + t.Errorf("HeaderName = %q, want %q", o.requestIDCfg.HeaderName, "X-Correlation-ID") + } +} + +func TestOptions_IndependentApplication(t *testing.T) { + t.Parallel() + + o := &Options{} + WithDeadline(deadline.Config{DefaultTimeout: 10 * time.Second, MaxTimeout: 60 * time.Second})(o) + + if o.requestIDCfg != nil { + t.Error("requestIDCfg should be nil when only WithDeadline is applied") + } + + WithRequestID(requestid.Config{HeaderName: "X-Custom"})(o) + + if o.deadlineCfg == nil { + t.Error("deadlineCfg should still be set after applying WithRequestID") + } + if o.requestIDCfg == nil { + t.Error("requestIDCfg should not be nil after applying WithRequestID") + } +} + +func TestOptions_LastAppliedWins(t *testing.T) { + t.Parallel() + + o := &Options{} + WithDeadline(deadline.Config{DefaultTimeout: 10 * time.Second, MaxTimeout: 60 * time.Second})(o) + WithDeadline(deadline.Config{DefaultTimeout: 45 * time.Second, MaxTimeout: 180 * time.Second})(o) + + if o.deadlineCfg.DefaultTimeout != 45*time.Second { + t.Errorf("DefaultTimeout = %v, want %v (last option should win)", o.deadlineCfg.DefaultTimeout, 45*time.Second) + } + if o.deadlineCfg.MaxTimeout != 180*time.Second { + t.Errorf("MaxTimeout = %v, want %v (last option should win)", o.deadlineCfg.MaxTimeout, 180*time.Second) + } +} + +// --------------------------------------------------------------------------- +// B. Builder tests (table-driven, black-box) +// --------------------------------------------------------------------------- + func TestBuildDefault(t *testing.T) { t.Parallel() @@ -17,7 +155,7 @@ func TestBuildDefault(t *testing.T) { wantCount int }{ { - name: "default config", + name: "no options", opts: nil, wantCount: 7, }, @@ -53,6 +191,11 @@ func TestBuildDefault(t *testing.T) { }, wantCount: 7, }, + { + name: "empty options slice", + opts: []Option{}, + wantCount: 7, + }, } for _, tt := range tests { @@ -76,15 +219,14 @@ func TestBuildDefaultWithAuth_NilAuth(t *testing.T) { if err == nil { t.Fatal("BuildDefaultWithAuth(nil) should return error") } + if !strings.Contains(err.Error(), "authenticator is required") { + t.Errorf("error = %q, want message containing %q", err.Error(), "authenticator is required") + } } func TestBuildDefaultWithAuth_ValidAuth(t *testing.T) { t.Parallel() - // Create a mock authenticator for testing - // Since we can't easily create a real Authenticator without JWKS, - // we test that the function signature works with a non-nil pointer. - // In integration tests, a real authenticator would be used. auth := &jwtauth.Authenticator{} interceptors, err := BuildDefaultWithAuth(auth) @@ -117,3 +259,404 @@ func TestBuildDefaultWithAuth_WithOptions(t *testing.T) { t.Errorf("BuildDefaultWithAuth() returned %d interceptors, want 8", len(interceptors)) } } + +// --------------------------------------------------------------------------- +// C. Ordering tests (reflect-based) +// --------------------------------------------------------------------------- + +func TestBuildDefault_InterceptorOrder(t *testing.T) { + t.Parallel() + + interceptors, err := BuildDefault() + if err != nil { + t.Fatalf("BuildDefault() error = %v", err) + } + + expectedOrder := []string{ + "recovery", + "deadline", + "requestid", + "otelconnect", + "logging", + "validate", + "errors", + } + + if len(interceptors) != len(expectedOrder) { + t.Fatalf("got %d interceptors, want %d", len(interceptors), len(expectedOrder)) + } + + for i, ic := range interceptors { + pkg := interceptorPkgName(ic) + if pkg != expectedOrder[i] { + t.Errorf("interceptor[%d] package = %q, want %q", i, pkg, expectedOrder[i]) + } + } +} + +func TestBuildDefaultWithAuth_InterceptorOrder(t *testing.T) { + t.Parallel() + + auth := &jwtauth.Authenticator{} + interceptors, err := BuildDefaultWithAuth(auth) + if err != nil { + t.Fatalf("BuildDefaultWithAuth() error = %v", err) + } + + expectedOrder := []string{ + "recovery", + "deadline", + "requestid", + "otelconnect", + "logging", + "jwtauth", + "validate", + "errors", + } + + if len(interceptors) != len(expectedOrder) { + t.Fatalf("got %d interceptors, want %d", len(interceptors), len(expectedOrder)) + } + + for i, ic := range interceptors { + pkg := interceptorPkgName(ic) + if pkg != expectedOrder[i] { + t.Errorf("interceptor[%d] package = %q, want %q", i, pkg, expectedOrder[i]) + } + } +} + +// --------------------------------------------------------------------------- +// D. Behavioral integration tests +// --------------------------------------------------------------------------- + +func TestChain_RecoveryCatchesPanics(t *testing.T) { + t.Parallel() + + interceptors, err := BuildDefault() + if err != nil { + t.Fatalf("BuildDefault() error = %v", err) + } + + chain := buildUnaryChain(interceptors, func(_ context.Context, _ connect.AnyRequest) (connect.AnyResponse, error) { + panic("handler panic") + }) + + req := &mockRequest{ + procedure: "/test.Service/Method", + headers: http.Header{}, + } + + _, err = chain(context.Background(), req) + + var connectErr *connect.Error + if !errors.As(err, &connectErr) { + t.Fatalf("expected connect.Error, got %T: %v", err, err) + } + if connectErr.Code() != connect.CodeInternal { + t.Errorf("code = %v, want CodeInternal (recovery should catch panics)", connectErr.Code()) + } +} + +func TestChain_RequestIDPropagation(t *testing.T) { + t.Parallel() + + interceptors, err := BuildDefault() + if err != nil { + t.Fatalf("BuildDefault() error = %v", err) + } + + var capturedID string + chain := buildUnaryChain(interceptors, func(ctx context.Context, _ connect.AnyRequest) (connect.AnyResponse, error) { + if id, ok := ctxutil.RequestID(ctx); ok { + capturedID = id + } + return &mockResponse{}, nil + }) + + headers := http.Header{} + headers.Set("X-Request-ID", "test-correlation-123") + req := &mockRequest{ + procedure: "/test.Service/Method", + headers: headers, + } + + _, err = chain(context.Background(), req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if capturedID != "test-correlation-123" { + t.Errorf("capturedID = %q, want %q", capturedID, "test-correlation-123") + } +} + +func TestChain_RequestIDGeneration(t *testing.T) { + t.Parallel() + + interceptors, err := BuildDefault() + if err != nil { + t.Fatalf("BuildDefault() error = %v", err) + } + + var capturedID string + chain := buildUnaryChain(interceptors, func(ctx context.Context, _ connect.AnyRequest) (connect.AnyResponse, error) { + if id, ok := ctxutil.RequestID(ctx); ok { + capturedID = id + } + return &mockResponse{}, nil + }) + + req := &mockRequest{ + procedure: "/test.Service/Method", + headers: http.Header{}, + } + + _, err = chain(context.Background(), req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if capturedID == "" { + t.Fatal("expected request ID to be generated when no header is present") + } + if len(capturedID) != 32 { + t.Errorf("generated request ID length = %d, want 32 (UUID hex)", len(capturedID)) + } +} + +func TestChain_DeadlineApplied(t *testing.T) { + t.Parallel() + + interceptors, err := BuildDefault() + if err != nil { + t.Fatalf("BuildDefault() error = %v", err) + } + + var hasDeadline bool + chain := buildUnaryChain(interceptors, func(ctx context.Context, _ connect.AnyRequest) (connect.AnyResponse, error) { + _, hasDeadline = ctx.Deadline() + return &mockResponse{}, nil + }) + + req := &mockRequest{ + procedure: "/test.Service/Method", + headers: http.Header{}, + } + + // Use a context without deadline to verify the interceptor adds one. + _, err = chain(context.Background(), req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !hasDeadline { + t.Error("expected context to have a deadline after passing through the chain") + } +} + +func TestChain_ErrorMapping(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + handlerErr error + wantCode connect.Code + wantMessage string + }{ + { + name: "raw error mapped to CodeInternal", + handlerErr: errors.New("database connection failed"), + wantCode: connect.CodeInternal, + wantMessage: "internal error", + }, + { + name: "context.Canceled mapped to CodeCanceled", + handlerErr: context.Canceled, + wantCode: connect.CodeCanceled, + wantMessage: "context canceled", + }, + { + name: "context.DeadlineExceeded mapped to CodeDeadlineExceeded", + handlerErr: context.DeadlineExceeded, + wantCode: connect.CodeDeadlineExceeded, + wantMessage: "context deadline exceeded", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + interceptors, err := BuildDefault() + if err != nil { + t.Fatalf("BuildDefault() error = %v", err) + } + + chain := buildUnaryChain(interceptors, func(_ context.Context, _ connect.AnyRequest) (connect.AnyResponse, error) { + return nil, tt.handlerErr + }) + + req := &mockRequest{ + procedure: "/test.Service/Method", + headers: http.Header{}, + } + + _, err = chain(context.Background(), req) + + var connectErr *connect.Error + if !errors.As(err, &connectErr) { + t.Fatalf("expected connect.Error, got %T: %v", err, err) + } + if connectErr.Code() != tt.wantCode { + t.Errorf("code = %v, want %v", connectErr.Code(), tt.wantCode) + } + if connectErr.Message() != tt.wantMessage { + t.Errorf("message = %q, want %q", connectErr.Message(), tt.wantMessage) + } + }) + } +} + +func TestChain_SuccessPath(t *testing.T) { + t.Parallel() + + interceptors, err := BuildDefault() + if err != nil { + t.Fatalf("BuildDefault() error = %v", err) + } + + handlerCalled := false + chain := buildUnaryChain(interceptors, func(_ context.Context, _ connect.AnyRequest) (connect.AnyResponse, error) { + handlerCalled = true + return &mockResponse{}, nil + }) + + req := &mockRequest{ + procedure: "/test.Service/Method", + headers: http.Header{}, + } + + resp, err := chain(context.Background(), req) + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp == nil { + t.Error("expected non-nil response") + } + if !handlerCalled { + t.Error("expected handler to be called") + } +} + +// --------------------------------------------------------------------------- +// E. Options integration tests +// --------------------------------------------------------------------------- + +func TestChain_CustomDeadlineConfig(t *testing.T) { + t.Parallel() + + customTimeout := 5 * time.Second + interceptors, err := BuildDefault(WithDeadline(deadline.Config{ + DefaultTimeout: customTimeout, + MaxTimeout: 10 * time.Second, + })) + if err != nil { + t.Fatalf("BuildDefault() error = %v", err) + } + + var contextDeadline time.Time + var hasDeadline bool + chain := buildUnaryChain(interceptors, func(ctx context.Context, _ connect.AnyRequest) (connect.AnyResponse, error) { + contextDeadline, hasDeadline = ctx.Deadline() + return &mockResponse{}, nil + }) + + req := &mockRequest{ + procedure: "/test.Service/Method", + headers: http.Header{}, + } + + before := time.Now() + _, err = chain(context.Background(), req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !hasDeadline { + t.Fatal("expected context to have a deadline") + } + + maxExpected := before.Add(customTimeout + time.Second) // allow 1s tolerance + if contextDeadline.After(maxExpected) { + t.Errorf("context deadline = %v, should be within ~%v of %v", contextDeadline, customTimeout, before) + } + if contextDeadline.Before(before) { + t.Errorf("context deadline = %v, should be after %v", contextDeadline, before) + } +} + +func TestChain_CustomRequestIDHeader(t *testing.T) { + t.Parallel() + + interceptors, err := BuildDefault(WithRequestID(requestid.Config{ + HeaderName: "X-Correlation-ID", + })) + if err != nil { + t.Fatalf("BuildDefault() error = %v", err) + } + + var capturedID string + chain := buildUnaryChain(interceptors, func(ctx context.Context, _ connect.AnyRequest) (connect.AnyResponse, error) { + if id, ok := ctxutil.RequestID(ctx); ok { + capturedID = id + } + return &mockResponse{}, nil + }) + + headers := http.Header{} + headers.Set("X-Correlation-ID", "custom-correlation-456") + req := &mockRequest{ + procedure: "/test.Service/Method", + headers: headers, + } + + _, err = chain(context.Background(), req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if capturedID != "custom-correlation-456" { + t.Errorf("capturedID = %q, want %q", capturedID, "custom-correlation-456") + } +} + +func TestBuildDefault_OptionsOrderIndependence(t *testing.T) { + t.Parallel() + + deadlineCfg := deadline.Config{DefaultTimeout: 45 * time.Second, MaxTimeout: 180 * time.Second} + requestIDCfg := requestid.Config{HeaderName: "X-Custom"} + + interceptorsAB, err := BuildDefault(WithDeadline(deadlineCfg), WithRequestID(requestIDCfg)) + if err != nil { + t.Fatalf("BuildDefault(deadline, requestid) error = %v", err) + } + + interceptorsBA, err := BuildDefault(WithRequestID(requestIDCfg), WithDeadline(deadlineCfg)) + if err != nil { + t.Fatalf("BuildDefault(requestid, deadline) error = %v", err) + } + + if len(interceptorsAB) != len(interceptorsBA) { + t.Fatalf("option order changed interceptor count: %d vs %d", len(interceptorsAB), len(interceptorsBA)) + } + + for i := range interceptorsAB { + pkgAB := interceptorPkgName(interceptorsAB[i]) + pkgBA := interceptorPkgName(interceptorsBA[i]) + if pkgAB != pkgBA { + t.Errorf("interceptor[%d] type differs: %q vs %q", i, pkgAB, pkgBA) + } + } +} From d203435a5ccf52bdcbe191936cbbb9480955a500 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Feb 2026 21:39:49 +0000 Subject: [PATCH 2/3] ci: add dorny/test-reporter for Go test results on PRs Add dorny/test-reporter@v2 to display individual test results as a GitHub check run. Tests now output JSON format (-json flag) which the reporter parses. Both the test report and coverage artifact upload use if: always() to run even when tests fail. https://claude.ai/code/session_011YbD59ctwdD2jMEbvBBTBN --- .github/workflows/ci.yml | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 92dfa0a..1f95a2e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,6 +11,10 @@ jobs: test: name: Test runs-on: ubuntu-latest + permissions: + contents: read + actions: read + checks: write steps: - uses: actions/checkout@v6 @@ -19,9 +23,18 @@ jobs: go-version: "1.25" - name: Run tests with coverage - run: go test -race -coverprofile=coverage.txt ./... + run: go test -json -race -cover -coverprofile=coverage.txt ./... > test-results.json + + - name: Test report + if: always() + uses: dorny/test-reporter@v2 + with: + name: Go Test Results + path: test-results.json + reporter: golang-json - name: Archive coverage results + if: always() uses: actions/upload-artifact@v6 with: name: code-coverage From 7a9a9892444d3884ec0ee56faace115201a0d820 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Feb 2026 21:43:36 +0000 Subject: [PATCH 3/3] fix(ci): pin fgrosse/go-coverage-report to exact tag v1.2.0 The floating tag v1.2 does not exist; use the full semver tag v1.2.0. https://claude.ai/code/session_011YbD59ctwdD2jMEbvBBTBN --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1f95a2e..a44fa14 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,7 +50,7 @@ jobs: actions: read pull-requests: write steps: - - uses: fgrosse/go-coverage-report@v1.2 + - uses: fgrosse/go-coverage-report@v1.2.0 with: coverage-artifact-name: code-coverage coverage-file-name: coverage.txt