review-bot/github/client_test.go

package github

import (
	"errors"
	"context"
	"net/http"
	"net/http/httptest"
	"testing"
	"time"
)

func TestNewClient_DefaultBaseURL(t *testing.T) {
	c := NewClient("tok", "")
	if c.baseURL != defaultBaseURL {
		t.Errorf("baseURL = %q, want %q", c.baseURL, defaultBaseURL)
	}
}

func TestNewClient_CustomBaseURL(t *testing.T) {
	c := NewClient("tok", "https://github.concur.com/api/v3/")
	if c.baseURL != "https://github.concur.com/api/v3" {
		t.Errorf("baseURL = %q, want trailing slash stripped", c.baseURL)
	}
}

func TestDoRequest_Success(t *testing.T) {
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		if got := r.Header.Get("Authorization"); got != "Bearer test-token" {
			t.Errorf("Authorization = %q, want Bearer test-token", got)
		}
		w.WriteHeader(http.StatusOK)
		w.Write([]byte(`{"ok":true}`))
	}))
	defer srv.Close()

	c := NewClient("test-token", srv.URL)
	body, err := c.doGet(context.Background(), srv.URL+"/test")
	if err != nil {
		t.Fatalf("unexpected error: %v", err)
	}
	if string(body) != `{"ok":true}` {
		t.Errorf("body = %q, want %q", body, `{"ok":true}`)
	}
}

func TestDoRequest_429_RetryAfter_IntegerSeconds(t *testing.T) {
	attempts := 0
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		attempts++
		if attempts == 1 {
			w.Header().Set("Retry-After", "3")
			w.WriteHeader(http.StatusTooManyRequests)
			w.Write([]byte("rate limited"))
			return
		}
		w.WriteHeader(http.StatusOK)
		w.Write([]byte("success"))
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL)
	// Use zero backoff so test doesn't wait — the Retry-After override only
	// affects backoff[attempt] which is used on the NEXT iteration. Since
	// we only have one retry, we set backoff[0] to 0 initially, then
	// the 429 handler overrides it. To avoid waiting, we cancel quickly.
	// Actually: the flow is attempt=0 gets 429, handler overrides backoff[0],
	// then attempt=1 reads backoff[0]. So we need backoff[0] to be small after override.
	// With Retry-After: 3, backoff[0] becomes 3s. Let's use context timeout.
	// Better approach: just set backoff large enough and use very short timeout.
	// Simplest: verify parsing works via parseRetryAfter unit tests and keep
	// the integration test fast by not actually waiting.

	// For integration: set backoff to 0 initially. The 429 handler will override
	// backoff[0] to 3s. To avoid waiting 3s, we'll just verify it retried.
	// Actually we need to accept the 3s wait OR use a different test strategy.

	// Best approach: use a 1ms initial backoff that gets overridden, but we
	// check correctness via the parseRetryAfter unit tests. For the integration
	// test, use Retry-After: 0 edge case OR just test that retry happens.
	c.SetRetryBackoff([]time.Duration{0, 0})

	// The handler sets Retry-After: 3, which will override backoff[0] to 3s.
	// But since we start with backoff[0]=0, the first attempt runs immediately,
	// then on 429 the code does: backoff[0] = 3s. The retry loop then uses
	// backoff[attempt-1] = backoff[0] = 3s for the delay before attempt 1.
	// To keep the test fast, let's just test a small value.
	srv.Close()

	// Recreate with small Retry-After
	attempts = 0
	srv2 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		attempts++
		if attempts == 1 {
			w.Header().Set("Retry-After", "1")
			w.WriteHeader(http.StatusTooManyRequests)
			w.Write([]byte("rate limited"))
			return
		}
		w.WriteHeader(http.StatusOK)
		w.Write([]byte("success"))
	}))
	defer srv2.Close()

	c2 := NewClient("tok", srv2.URL)
	c2.SetRetryBackoff([]time.Duration{0, 0})

	body, err := c2.doGet(context.Background(), srv2.URL+"/test")
	if err != nil {
		t.Fatalf("unexpected error: %v", err)
	}
	if string(body) != "success" {
		t.Errorf("body = %q, want %q", body, "success")
	}
	if attempts != 2 {
		t.Errorf("attempts = %d, want 2", attempts)
	}
}

func TestDoRequest_429_RetryAfter_HTTPDate(t *testing.T) {
	// Fix "now" to a known time for deterministic testing.
	fixedNow := time.Date(2025, 12, 1, 15, 59, 59, 0, time.UTC)
	retryAt := "Mon, 01 Dec 2025 16:00:00 GMT" // 1 second in the future

	attempts := 0
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		attempts++
		if attempts == 1 {
			w.Header().Set("Retry-After", retryAt)
			w.WriteHeader(http.StatusTooManyRequests)
			w.Write([]byte("rate limited"))
			return
		}
		w.WriteHeader(http.StatusOK)
		w.Write([]byte("success"))
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL)
	c.now = func() time.Time { return fixedNow }
	// Initial backoff is 0; the HTTP-date parser will compute 1s and override.
	c.SetRetryBackoff([]time.Duration{0, 0})

	body, err := c.doGet(context.Background(), srv.URL+"/test")
	if err != nil {
		t.Fatalf("unexpected error: %v", err)
	}
	if string(body) != "success" {
		t.Errorf("body = %q, want %q", body, "success")
	}
	if attempts != 2 {
		t.Errorf("attempts = %d, want 2", attempts)
	}
}

func TestDoRequest_429_RetryAfter_HTTPDate_InPast(t *testing.T) {
	// If the HTTP-date is in the past, delay should be 0 (retry immediately).
	fixedNow := time.Date(2025, 12, 1, 17, 0, 0, 0, time.UTC)
	retryAt := "Mon, 01 Dec 2025 16:00:00 GMT" // 1 hour in the past

	attempts := 0
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		attempts++
		if attempts == 1 {
			w.Header().Set("Retry-After", retryAt)
			w.WriteHeader(http.StatusTooManyRequests)
			w.Write([]byte("rate limited"))
			return
		}
		w.WriteHeader(http.StatusOK)
		w.Write([]byte("success"))
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL)
	c.now = func() time.Time { return fixedNow }
	c.SetRetryBackoff([]time.Duration{0, 0})

	body, err := c.doGet(context.Background(), srv.URL+"/test")
	if err != nil {
		t.Fatalf("unexpected error: %v", err)
	}
	if string(body) != "success" {
		t.Errorf("body = %q, want %q", body, "success")
	}
}

func TestDoRequest_429_NoRetryAfter_UsesDefaultBackoff(t *testing.T) {
	attempts := 0
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		attempts++
		if attempts == 1 {
			w.WriteHeader(http.StatusTooManyRequests)
			w.Write([]byte("rate limited"))
			return
		}
		w.WriteHeader(http.StatusOK)
		w.Write([]byte("success"))
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL)
	c.SetRetryBackoff([]time.Duration{0, 0})

	body, err := c.doGet(context.Background(), srv.URL+"/test")
	if err != nil {
		t.Fatalf("unexpected error: %v", err)
	}
	if string(body) != "success" {
		t.Errorf("body = %q, want %q", body, "success")
	}
	if attempts != 2 {
		t.Errorf("attempts = %d, want 2", attempts)
	}
}

func TestDoRequest_429_InvalidRetryAfter_UsesDefaultBackoff(t *testing.T) {
	attempts := 0
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		attempts++
		if attempts == 1 {
			w.Header().Set("Retry-After", "not-a-number-or-date")
			w.WriteHeader(http.StatusTooManyRequests)
			w.Write([]byte("rate limited"))
			return
		}
		w.WriteHeader(http.StatusOK)
		w.Write([]byte("success"))
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL)
	c.SetRetryBackoff([]time.Duration{0, 0})

	body, err := c.doGet(context.Background(), srv.URL+"/test")
	if err != nil {
		t.Fatalf("unexpected error: %v", err)
	}
	if string(body) != "success" {
		t.Errorf("body = %q, want %q", body, "success")
	}
}

func TestDoRequest_404_NoRetry(t *testing.T) {
	attempts := 0
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		attempts++
		w.WriteHeader(http.StatusNotFound)
		w.Write([]byte("not found"))
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL)
	_, err := c.doGet(context.Background(), srv.URL+"/test")
	if err == nil {
		t.Fatal("expected error, got nil")
	}
	if !IsNotFound(err) {
		t.Errorf("expected IsNotFound, got %v", err)
	}
	if attempts != 1 {
		t.Errorf("attempts = %d, want 1 (no retry on 404)", attempts)
	}
}

func TestDoRequest_401_NoRetry(t *testing.T) {
	attempts := 0
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		attempts++
		w.WriteHeader(http.StatusUnauthorized)
		w.Write([]byte("unauthorized"))
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL)
	_, err := c.doGet(context.Background(), srv.URL+"/test")
	if err == nil {
		t.Fatal("expected error, got nil")
	}
	if !IsUnauthorized(err) {
		t.Errorf("expected IsUnauthorized, got %v", err)
	}
	if attempts != 1 {
		t.Errorf("attempts = %d, want 1 (no retry on 401)", attempts)
	}
}

func TestDoRequest_ContextCanceled(t *testing.T) {
	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("Retry-After", "10")
		w.WriteHeader(http.StatusTooManyRequests)
	}))
	defer srv.Close()

	c := NewClient("tok", srv.URL)
	c.SetRetryBackoff([]time.Duration{5 * time.Second, 5 * time.Second})

	ctx, cancel := context.WithCancel(context.Background())
	// Cancel immediately so the retry timer is interrupted.
	cancel()

	_, err := c.doGet(ctx, srv.URL+"/test")
	if err == nil {
		t.Fatal("expected error, got nil")
	}
	if !errors.Is(err, context.Canceled) {
		t.Errorf("err = %v, want context.Canceled", err)
	}
}

func TestParseRetryAfter_IntegerSeconds(t *testing.T) {
	c := NewClient("tok", "")
	delay, ok := c.parseRetryAfter("42")
	if !ok {
		t.Fatal("expected ok=true")
	}
	if delay != 42*time.Second {
		t.Errorf("delay = %v, want 42s", delay)
	}
}

func TestParseRetryAfter_ZeroSeconds(t *testing.T) {
	c := NewClient("tok", "")
	_, ok := c.parseRetryAfter("0")
	if ok {
		t.Error("expected ok=false for zero seconds")
	}
}

func TestParseRetryAfter_NegativeSeconds(t *testing.T) {
	c := NewClient("tok", "")
	_, ok := c.parseRetryAfter("-5")
	if ok {
		t.Error("expected ok=false for negative seconds")
	}
}

func TestParseRetryAfter_HTTPDate_Future(t *testing.T) {
	fixedNow := time.Date(2025, 12, 1, 15, 59, 50, 0, time.UTC)
	c := NewClient("tok", "")
	c.now = func() time.Time { return fixedNow }

	delay, ok := c.parseRetryAfter("Mon, 01 Dec 2025 16:00:00 GMT")
	if !ok {
		t.Fatal("expected ok=true")
	}
	// Should be 10 seconds in the future.
	if delay != 10*time.Second {
		t.Errorf("delay = %v, want 10s", delay)
	}
}

func TestParseRetryAfter_HTTPDate_Past(t *testing.T) {
	fixedNow := time.Date(2025, 12, 1, 17, 0, 0, 0, time.UTC)
	c := NewClient("tok", "")
	c.now = func() time.Time { return fixedNow }

	delay, ok := c.parseRetryAfter("Mon, 01 Dec 2025 16:00:00 GMT")
	if !ok {
		t.Fatal("expected ok=true")
	}
	if delay != 0 {
		t.Errorf("delay = %v, want 0 (past date)", delay)
	}
}

func TestParseRetryAfter_RFC850_Format(t *testing.T) {
	fixedNow := time.Date(2025, 12, 1, 15, 59, 50, 0, time.UTC)
	c := NewClient("tok", "")
	c.now = func() time.Time { return fixedNow }

	// RFC 850 format
	delay, ok := c.parseRetryAfter("Monday, 01-Dec-25 16:00:00 GMT")
	if !ok {
		t.Fatal("expected ok=true for RFC 850 format")
	}
	if delay != 10*time.Second {
		t.Errorf("delay = %v, want 10s", delay)
	}
}

func TestParseRetryAfter_Invalid(t *testing.T) {
	c := NewClient("tok", "")
	_, ok := c.parseRetryAfter("not-valid")
	if ok {
		t.Error("expected ok=false for invalid value")
	}
}

func TestParseRetryAfter_EmptyString(t *testing.T) {
	c := NewClient("tok", "")
	_, ok := c.parseRetryAfter("")
	if ok {
		t.Error("expected ok=false for empty string")
	}
}

func TestParseRetryAfter_MaxCap(t *testing.T) {
	// Verify that parseRetryAfter returns the raw value (capping is done by caller).
	c := NewClient("tok", "")
	delay, ok := c.parseRetryAfter("3600")
	if !ok {
		t.Fatal("expected ok=true")
	}
	if delay != 3600*time.Second {
		t.Errorf("delay = %v, want 3600s (caller is responsible for capping)", delay)
	}
}

func TestAPIError_Error_Truncation(t *testing.T) {
	longBody := make([]byte, 300)
	for i := range longBody {
		longBody[i] = 'x'
	}
	apiErr := &APIError{StatusCode: 500, Body: string(longBody)}
	msg := apiErr.Error()
	if len(msg) > 250 {
		// "HTTP 500: " (10) + 200 + "...(truncated)" (14) = 224
		t.Errorf("error message too long: %d chars", len(msg))
	}
}

func TestAPIError_Error_NewlineSanitized(t *testing.T) {
	apiErr := &APIError{StatusCode: 400, Body: "line1\nline2\rline3"}
	msg := apiErr.Error()
	for _, c := range msg {
		if c == '\n' || c == '\r' {
			t.Errorf("error message contains unsanitized newline: %q", msg)
			break
		}
	}
}