feat(internal/librarian): record library generation failure message in PR message (#999)

catchyzheng · web-flow · commit 97720cd23e58 · 2025-07-29T09:28:39.000-05:00
Record library generation failure message in PR message Fix: #983
diff --git a/internal/librarian/command.go b/internal/librarian/command.go
@@ -137,7 +137,7 @@ func createWorkRoot(t time.Time, workRootOverride string) (string, error) {
 
 // commitAndPush creates a commit and push request to Github for the generated changes.
 // It uses the GitHub client to create a PR with the specified branch, title, and description to the repository.
-func commitAndPush(ctx context.Context, repo *gitrepo.Repository, ghClient GitHubClient, pushConfig string) error {
+func commitAndPush(ctx context.Context, repo *gitrepo.Repository, ghClient GitHubClient, pushConfig, commitMessage string) error {
 	if pushConfig == "" {
 		slog.Info("PushConfig flag not specified, skipping")
 		return nil
@@ -157,16 +157,15 @@ func commitAndPush(ctx context.Context, repo *gitrepo.Repository, ghClient GitHu
 	}
 
 	// TODO: get correct language for message (https://github.com/googleapis/librarian/issues/885)
-	message := "Changes in this PR"
-	repo.Commit(message, userName, userEmail)
+	repo.Commit(commitMessage, userName, userEmail)
 
 	// Create a new branch, set title and message for the PR.
 	datetimeNow := formatTimestamp(time.Now())
 	titlePrefix := "Librarian pull request"
 	branch := fmt.Sprintf("librarian-%s", datetimeNow)
 	title := fmt.Sprintf("%s: %s", titlePrefix, datetimeNow)
 
-	_, err = ghClient.CreatePullRequest(ctx, gitHubRepo, branch, title, message)
+	_, err = ghClient.CreatePullRequest(ctx, gitHubRepo, branch, title, commitMessage)
 	if err != nil {
 		return fmt.Errorf("failed to create pull request: %w", err)
 	}
diff --git a/internal/librarian/command_test.go b/internal/librarian/command_test.go
@@ -496,7 +496,7 @@ func TestCommitAndPush(t *testing.T) {
 			repo := test.setupMockRepo(t)
 			client := test.setupMockClient(t)
 
-			err := commitAndPush(context.Background(), repo, client, test.pushConfig)
+			err := commitAndPush(context.Background(), repo, client, test.pushConfig, "")
 
 			if err != nil && !strings.Contains(err.Error(), test.expectedErrMsg) {
 				t.Errorf("commitAndPush() error = %v, expected to contain: %q", err, test.expectedErrMsg)
diff --git a/internal/librarian/generate.go b/internal/librarian/generate.go
@@ -158,6 +158,7 @@ func (r *generateRunner) run(ctx context.Context) error {
 	}
 	slog.Info("Code will be generated", "dir", outputDir)
 
+	prBody := ""
 	if r.cfg.API != "" || r.cfg.Library != "" {
 		libraryID := r.cfg.Library
 		if libraryID == "" {
@@ -171,11 +172,12 @@ func (r *generateRunner) run(ctx context.Context) error {
 			if err := r.generateSingleLibrary(ctx, library.ID, outputDir); err != nil {
 				// TODO(https://github.com/googleapis/librarian/issues/983): record failure and report in PR body when applicable
 				slog.Error("failed to generate library", "id", library.ID, "err", err)
+				prBody += fmt.Sprintf("%s failed to generate\n", library.ID)
 			}
 		}
 	}
 
-	if err := commitAndPush(ctx, r.repo, r.ghClient, r.cfg.PushConfig); err != nil {
+	if err := commitAndPush(ctx, r.repo, r.ghClient, r.cfg.PushConfig, prBody); err != nil {
 		return err
 	}
 	return nil
diff --git a/internal/librarian/generate_test.go b/internal/librarian/generate_test.go
@@ -37,15 +37,22 @@ import (
 // mockContainerClient is a mock implementation of the ContainerClient interface for testing.
 type mockContainerClient struct {
 	ContainerClient
-	generateCalls  int
-	buildCalls     int
-	configureCalls int
-	generateErr    error
-	buildErr       error
+	generateCalls     int
+	buildCalls        int
+	configureCalls    int
+	generateErr       error
+	buildErr          error
+	failGenerateForID string
 }
 
 func (m *mockContainerClient) Generate(ctx context.Context, request *docker.GenerateRequest) error {
 	m.generateCalls++
+	if m.failGenerateForID != "" {
+		if request.LibraryID == m.failGenerateForID {
+			return m.generateErr
+		}
+		return nil
+	}
 	return m.generateErr
 }
 
@@ -472,6 +479,31 @@ func TestGenerateRun(t *testing.T) {
 			build:      true,
 			wantErr:    true,
 		},
+		{
+			name: "generate all, partial failure does not halt execution",
+			repo: newTestGitRepo(t),
+			state: &config.LibrarianState{
+				Image: "gcr.io/test/image:v1.2.3",
+				Libraries: []*config.LibraryState{
+					{
+						ID:   "lib1",
+						APIs: []*config.API{{Path: "some/api1"}},
+					},
+					{
+						ID:   "lib2",
+						APIs: []*config.API{{Path: "some/api2"}},
+					},
+				},
+			},
+			container: &mockContainerClient{
+				failGenerateForID: "lib1",
+				generateErr:       errors.New("generate error"),
+			},
+			ghClient:          &mockGitHubClient{},
+			build:             true,
+			wantGenerateCalls: 2,
+			wantBuildCalls:    1,
+		},
 		{
 			name: "commit and push error",
 			api:  "some/api",

Original file line number	Diff line number	Diff line change
`@@ -158,6 +158,7 @@ func (r *generateRunner) run(ctx context.Context) error {`
`158`	`158`	`}`
`159`	`159`	`slog.Info("Code will be generated", "dir", outputDir)`
`160`	`160`
	`161`	`+ prBody := ""`
`161`	`162`	`if r.cfg.API != "" \|\| r.cfg.Library != "" {`
`162`	`163`	`libraryID := r.cfg.Library`
`163`	`164`	`if libraryID == "" {`
`@@ -171,11 +172,12 @@ func (r *generateRunner) run(ctx context.Context) error {`
`171`	`172`	`if err := r.generateSingleLibrary(ctx, library.ID, outputDir); err != nil {`
`172`	`173`	`// TODO(https://github.com/googleapis/librarian/issues/983): record failure and report in PR body when applicable`
`173`	`174`	`slog.Error("failed to generate library", "id", library.ID, "err", err)`
	`175`	`+ prBody += fmt.Sprintf("%s failed to generate\n", library.ID)`
`174`	`176`	`}`
`175`	`177`	`}`
`176`	`178`	`}`
`177`	`179`
`178`		`- if err := commitAndPush(ctx, r.repo, r.ghClient, r.cfg.PushConfig); err != nil {`
	`180`	`+ if err := commitAndPush(ctx, r.repo, r.ghClient, r.cfg.PushConfig, prBody); err != nil {`
`179`	`181`	`return err`
`180`	`182`	`}`
`181`	`183`	`return nil`