mock github and fix test cases

irapandey · irapandey · commit 833a8763feb5 · 2025-08-04T14:36:42.000+05:30
diff --git a/hack/tools/release/notes/github.go b/hack/tools/release/notes/github.go
@@ -35,6 +35,19 @@ type githubClient struct {
 	repo string
 }
 
+// githubClientInterface defines the interface for GitHub client operations
+// to allow for mocking in tests
+type githubClientInterface interface {
+	getDiffAllCommits(base, head string) (*githubDiff, error)
+	getRef(ref string) (githubRef, error)
+	getTag(tagSHA string) (githubTag, error)
+	getCommit(sha string) (githubCommit, error)
+	listMergedPRs(after, before time.Time, baseBranches ...string) ([]githubPR, error)
+}
+
+// Ensure githubClient implements githubClientInterface
+var _ githubClientInterface = (*githubClient)(nil)
+
 // githubDiff is the API response for the "compare" endpoint.
 type githubDiff struct {
 	// MergeBaseCommit points to most recent common ancestor between two references.
diff --git a/hack/tools/release/notes/list.go b/hack/tools/release/notes/list.go
@@ -30,7 +30,7 @@ import (
 
 // githubFromToPRLister lists PRs from GitHub contained between two refs.
 type githubFromToPRLister struct {
-	client         *githubClient
+	client         githubClientInterface
 	fromRef, toRef ref
 	// branch is optional. It helps optimize the PR query by restricting
 	// the results to PRs merged in the selected branch and in main
@@ -46,6 +46,17 @@ func newGithubFromToPRLister(repo string, fromRef, toRef ref, branch string) *gi
 	}
 }
 
+// newGithubFromToPRListerWithClient creates a new lister with a custom client
+// for testing purposes
+func newGithubFromToPRListerWithClient(client githubClientInterface, fromRef, toRef ref, branch string) *githubFromToPRLister {
+	return &githubFromToPRLister{
+		client:  client,
+		fromRef: fromRef,
+		toRef:   toRef,
+		branch:  branch,
+	}
+}
+
 // listPRs returns the PRs merged between `fromRef` and `toRef` (included).
 // It lists all PRs merged in main in the configured branch in the date
 // range between fromRef and toRef (we include main because minor releases
diff --git a/hack/tools/release/notes/list_test.go b/hack/tools/release/notes/list_test.go
@@ -20,6 +20,7 @@ limitations under the License.
 package main
 
 import (
+	"fmt"
 	"testing"
 
 	. "github.com/onsi/gomega"
@@ -69,26 +70,18 @@ func Test_buildSetOfPRNumbers(t *testing.T) {
 func Test_githubFromToPRLister_listPRs(t *testing.T) {
 	tests := []struct {
 		name    string
-		fields  *githubFromToPRLister
+		lister  *githubFromToPRLister
 		args    ref
 		wantErr bool
 	}{
 		{
 			name: "Successful PR Listing",
-			fields: &githubFromToPRLister{
-				client: &githubClient{
-					repo: "kubernetes-sigs/kind",
-				},
-				fromRef: ref{
-					reType: "tags",
-					value:  "v0.26.0",
-				},
-				toRef: ref{
-					reType: "tags",
-					value:  "v0.27.0",
-				},
-				branch: "main",
-			},
+			lister: newGithubFromToPRListerWithClient(
+				newMockGithubClient(),
+				ref{reType: "tags", value: "v0.26.0"},
+				ref{reType: "tags", value: "v0.27.0"},
+				"main",
+			),
 			args: ref{
 				reType: "tags",
 				value:  "v0.26.0",
@@ -97,20 +90,12 @@ func Test_githubFromToPRLister_listPRs(t *testing.T) {
 		},
 		{
 			name: "Setting previousReleaseRef.value blank - should use toRef and fromRef from fields",
-			fields: &githubFromToPRLister{
-				client: &githubClient{
-					repo: "kubernetes-sigs/kind",
-				},
-				fromRef: ref{
-					reType: "tags",
-					value:  "v0.26.0",
-				},
-				toRef: ref{
-					reType: "tags",
-					value:  "v0.27.0",
-				},
-				branch: "main",
-			},
+			lister: newGithubFromToPRListerWithClient(
+				newMockGithubClient(),
+				ref{reType: "tags", value: "v0.26.0"},
+				ref{reType: "tags", value: "v0.27.0"},
+				"main",
+			),
 			args: ref{
 				reType: "tags",
 				value:  "",
@@ -119,16 +104,12 @@ func Test_githubFromToPRLister_listPRs(t *testing.T) {
 		},
 		{
 			name: "Create PR List when fromRef is not set",
-			fields: &githubFromToPRLister{
-				client: &githubClient{
-					repo: "kubernetes-sigs/kind",
-				},
-				toRef: ref{
-					reType: "tags",
-					value:  "v0.27.0",
-				},
-				branch: "main",
-			},
+			lister: newGithubFromToPRListerWithClient(
+				newMockGithubClient(),
+				ref{reType: "tags", value: ""},
+				ref{reType: "tags", value: "v0.27.0"},
+				"main",
+			),
 			args: ref{
 				reType: "tags",
 				value:  "v0.26.0",
@@ -137,20 +118,12 @@ func Test_githubFromToPRLister_listPRs(t *testing.T) {
 		},
 		{
 			name: "Fail when previousReleaseRef.value is set to invalid",
-			fields: &githubFromToPRLister{
-				client: &githubClient{
-					repo: "kubernetes-sigs/kind",
-				},
-				fromRef: ref{
-					reType: "tags",
-					value:  "v0.26.0",
-				},
-				toRef: ref{
-					reType: "tags",
-					value:  "v0.27.0",
-				},
-				branch: "main",
-			},
+			lister: newGithubFromToPRListerWithClient(
+				newMockGithubClientForInvalidRef(),
+				ref{reType: "tags", value: "v0.26.0"},
+				ref{reType: "tags", value: "v0.27.0"},
+				"main",
+			),
 			args: ref{
 				reType: "tags",
 				value:  "invalid",
@@ -159,20 +132,12 @@ func Test_githubFromToPRLister_listPRs(t *testing.T) {
 		},
 		{
 			name: "Fail when toRef and previousReleaseRef set blank",
-			fields: &githubFromToPRLister{
-				client: &githubClient{
-					repo: "kubernetes-sigs/kind",
-				},
-				fromRef: ref{
-					reType: "tags",
-					value:  "v0.26.0",
-				},
-				toRef: ref{
-					reType: "tags",
-					value:  "",
-				},
-				branch: "main",
-			},
+			lister: newGithubFromToPRListerWithClient(
+				newMockGithubClientWithError("diff", fmt.Errorf("invalid ref")),
+				ref{reType: "tags", value: "v0.26.0"},
+				ref{reType: "tags", value: ""},
+				"main",
+			),
 			args: ref{
 				reType: "tags",
 				value:  "",
@@ -182,13 +147,7 @@ func Test_githubFromToPRLister_listPRs(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			l := &githubFromToPRLister{
-				client:  tt.fields.client,
-				fromRef: tt.fields.fromRef,
-				toRef:   tt.fields.toRef,
-				branch:  tt.fields.branch,
-			}
-			_, err := l.listPRs(tt.args)
+			_, err := tt.lister.listPRs(tt.args)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("githubFromToPRLister.listPRs() error = %v, wantErr %v", err, tt.wantErr)
 				return
diff --git a/hack/tools/release/notes/mock_github.go b/hack/tools/release/notes/mock_github.go
@@ -0,0 +1,158 @@
+//go:build tools
+// +build tools
+
+/*
+Copyright 2023 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package main
+
+import (
+	"fmt"
+	"time"
+)
+
+// mockGithubClient is a mock implementation of githubClientInterface for testing
+type mockGithubClient struct {
+	// Mock responses
+	diffResponse *githubDiff
+	refResponse  githubRef
+	tagResponse  githubTag
+	commitResponse githubCommit
+	prsResponse  []githubPR
+	
+	// Mock errors
+	diffError   error
+	refError    error
+	tagError    error
+	commitError error
+	prsError    error
+}
+
+// Ensure mockGithubClient implements githubClientInterface
+var _ githubClientInterface = (*mockGithubClient)(nil)
+
+func (m *mockGithubClient) getDiffAllCommits(base, head string) (*githubDiff, error) {
+	if m.diffError != nil {
+		return nil, m.diffError
+	}
+	return m.diffResponse, nil
+}
+
+func (m *mockGithubClient) getRef(ref string) (githubRef, error) {
+	if m.refError != nil {
+		return githubRef{}, m.refError
+	}
+	return m.refResponse, nil
+}
+
+func (m *mockGithubClient) getTag(tagSHA string) (githubTag, error) {
+	if m.tagError != nil {
+		return githubTag{}, m.tagError
+	}
+	return m.tagResponse, nil
+}
+
+func (m *mockGithubClient) getCommit(sha string) (githubCommit, error) {
+	if m.commitError != nil {
+		return githubCommit{}, m.commitError
+	}
+	return m.commitResponse, nil
+}
+
+func (m *mockGithubClient) listMergedPRs(after, before time.Time, baseBranches ...string) ([]githubPR, error) {
+	if m.prsError != nil {
+		return nil, m.prsError
+	}
+	return m.prsResponse, nil
+}
+
+// newMockGithubClient creates a new mock client with default responses
+func newMockGithubClient() *mockGithubClient {
+	return &mockGithubClient{
+		diffResponse: &githubDiff{
+			MergeBaseCommit: githubCommitNode{
+				Commit: githubCommit{
+					Message: "Merge commit",
+					Committer: githubCommitter{
+						Date: time.Date(2023, 1, 1, 0, 0, 0, 0, time.UTC),
+					},
+				},
+			},
+			Commits: []githubCommitNode{
+				{
+					Commit: githubCommit{
+						Message: "Merge pull request #1234 from test/branch",
+					},
+				},
+			},
+		},
+		refResponse: githubRef{
+			Object: githubObject{
+				ObjectType: commitType,
+				SHA:        "abc123",
+			},
+		},
+		tagResponse: githubTag{
+			Object: githubObject{
+				ObjectType: tagType,
+				SHA:        "def456",
+			},
+		},
+		commitResponse: githubCommit{
+			Message: "Test commit",
+			Committer: githubCommitter{
+				Date: time.Date(2023, 1, 2, 0, 0, 0, 0, time.UTC),
+			},
+		},
+		prsResponse: []githubPR{
+			{
+				Number: 1234,
+				Title:  "Test PR",
+				Labels: []githubLabel{
+					{Name: "area/testing"},
+				},
+				User: githubUser{
+					Login: "testuser",
+				},
+			},
+		},
+	}
+}
+
+// newMockGithubClientWithError creates a mock client that returns an error for specific operations
+func newMockGithubClientWithError(operation string, err error) *mockGithubClient {
+	mock := newMockGithubClient()
+	switch operation {
+	case "diff":
+		mock.diffError = err
+	case "ref":
+		mock.refError = err
+	case "tag":
+		mock.tagError = err
+	case "commit":
+		mock.commitError = err
+	case "prs":
+		mock.prsError = err
+	}
+	return mock
+}
+
+// newMockGithubClientForInvalidRef creates a mock client that simulates invalid ref scenarios
+func newMockGithubClientForInvalidRef() *mockGithubClient {
+	mock := newMockGithubClient()
+	mock.diffError = fmt.Errorf("invalid ref")
+	return mock
+} 
