diff --git a/hack/tools/release/notes/github.go b/hack/tools/release/notes/github.go index 5911eca91829..3c4a8bacf011 100644 --- a/hack/tools/release/notes/github.go +++ b/hack/tools/release/notes/github.go @@ -35,6 +35,18 @@ type githubClient struct { repo string } +// 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/github_test.go b/hack/tools/release/notes/github_test.go new file mode 100644 index 000000000000..541560da562d --- /dev/null +++ b/hack/tools/release/notes/github_test.go @@ -0,0 +1,158 @@ +//go:build tools +// +build tools + +/* +Copyright 2025 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(_, _ string) (*githubDiff, error) { + if m.diffError != nil { + return nil, m.diffError + } + return m.diffResponse, nil +} + +func (m *mockGithubClient) getRef(_ string) (githubRef, error) { + if m.refError != nil { + return githubRef{}, m.refError + } + return m.refResponse, nil +} + +func (m *mockGithubClient) getTag(_ string) (githubTag, error) { + if m.tagError != nil { + return githubTag{}, m.tagError + } + return m.tagResponse, nil +} + +func (m *mockGithubClient) getCommit(_ string) (githubCommit, error) { + if m.commitError != nil { + return githubCommit{}, m.commitError + } + return m.commitResponse, nil +} + +func (m *mockGithubClient) listMergedPRs(_, _ time.Time, _ ...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 +} diff --git a/hack/tools/release/notes/list.go b/hack/tools/release/notes/list.go index 4cb5bcf1cbf2..03a2932f36b0 100644 --- 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 diff --git a/hack/tools/release/notes/list_test.go b/hack/tools/release/notes/list_test.go index 688abc9a1c0e..e5410e516ae6 100644 --- a/hack/tools/release/notes/list_test.go +++ b/hack/tools/release/notes/list_test.go @@ -20,11 +20,22 @@ limitations under the License. package main import ( + "fmt" "testing" . "github.com/onsi/gomega" ) +// newGithubFromToPRListerWithClient is a helper function for testing purposes. +// It creates a new githubFromToPRLister with the given client, fromRef, toRef and branch. +func newGithubFromToPRListerWithClient(client githubClientInterface, fromRef, toRef ref, branch string) *githubFromToPRLister { + return &githubFromToPRLister{ + client: client, + fromRef: fromRef, + toRef: toRef, + branch: branch, + } +} func Test_buildSetOfPRNumbers(t *testing.T) { tests := []struct { name string @@ -65,3 +76,121 @@ func Test_buildSetOfPRNumbers(t *testing.T) { }) } } + +func Test_githubFromToPRLister_listPRs(t *testing.T) { + tests := []struct { + name string + lister *githubFromToPRLister + args ref + want []pr + wantErr bool + }{ + { + name: "Successful PR Listing", + 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", + }, + want: []pr{ + { + number: 1234, + title: "Test PR", + labels: []string{"area/testing"}, + user: "testuser", + }, + }, + wantErr: false, + }, + { + name: "Setting previousReleaseRef.value blank - should use toRef and fromRef from fields", + lister: newGithubFromToPRListerWithClient( + newMockGithubClient(), + ref{reType: "tags", value: "v0.26.0"}, + ref{reType: "tags", value: "v0.27.0"}, + "main", + ), + args: ref{ + reType: "tags", + value: "", + }, + want: []pr{ + { + number: 1234, + title: "Test PR", + labels: []string{"area/testing"}, + user: "testuser", + }, + }, + wantErr: false, + }, + { + name: "Create PR List when fromRef is not set", + lister: newGithubFromToPRListerWithClient( + newMockGithubClient(), + ref{reType: "tags", value: ""}, + ref{reType: "tags", value: "v0.27.0"}, + "main", + ), + args: ref{ + reType: "tags", + value: "v0.26.0", + }, + want: []pr{ + { + number: 1234, + title: "Test PR", + labels: []string{"area/testing"}, + user: "testuser", + }, + }, + wantErr: false, + }, + { + name: "Fail when previousReleaseRef.value is set to invalid", + lister: newGithubFromToPRListerWithClient( + newMockGithubClientForInvalidRef(), + ref{reType: "tags", value: "v0.26.0"}, + ref{reType: "tags", value: "v0.27.0"}, + "main", + ), + args: ref{ + reType: "tags", + value: "invalid", + }, + want: nil, + wantErr: true, + }, + { + name: "Fail when toRef and previousReleaseRef set blank", + 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: "", + }, + want: nil, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + got, err := tt.lister.listPRs(tt.args) + if (err != nil) != tt.wantErr { + t.Errorf("githubFromToPRLister.listPRs() error = %v, wantErr %v", err, tt.wantErr) + return + } + g.Expect(got).To(Equal(tt.want)) + }) + } +} diff --git a/hack/tools/release/notes/process.go b/hack/tools/release/notes/process.go index 9bcd256bb4d5..d33864ee17b4 100644 --- a/hack/tools/release/notes/process.go +++ b/hack/tools/release/notes/process.go @@ -48,6 +48,7 @@ var ( "api": "API", "machinepool": "MachinePool", "clustercachetracker": "ClusterCacheTracker", + "clustercache": "ClusterCache", "clusterclass": "ClusterClass", "testing": "Testing", "release": "Release",