Skip to content

Commit fba7a84

Browse files
committed
gemini review fixes
1 parent 0a87de2 commit fba7a84

File tree

2 files changed

+24
-16
lines changed

2 files changed

+24
-16
lines changed

helpers/foundation-deployer/gcp/gcp.go

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -53,25 +53,23 @@ type Build struct {
5353

5454
var (
5555
retryRegexp = map[*regexp.Regexp]string{}
56-
ctx = context.Background()
56+
// ctx = context.Background()
5757
)
5858

5959
func init() {
60-
if len(retryRegexp) == 0 {
61-
for e, m := range testutils.RetryableTransientErrors {
62-
r, err := regexp.Compile(fmt.Sprintf("(?s)%s", e)) //(?s) enables dot (.) to match newline.
63-
if err != nil {
64-
fmt.Printf("failed to compile regex %s: %s", e, err.Error())
65-
}
66-
retryRegexp[r] = m
60+
for e, m := range testutils.RetryableTransientErrors {
61+
r, err := regexp.Compile(fmt.Sprintf("(?s)%s", e)) //(?s) enables dot (.) to match newline.
62+
if err != nil {
63+
panic(fmt.Sprintf("failed to compile regex %s: %s", e, err.Error()))
6764
}
65+
retryRegexp[r] = m
6866
}
6967
}
7068

7169
type GCP struct {
7270
Runf func(t testing.TB, cmd string, args ...interface{}) gjson.Result
7371
RunCmd func(t testing.TB, cmd string, args ...interface{}) string
74-
TriggerNewBuild func(t testing.TB, buildName string) (string, error)
72+
TriggerNewBuild func(t testing.TB, ctx context.Context, buildName string) (string, error)
7573
sleepTime time.Duration
7674
}
7775

@@ -81,7 +79,7 @@ func runCmd(t testing.TB, cmd string, args ...interface{}) string {
8179
}
8280

8381
// triggerNewBuild triggers a new build based on the build provided
84-
func triggerNewBuild(t testing.TB, buildName string) (string, error) {
82+
func triggerNewBuild(t testing.TB, ctx context.Context, buildName string) (string, error) {
8583

8684
buildService, err := cloudbuild.NewService(ctx, option.WithScopes(cloudbuild.CloudPlatformScope))
8785
if err != nil {
@@ -135,7 +133,11 @@ func (g GCP) GetBuilds(t testing.TB, projectID, region, filter string) map[strin
135133

136134
// GetLastBuildStatus gets the status of the last build form a project and region that satisfy the given filter.
137135
func (g GCP) GetLastBuildStatus(t testing.TB, projectID, region, filter string) (string, string) {
138-
build := g.Runf(t, "builds list --project %s --region %s --limit 1 --sort-by ~createTime --filter %s", projectID, region, filter).Array()[0]
136+
builds := g.Runf(t, "builds list --project %s --region %s --limit 1 --sort-by ~createTime --filter %s", projectID, region, filter).Array()
137+
if len(builds) == 0 {
138+
return "", ""
139+
}
140+
build := builds[0]
139141
return build.Get("status").String(), build.Get("id").String()
140142
}
141143

@@ -185,6 +187,7 @@ func (g GCP) GetFinalBuildState(t testing.TB, projectID, region, buildID string,
185187
func (g GCP) WaitBuildSuccess(t testing.TB, project, region, repo, commitSha, failureMsg string, maxBuildRetry, maxErrorRetries int, timeBetweenErrorRetries time.Duration) error {
186188
var filter, status, build string
187189
var timeoutErr, err error
190+
ctx := context.Background()
188191

189192
if commitSha == "" {
190193
filter = fmt.Sprintf("source.repoSource.repoName:%s", repo)
@@ -196,13 +199,17 @@ func (g GCP) WaitBuildSuccess(t testing.TB, project, region, repo, commitSha, fa
196199
for i := 0; i < maxErrorRetries; i++ {
197200
if build != "" {
198201
status, timeoutErr = g.GetFinalBuildState(t, project, region, build, maxBuildRetry)
202+
if timeoutErr != nil {
203+
return timeoutErr
204+
}
199205
} else {
200206
status, build = g.GetLastBuildStatus(t, project, region, filter)
207+
if build == "" {
208+
return fmt.Errorf("no build found for filter: %s", filter)
209+
}
201210
}
202211

203-
if timeoutErr != nil {
204-
return timeoutErr
205-
} else if status != StatusSuccess {
212+
if status != StatusSuccess {
206213
if !g.IsRetryableError(t, project, region, build) {
207214
return fmt.Errorf("%s\nSee:\nhttps://console.cloud.google.com/cloud-build/builds;region=%s/%s?project=%s\nfor details", failureMsg, region, build, project)
208215
}
@@ -212,7 +219,7 @@ func (g GCP) WaitBuildSuccess(t testing.TB, project, region, repo, commitSha, fa
212219
}
213220

214221
// Trigger a new build
215-
build, err = g.TriggerNewBuild(t, fmt.Sprintf("projects/%s/locations/%s/builds/%s", project, region, build))
222+
build, err = g.TriggerNewBuild(t, ctx, fmt.Sprintf("projects/%s/locations/%s/builds/%s", project, region, build))
216223
if err != nil {
217224
return fmt.Errorf("failed to trigger new build after %d retries: %w", maxErrorRetries, err)
218225
}

helpers/foundation-deployer/gcp/gcp_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package gcp
1616

1717
import (
18+
"context"
1819
"fmt"
1920
"os"
2021
"path/filepath"
@@ -219,7 +220,7 @@ func TestWaitBuildSuccessRetry(t *gotest.T) {
219220
runCmdCallCount = runCmdCallCount + 1
220221
return "a\nError 403. Compute Engine API has not been used in project\nz" // get build logs
221222
},
222-
TriggerNewBuild: func(t testing.TB, buildName string) (string, error) {
223+
TriggerNewBuild: func(t testing.TB, ctx context.Context, buildName string) (string, error) {
223224
triggerNewBuildCallCount = triggerNewBuildCallCount + 1
224225
return "845f5790-2497-4382-afd0-b5f0f50eea5a", nil // buildService.Projects.Locations.Builds.Retry
225226
},

0 commit comments

Comments
 (0)