Skip to content

Commit 7309cad

Browse files
authored
fix(librarian): use templates for update-image PR body (#2602)
Fixes #2587
1 parent 14a1483 commit 7309cad

File tree

2 files changed

+88
-11
lines changed

2 files changed

+88
-11
lines changed

internal/librarian/update_image.go

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,13 @@
1515
package librarian
1616

1717
import (
18+
"bytes"
1819
"context"
1920
"fmt"
21+
"html/template"
2022
"log/slog"
2123
"path/filepath"
24+
"strings"
2225

2326
"github.com/googleapis/librarian/internal/config"
2427
"github.com/googleapis/librarian/internal/gitrepo"
@@ -126,18 +129,8 @@ func (r *updateImageRunner) run(ctx context.Context) error {
126129
}
127130
slog.Info("successful generations", slog.Int("num", len(successfulGenerations)))
128131

129-
// TODO(#2587): improve PR body content
130132
prBodyBuilder := func() (string, error) {
131-
body := fmt.Sprintf("feat: update image to %s\n\n", r.image)
132-
if len(failedGenerations) > 0 {
133-
body += "The following libraries failed to regenerate:\n"
134-
for _, lib := range failedGenerations {
135-
body += fmt.Sprintf("- %s\n", lib.ID)
136-
}
137-
} else {
138-
body += "All libraries regenerated successfully."
139-
}
140-
return body, nil
133+
return formatUpdateImagePRBody(r.image, failedGenerations)
141134
}
142135
commitMessage := fmt.Sprintf("feat: update image to %s", r.image)
143136
// TODO(#2588): open PR as draft if there are failures
@@ -185,3 +178,33 @@ func (r *updateImageRunner) regenerateSingleLibrary(ctx context.Context, library
185178

186179
return nil
187180
}
181+
182+
var updateImageTemplate = template.Must(template.New("updateImage").Parse(`feat: update image to {{.Image}}
183+
{{ if .FailedLibraries }}
184+
## Generation failed for
185+
{{- range .FailedLibraries }}
186+
- {{ . }}
187+
{{- end -}}
188+
{{- end }}
189+
`))
190+
191+
type updateImagePRBody struct {
192+
Image string
193+
FailedLibraries []string
194+
}
195+
196+
func formatUpdateImagePRBody(image string, failedGenerations []*config.LibraryState) (string, error) {
197+
failedLibraries := make([]string, 0, len(failedGenerations))
198+
for _, failedGeneration := range failedGenerations {
199+
failedLibraries = append(failedLibraries, failedGeneration.ID)
200+
}
201+
data := &updateImagePRBody{
202+
Image: image,
203+
FailedLibraries: failedLibraries,
204+
}
205+
var out bytes.Buffer
206+
if err := updateImageTemplate.Execute(&out, data); err != nil {
207+
return "", fmt.Errorf("error executing template %w", err)
208+
}
209+
return strings.TrimSpace(out.String()), nil
210+
}

internal/librarian/update_image_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -760,3 +760,57 @@ func TestUpdateImageRunnerRun(t *testing.T) {
760760
})
761761
}
762762
}
763+
764+
func TestFormatUpdateImagePRBody(t *testing.T) {
765+
t.Parallel()
766+
for _, test := range []struct {
767+
name string
768+
image string
769+
failedGenerations []*config.LibraryState
770+
want string
771+
wantErr bool
772+
wantErrMsg string
773+
}{
774+
{
775+
name: "success",
776+
image: "some-image",
777+
want: "feat: update image to some-image",
778+
},
779+
{
780+
name: "multiple errors",
781+
image: "some-image",
782+
failedGenerations: []*config.LibraryState{
783+
{
784+
ID: "library-id-1",
785+
},
786+
{
787+
ID: "library-id-2",
788+
},
789+
},
790+
want: `feat: update image to some-image
791+
792+
## Generation failed for
793+
- library-id-1
794+
- library-id-2`,
795+
},
796+
} {
797+
t.Run(test.name, func(t *testing.T) {
798+
t.Parallel()
799+
got, err := formatUpdateImagePRBody(test.image, test.failedGenerations)
800+
801+
if test.wantErr {
802+
if err == nil {
803+
t.Fatalf("formatUpdateImagePRBody() error = %v, wantErr %v", err, test.wantErr)
804+
}
805+
806+
if !strings.Contains(err.Error(), test.wantErrMsg) {
807+
t.Fatalf("want error message: %s, got: %s", test.wantErrMsg, err.Error())
808+
}
809+
return
810+
}
811+
if diff := cmp.Diff(got, test.want); diff != "" {
812+
t.Errorf("%s: formatUpdateImagePRBody() mismatch (-want +got):%s", test.name, diff)
813+
}
814+
})
815+
}
816+
}

0 commit comments

Comments
 (0)