Skip to content

Commit

Permalink
github sdk: add withClient options so we can override the default cli…
Browse files Browse the repository at this point in the history
…ent, useful for adding unit tests with httptest.NewServer

Signed-off-by: James Rawlings <[email protected]>
  • Loading branch information
rawlingsj committed Feb 3, 2025
1 parent 490dc20 commit a621fdb
Show file tree
Hide file tree
Showing 2 changed files with 127 additions and 1 deletion.
14 changes: 13 additions & 1 deletion modules/github-bots/sdk/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ func NewGitHubClient(ctx context.Context, org, repo, policyName string, opts ...
})

client := GitHubClient{
inner: github.NewClient(oauth2.NewClient(ctx, ts)),
ts: ts,
bufSize: 1024 * 1024, // 1MB buffer for requests
org: org,
Expand All @@ -51,6 +50,11 @@ func NewGitHubClient(ctx context.Context, org, repo, policyName string, opts ...
opt(&client)
}

// if client has not been configured withOpts then create a new client
if client.inner == nil {
client.inner = github.NewClient(oauth2.NewClient(ctx, ts))
}

return client
}

Expand Down Expand Up @@ -142,6 +146,14 @@ func WithBufferSize(bufSize int) GitHubClientOption {
}
}

// WithClient sets the inner GitHub client to the given client
// useful for testing
func WithClient(client *github.Client) GitHubClientOption {
return func(c *GitHubClient) {
c.inner = client
}
}

type GitHubClient struct {
inner *github.Client
ts oauth2.TokenSource
Expand Down
114 changes: 114 additions & 0 deletions modules/github-bots/sdk/github_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
package sdk

import (
"context"
"net/http"
"net/http/httptest"
"net/url"
"testing"

"github.com/google/go-github/v61/github"
)

func TestGitHubClientConfiguration(t *testing.T) {
tests := []struct {
name string
opts []GitHubClientOption
want func(*testing.T, GitHubClient)
}{
{
name: "default configuration without WithClient",
opts: nil,
want: func(t *testing.T, client GitHubClient) {
t.Helper()

// Verify a default client was created
if got := client.Client(); got == nil {
t.Error("expected default client to be created, got nil")
}

// Verify default buffer size
if got := client.bufSize; got != 1024*1024 {
t.Errorf("default bufSize = %v, want %v", got, 1024*1024)
}
},
},
{
name: "with custom HTTP client",
opts: []GitHubClientOption{
WithClient(github.NewClient(&http.Client{
Transport: &http.Transport{},
})),
},
want: func(t *testing.T, client GitHubClient) {
t.Helper()

// Verify client was set
if got := client.Client(); got == nil {
t.Error("expected client to be set, got nil")
}
},
},
{
name: "with test server",
opts: nil,
want: func(t *testing.T, origClient GitHubClient) {

Check failure on line 55 in modules/github-bots/sdk/github_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

unused-parameter: parameter 'origClient' seems to be unused, consider removing or renaming it as _ (revive)
t.Helper()

// Create test server
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Content-Type", "application/json")
w.Write([]byte(`{"status": "ok"}`))
}))
defer ts.Close()

// Parse the test server URL
baseURL, err := url.Parse(ts.URL + "/")
if err != nil {
t.Fatalf("failed to parse test server URL: %v", err)
}

// Create a client pointing to test server
httpClient := &http.Client{Transport: &http.Transport{}}
testClient := github.NewClient(httpClient)
testClient.BaseURL = baseURL

customClient := NewGitHubClient(
context.Background(),
"test-org",
"test-repo",
"test-policy",
WithClient(testClient),
)

// Client should now be configured to use the test server
if got := customClient.Client().BaseURL.String(); got != baseURL.String() {
t.Errorf("baseURL = %v, want %v", got, baseURL)
}
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
client := NewGitHubClient(
context.Background(),
"test-org",
"test-repo",
"test-policy",
tt.opts...,
)

// Verify org and repo are set correctly in all cases
if got := client.org; got != "test-org" {
t.Errorf("client.org = %v, want test-org", got)
}
if got := client.repo; got != "test-repo" {
t.Errorf("client.repo = %v, want test-repo", got)
}

// Run the specific test case assertions
tt.want(t, client)
})
}
}

0 comments on commit a621fdb

Please sign in to comment.