From 75ccec3923cd8473bfea1aad43a7e62a41dafbd5 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Thu, 16 Oct 2025 02:48:06 +0000 Subject: [PATCH 1/5] fix: populate configure command pr content --- internal/librarian/generate_command.go | 8 +++-- internal/librarian/generate_command_test.go | 34 +++++++++++++++++++-- internal/librarian/mocks_test.go | 2 ++ 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/internal/librarian/generate_command.go b/internal/librarian/generate_command.go index f617f3ef25..74961d07e7 100644 --- a/internal/librarian/generate_command.go +++ b/internal/librarian/generate_command.go @@ -176,10 +176,14 @@ func (r *generateRunner) run(ctx context.Context) error { return fmt.Errorf("unexpected prType %s", prType) } + commitMessage := "feat: generate libraries" + if prType == pullRequestOnboard { + commitMessage = "feat: onboard a new library" + } commitInfo := &commitInfo{ branch: r.branch, commit: r.commit, - commitMessage: "feat: generate libraries", + commitMessage: commitMessage, ghClient: r.ghClient, prType: prType, push: r.push, @@ -221,7 +225,7 @@ func (r *generateRunner) generateSingleLibrary(ctx context.Context, libraryID, o return nil, err } - prType = pullRequestGenerate + prType = pullRequestOnboard libraryID = configuredLibraryID } diff --git a/internal/librarian/generate_command_test.go b/internal/librarian/generate_command_test.go index b7ff897523..d1656a898e 100644 --- a/internal/librarian/generate_command_test.go +++ b/internal/librarian/generate_command_test.go @@ -837,7 +837,21 @@ func TestGenerateScenarios(t *testing.T) { }, } { t.Run(test.name, func(t *testing.T) { - repo := newTestGitRepoWithState(t, test.state, true) + var repo gitrepo.Repository + useMockRepo := test.name == "generate_single_library_including_initial_configuration" || test.name == "generate single existing library by library id" + if useMockRepo { + repo = &MockRepository{ + Dir: t.TempDir(), + RemotesValue: []*gitrepo.Remote{ + { + Name: "origin", + URLs: []string{"https://github.com/googleapis/librarian.git"}, + }, + }, + } + } else { + repo = newTestGitRepoWithState(t, test.state, true) + } r := &generateRunner{ api: test.api, @@ -848,9 +862,13 @@ func TestGenerateScenarios(t *testing.T) { state: test.state, librarianConfig: test.librarianConfig, containerClient: test.container, - ghClient: test.ghClient, + ghClient: &mockGitHubClient{}, workRoot: t.TempDir(), } + if useMockRepo { + r.commit = true + r.push = true + } // Create a service config in api path. if err := os.MkdirAll(filepath.Join(r.sourceRepo.GetDir(), test.api), 0755); err != nil { @@ -906,6 +924,18 @@ func TestGenerateScenarios(t *testing.T) { if diff := cmp.Diff(test.wantConfigureCalls, test.container.configureCalls); diff != "" { t.Errorf("%s: run() configureCalls mismatch (-want +got):%s", test.name, diff) } + if test.name == "generate_single_library_including_initial_configuration" { + mockRepo := repo.(*MockRepository) + if mockRepo.LastCommitMessage != "feat: onboard a new library" { + t.Errorf("want commit message %q, got %q", "feat: onboard a new library", mockRepo.LastCommitMessage) + } + } + if test.name == "generate single existing library by library id" { + mockRepo := repo.(*MockRepository) + if mockRepo.LastCommitMessage != "feat: generate libraries" { + t.Errorf("want commit message %q, got %q", "feat: generate libraries", mockRepo.LastCommitMessage) + } + } }) } } diff --git a/internal/librarian/mocks_test.go b/internal/librarian/mocks_test.go index 7196dea928..489c6a589b 100644 --- a/internal/librarian/mocks_test.go +++ b/internal/librarian/mocks_test.go @@ -308,6 +308,7 @@ type MockRepository struct { RemotesValue []*gitrepo.Remote RemotesError error CommitCalls int + LastCommitMessage string GetCommitError error GetLatestCommitError error GetCommitByHash map[string]*gitrepo.Commit @@ -355,6 +356,7 @@ func (m *MockRepository) AddAll() error { func (m *MockRepository) Commit(msg string) error { m.CommitCalls++ + m.LastCommitMessage = msg return m.CommitError } From e3c7b8e1f8e76288ece43a108e169dd0b53b1ff7 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Thu, 16 Oct 2025 03:06:26 +0000 Subject: [PATCH 2/5] address PR feedback --- internal/librarian/generate_command_test.go | 23 ++++++++++----------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/internal/librarian/generate_command_test.go b/internal/librarian/generate_command_test.go index d1656a898e..3a571af289 100644 --- a/internal/librarian/generate_command_test.go +++ b/internal/librarian/generate_command_test.go @@ -404,11 +404,13 @@ func TestGenerateScenarios(t *testing.T) { container *mockContainerClient ghClient GitHubClient build bool + useMockRepo bool wantErr bool wantErrMsg string wantGenerateCalls int wantBuildCalls int wantConfigureCalls int + wantCommitMessage string }{ { name: "generate_single_library_including_initial_configuration", @@ -425,9 +427,11 @@ func TestGenerateScenarios(t *testing.T) { }, ghClient: &mockGitHubClient{}, build: true, + useMockRepo: true, wantGenerateCalls: 1, wantBuildCalls: 1, wantConfigureCalls: 1, + wantCommitMessage: "feat: onboard a new library", }, { name: "generate_single_library_with_librarian_config", @@ -476,9 +480,11 @@ func TestGenerateScenarios(t *testing.T) { }, ghClient: &mockGitHubClient{}, build: true, + useMockRepo: true, wantGenerateCalls: 1, wantBuildCalls: 1, wantConfigureCalls: 0, + wantCommitMessage: "feat: generate libraries", }, { name: "generate single existing library by api", @@ -838,8 +844,7 @@ func TestGenerateScenarios(t *testing.T) { } { t.Run(test.name, func(t *testing.T) { var repo gitrepo.Repository - useMockRepo := test.name == "generate_single_library_including_initial_configuration" || test.name == "generate single existing library by library id" - if useMockRepo { + if test.useMockRepo { repo = &MockRepository{ Dir: t.TempDir(), RemotesValue: []*gitrepo.Remote{ @@ -865,7 +870,7 @@ func TestGenerateScenarios(t *testing.T) { ghClient: &mockGitHubClient{}, workRoot: t.TempDir(), } - if useMockRepo { + if test.useMockRepo { r.commit = true r.push = true } @@ -924,16 +929,10 @@ func TestGenerateScenarios(t *testing.T) { if diff := cmp.Diff(test.wantConfigureCalls, test.container.configureCalls); diff != "" { t.Errorf("%s: run() configureCalls mismatch (-want +got):%s", test.name, diff) } - if test.name == "generate_single_library_including_initial_configuration" { + if test.useMockRepo && test.wantCommitMessage != "" { mockRepo := repo.(*MockRepository) - if mockRepo.LastCommitMessage != "feat: onboard a new library" { - t.Errorf("want commit message %q, got %q", "feat: onboard a new library", mockRepo.LastCommitMessage) - } - } - if test.name == "generate single existing library by library id" { - mockRepo := repo.(*MockRepository) - if mockRepo.LastCommitMessage != "feat: generate libraries" { - t.Errorf("want commit message %q, got %q", "feat: generate libraries", mockRepo.LastCommitMessage) + if mockRepo.LastCommitMessage != test.wantCommitMessage { + t.Errorf("want commit message %q, got %q", test.wantCommitMessage, mockRepo.LastCommitMessage) } } }) From 646f29d24c158924319c673df4244ace01dc5c59 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Thu, 16 Oct 2025 17:10:27 +0000 Subject: [PATCH 3/5] address PR feedback --- internal/librarian/generate_command.go | 6 +--- internal/librarian/generate_command_test.go | 33 ++------------------- 2 files changed, 3 insertions(+), 36 deletions(-) diff --git a/internal/librarian/generate_command.go b/internal/librarian/generate_command.go index 74961d07e7..7c036474bf 100644 --- a/internal/librarian/generate_command.go +++ b/internal/librarian/generate_command.go @@ -176,14 +176,10 @@ func (r *generateRunner) run(ctx context.Context) error { return fmt.Errorf("unexpected prType %s", prType) } - commitMessage := "feat: generate libraries" - if prType == pullRequestOnboard { - commitMessage = "feat: onboard a new library" - } commitInfo := &commitInfo{ branch: r.branch, commit: r.commit, - commitMessage: commitMessage, + commitMessage: "feat: generate libraries", ghClient: r.ghClient, prType: prType, push: r.push, diff --git a/internal/librarian/generate_command_test.go b/internal/librarian/generate_command_test.go index 3a571af289..b7ff897523 100644 --- a/internal/librarian/generate_command_test.go +++ b/internal/librarian/generate_command_test.go @@ -404,13 +404,11 @@ func TestGenerateScenarios(t *testing.T) { container *mockContainerClient ghClient GitHubClient build bool - useMockRepo bool wantErr bool wantErrMsg string wantGenerateCalls int wantBuildCalls int wantConfigureCalls int - wantCommitMessage string }{ { name: "generate_single_library_including_initial_configuration", @@ -427,11 +425,9 @@ func TestGenerateScenarios(t *testing.T) { }, ghClient: &mockGitHubClient{}, build: true, - useMockRepo: true, wantGenerateCalls: 1, wantBuildCalls: 1, wantConfigureCalls: 1, - wantCommitMessage: "feat: onboard a new library", }, { name: "generate_single_library_with_librarian_config", @@ -480,11 +476,9 @@ func TestGenerateScenarios(t *testing.T) { }, ghClient: &mockGitHubClient{}, build: true, - useMockRepo: true, wantGenerateCalls: 1, wantBuildCalls: 1, wantConfigureCalls: 0, - wantCommitMessage: "feat: generate libraries", }, { name: "generate single existing library by api", @@ -843,20 +837,7 @@ func TestGenerateScenarios(t *testing.T) { }, } { t.Run(test.name, func(t *testing.T) { - var repo gitrepo.Repository - if test.useMockRepo { - repo = &MockRepository{ - Dir: t.TempDir(), - RemotesValue: []*gitrepo.Remote{ - { - Name: "origin", - URLs: []string{"https://github.com/googleapis/librarian.git"}, - }, - }, - } - } else { - repo = newTestGitRepoWithState(t, test.state, true) - } + repo := newTestGitRepoWithState(t, test.state, true) r := &generateRunner{ api: test.api, @@ -867,13 +848,9 @@ func TestGenerateScenarios(t *testing.T) { state: test.state, librarianConfig: test.librarianConfig, containerClient: test.container, - ghClient: &mockGitHubClient{}, + ghClient: test.ghClient, workRoot: t.TempDir(), } - if test.useMockRepo { - r.commit = true - r.push = true - } // Create a service config in api path. if err := os.MkdirAll(filepath.Join(r.sourceRepo.GetDir(), test.api), 0755); err != nil { @@ -929,12 +906,6 @@ func TestGenerateScenarios(t *testing.T) { if diff := cmp.Diff(test.wantConfigureCalls, test.container.configureCalls); diff != "" { t.Errorf("%s: run() configureCalls mismatch (-want +got):%s", test.name, diff) } - if test.useMockRepo && test.wantCommitMessage != "" { - mockRepo := repo.(*MockRepository) - if mockRepo.LastCommitMessage != test.wantCommitMessage { - t.Errorf("want commit message %q, got %q", test.wantCommitMessage, mockRepo.LastCommitMessage) - } - } }) } } From 89e479b9d0454a341653449e59ff36a14982f5c4 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Thu, 16 Oct 2025 17:11:34 +0000 Subject: [PATCH 4/5] address PR feedback --- internal/librarian/mocks_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/librarian/mocks_test.go b/internal/librarian/mocks_test.go index 489c6a589b..7196dea928 100644 --- a/internal/librarian/mocks_test.go +++ b/internal/librarian/mocks_test.go @@ -308,7 +308,6 @@ type MockRepository struct { RemotesValue []*gitrepo.Remote RemotesError error CommitCalls int - LastCommitMessage string GetCommitError error GetLatestCommitError error GetCommitByHash map[string]*gitrepo.Commit @@ -356,7 +355,6 @@ func (m *MockRepository) AddAll() error { func (m *MockRepository) Commit(msg string) error { m.CommitCalls++ - m.LastCommitMessage = msg return m.CommitError } From 850c6f8fcf68caa5d90ff26c1a2cf271c306173c Mon Sep 17 00:00:00 2001 From: ohmayr Date: Thu, 16 Oct 2025 17:24:04 +0000 Subject: [PATCH 5/5] address PR feedback --- internal/librarian/generate_command_test.go | 109 ++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/internal/librarian/generate_command_test.go b/internal/librarian/generate_command_test.go index b7ff897523..3632d49b87 100644 --- a/internal/librarian/generate_command_test.go +++ b/internal/librarian/generate_command_test.go @@ -910,6 +910,115 @@ func TestGenerateScenarios(t *testing.T) { } } +func TestGenerateSingleLibraryCommand(t *testing.T) { + t.Parallel() + for _, test := range []struct { + name string + api string + library string + state *config.LibrarianState + container *mockContainerClient + ghClient GitHubClient + build bool + wantErr bool + wantErrMsg string + wantPRType pullRequestType + }{ + { + name: "onboard library returns pullRequestOnboard", + api: "some/api", + library: "some-library", + state: &config.LibrarianState{ + Image: "gcr.io/test/image:v1.2.3", + }, + container: &mockContainerClient{ + wantLibraryGen: true, + configureLibraryPaths: []string{ + "src/a", + }, + }, + ghClient: &mockGitHubClient{}, + build: true, + wantPRType: pullRequestOnboard, + }, + { + name: "generate existing library returns pullRequestGenerate", + library: "some-library", + state: &config.LibrarianState{ + Image: "gcr.io/test/image:v1.2.3", + Libraries: []*config.LibraryState{ + { + ID: "some-library", + APIs: []*config.API{{Path: "some/api"}}, + SourceRoots: []string{ + "src/a", + }, + }, + }, + }, + container: &mockContainerClient{ + wantLibraryGen: true, + }, + ghClient: &mockGitHubClient{}, + build: true, + wantPRType: pullRequestGenerate, + }, + } { + t.Run(test.name, func(t *testing.T) { + repo := newTestGitRepoWithState(t, test.state, true) + sourceRepo := newTestGitRepo(t) + r := &generateRunner{ + api: test.api, + library: test.library, + build: test.build, + repo: repo, + sourceRepo: sourceRepo, + state: test.state, + containerClient: test.container, + ghClient: test.ghClient, + workRoot: t.TempDir(), + } + + // Create a service config in api path. + if test.api != "" { + if err := os.MkdirAll(filepath.Join(r.sourceRepo.GetDir(), test.api), 0755); err != nil { + t.Fatal(err) + } + data := []byte("type: google.api.Service") + if err := os.WriteFile(filepath.Join(r.sourceRepo.GetDir(), test.api, "example_service_v2.yaml"), data, 0755); err != nil { + t.Fatal(err) + } + // Commit the service config file because configure command needs + // to find the piper id associated with the commit message. + if err := r.sourceRepo.AddAll(); err != nil { + t.Fatal(err) + } + message := "feat: add an api\n\nPiperOrigin-RevId: 123456" + if err := r.sourceRepo.Commit(message); err != nil { + t.Fatal(err) + } + } + + status, err := r.generateSingleLibrary(context.Background(), r.library, r.workRoot) + if test.wantErr { + if err == nil { + t.Fatalf("%s should return error", test.name) + } + if !strings.Contains(err.Error(), test.wantErrMsg) { + t.Errorf("want error message %s, got %s", test.wantErrMsg, err.Error()) + } + return + } + if err != nil { + t.Fatal(err) + } + if status.prType != test.wantPRType { + t.Errorf("generateSingleLibrary() prType = %v, want %v", status.prType, test.wantPRType) + } + }) + } +} + func TestUpdateLastGeneratedCommitState(t *testing.T) { t.Parallel() sourceRepo := newTestGitRepo(t)