fix(internal/librarian): should not push to github when no push flag specified (#2405)

now when commit = true and push = false, it does not push to GitHub.
Updated test case to verify.

Fixes #2390

Author: zhumin8

internal/librarian/command.go

@@ -356,15 +356,15 @@ func commitAndPush(ctx context.Context, info *commitInfo) error {
 		return err
 	}
 
-	if err := repo.Push(branch); err != nil {
-		return err
-	}
-
 	if !info.push {
 		slog.Info("Push flag is not specified, skipping pull request creation")
 		return nil
 	}
 
+	if err := repo.Push(branch); err != nil {
+		return err
+	}
+
 	gitHubRepo, err := GetGitHubRepositoryFromGitRepo(info.repo)
 	if err != nil {
 		return fmt.Errorf("failed to get GitHub repository: %w", err)

internal/librarian/command_test.go

@@ -1301,21 +1301,25 @@ func TestCommitAndPush(t *testing.T) {
 		push              bool
 		wantErr           bool
 		expectedErrMsg    string
+		check             func(t *testing.T, repo gitrepo.Repository)
 	}{
 		{
 			name: "Push flag and Commit flag are not specified",
 			setupMockRepo: func(t *testing.T) gitrepo.Repository {
-				repoDir := newTestGitRepoWithCommit(t, "")
-				repo, err := gitrepo.NewRepository(&gitrepo.RepositoryOptions{Dir: repoDir})
-				if err != nil {
-					t.Fatalf("Failed to create test repo: %v", err)
+				return &MockRepository{
+					Dir: t.TempDir(),
 				}
-				return repo
 			},
 			setupMockClient: func(t *testing.T) GitHubClient {
 				return nil
 			},
 			prType: "generate",
+			check: func(t *testing.T, repo gitrepo.Repository) {
+				mockRepo := repo.(*MockRepository)
+				if mockRepo.PushCalls != 0 {
+					t.Errorf("Push was called %d times, expected 0", mockRepo.PushCalls)
+				}
+			},
 		},
 		{
 			name: "create a commit",
@@ -1339,6 +1343,12 @@ func TestCommitAndPush(t *testing.T) {
 			},
 			prType: "generate",
 			commit: true,
+			check: func(t *testing.T, repo gitrepo.Repository) {
+				mockRepo := repo.(*MockRepository)
+				if mockRepo.PushCalls != 0 {
+					t.Errorf("Push was called %d times, expected 0", mockRepo.PushCalls)
+				}
+			},
 		},
 		{
 			name: "create a generate pull request",
@@ -1635,6 +1645,10 @@ func TestCommitAndPush(t *testing.T) {
 				t.Errorf("%s: commitAndPush() returned unexpected error: %v", test.name, err)
 				return
 			}
+
+			if test.check != nil {
+				test.check(t, repo)
+			}
 		})
 	}
 }
@@ -1651,7 +1665,7 @@ func TestWritePRBody(t *testing.T) {
 				repo: &MockRepository{
 					Dir: t.TempDir(),
 					RemotesValue: []*gitrepo.Remote{
-						&gitrepo.Remote{
+						{
 							Name: "origin",
 							URLs: []string{"https://github.com/googleapis/librarian.git"},
 						},
@@ -1669,7 +1683,7 @@ func TestWritePRBody(t *testing.T) {
 				repo: &MockRepository{
 					Dir: t.TempDir(),
 					RemotesValue: []*gitrepo.Remote{
-						&gitrepo.Remote{
+						{
 							Name: "not-origin",
 							URLs: []string{"https://github.com/googleapis/librarian.git"},
 						},
@@ -1685,7 +1699,7 @@ func TestWritePRBody(t *testing.T) {
 				repo: &MockRepository{
 					Dir: t.TempDir(),
 					RemotesValue: []*gitrepo.Remote{
-						&gitrepo.Remote{
+						{
 							Name: "origin",
 							URLs: []string{"https://github.com/googleapis/librarian.git"},
 						},
@@ -1702,7 +1716,7 @@ func TestWritePRBody(t *testing.T) {
 					Dir:          t.TempDir(),
 					AddAllStatus: make(git.Status),
 					RemotesValue: []*gitrepo.Remote{
-						&gitrepo.Remote{
+						{
 							Name: "origin",
 							URLs: []string{"https://github.com/googleapis/librarian.git"},
 						},

internal/librarian/mocks_test.go

@@ -323,6 +323,7 @@ type MockRepository struct {
 	ChangedFilesInCommitValueByHash        map[string][]string
 	ChangedFilesInCommitError              error
 	CreateBranchAndCheckoutError           error
+	PushCalls                              int
 	PushError                              error
 	RestoreError                           error
 }
@@ -427,6 +428,7 @@ func (m *MockRepository) CreateBranchAndCheckout(name string) error {
 }
 
 func (m *MockRepository) Push(name string) error {
+	m.PushCalls++
 	if m.PushError != nil {
 		return m.PushError
 	}