From b51313342d16a4669124ccba1f06667e693ae3dd Mon Sep 17 00:00:00 2001 From: "Stephen Lewis (Burrows)" Date: Wed, 13 Mar 2024 13:15:38 -0700 Subject: [PATCH] Fix generate comment diff processor build failure (#10164) * Refactored generate_comment to continue in the face of errors Also switched to go template for comment formatting * Moved to structured data for diffs Also added error formatting into comment * Added basic tests for comment formatting * Skipped missing tests and breaking changes if repo failed to clone * Force generation * Breaking change * Fixed error display in diff comment * Marked diff-processor targets as not being real files * Added PATH to diffProcessorEnv * Fixed formatting for error sections * Cleaned up passthrough env var usage * Made exec.sh exit 1 if there are any errors running the magician * Don't error on initial build of the magician binary * Added missing env var for diff processor build * Force generation of tf-oics * Revert "Force generation of tf-oics" This reverts commit fcb65f121cbb0f7040a6b40c635eae3b318d936d. * Revert "Force generation" This reverts commit 596d1eb5afb10d36ab934411e0f1c47aa8b363e9. * Revert "Breaking change" This reverts commit fb04ad401e1d4ee93ef767297a2a59af7d2acffa. * Force missing test run * force missing tests to run - take 2 * Revert "force missing tests to run - take 2" This reverts commit 34f7d16a8442f09aa4a52e0ceafb8dae73f89b08. * Revert "Force missing test run" This reverts commit 90e664a935c4645314c3aabf1f48836e4547fc68. --- .ci/magician/cmd/DIFF_COMMENT.md | 36 ++ .ci/magician/cmd/generate_comment.go | 466 ++++++++++++---------- .ci/magician/cmd/generate_comment_test.go | 192 +++++++-- .ci/magician/cmd/mock_runner_test.go | 53 +-- .ci/magician/source/repo.go | 2 +- .ci/scripts/go-plus/magician/exec.sh | 2 + tools/diff-processor/GNUmakefile | 2 + 7 files changed, 488 insertions(+), 265 deletions(-) create mode 100644 .ci/magician/cmd/DIFF_COMMENT.md diff --git a/.ci/magician/cmd/DIFF_COMMENT.md b/.ci/magician/cmd/DIFF_COMMENT.md new file mode 100644 index 000000000000..b2c44b960823 --- /dev/null +++ b/.ci/magician/cmd/DIFF_COMMENT.md @@ -0,0 +1,36 @@ +Hi there, I'm the Modular magician. I've detected the following information about your changes: + +## Diff report +{{ $diffsLength := len .Diffs }}{{if eq $diffsLength 0 }} +Your PR hasn't generated any diffs, but I'll let you know if a future commit does. +{{else}} +Your PR generated some diffs in downstreams - here they are. + +{{range .Diffs -}} +{{.Title}}: [Diff](https://github.com/modular-magician/{{.Repo}}/compare/auto-pr-{{$.PrNumber}}-old..auto-pr-{{$.PrNumber}}) ({{.DiffStats}}) +{{end -}} +{{end -}} + +{{- $breakingChangesLength := len .BreakingChanges }} +{{- if gt $breakingChangesLength 0}} +## Breaking Change(s) Detected + +The following breaking change(s) were detected within your pull request. + +{{- range .BreakingChanges}} +- {{.}}{{end}} + +If you believe this detection to be incorrect please raise the concern with your reviewer. +If you intend to make this change you will need to wait for a [major release](https://www.terraform.io/plugin/sdkv2/best-practices/versioning#example-major-number-increments) window. +An `override-breaking-change` label can be added to allow merging. +{{end}} +{{.MissingTests}} +{{- $errorsLength := len .Errors}} +{{- if gt $errorsLength 0}} +## Errors +{{range .Errors}} +{{.Title}}: +{{- range .Errors}} +- {{.}}{{end}} +{{end}} +{{- end -}} \ No newline at end of file diff --git a/.ci/magician/cmd/generate_comment.go b/.ci/magician/cmd/generate_comment.go index b78bbf35fd3c..d1d5cd2936b4 100644 --- a/.ci/magician/cmd/generate_comment.go +++ b/.ci/magician/cmd/generate_comment.go @@ -23,12 +23,41 @@ import ( "magician/source" "os" "path/filepath" - "regexp" + "sort" + "strconv" "strings" + "text/template" "github.com/spf13/cobra" + "golang.org/x/exp/maps" + + _ "embed" +) + +var ( + //go:embed DIFF_COMMENT.md + diffComment string ) +type Diff struct { + Title string + Repo string + DiffStats string +} + +type Errors struct { + Title string + Errors []string +} + +type diffCommentData struct { + PrNumber int + Diffs []Diff + BreakingChanges []string + MissingTests string + Errors []Errors +} + const allowBreakingChangesLabel = "override-breaking-change" var gcEnvironmentVariables = [...]string{ @@ -84,7 +113,22 @@ var generateCommentCmd = &cobra.Command{ os.Exit(1) } ctlr := source.NewController(filepath.Join("workspace", "go"), "modular-magician", env["GITHUB_TOKEN_DOWNSTREAMS"], rnr) - execGenerateComment(env, gh, rnr, ctlr) + prNumber, err := strconv.Atoi(env["PR_NUMBER"]) + if err != nil { + fmt.Println("Error parsing PR_NUMBER: ", err) + os.Exit(1) + } + execGenerateComment( + prNumber, + env["GITHUB_TOKEN_MAGIC_MODULES"], + env["BUILD_ID"], + env["BUILD_STEP"], + env["PROJECT_ID"], + env["COMMIT_SHA"], + gh, + rnr, + ctlr, + ) }, } @@ -96,196 +140,219 @@ func listGCEnvironmentVariables() string { return result } -func execGenerateComment(env map[string]string, gh GithubClient, rnr ExecRunner, ctlr *source.Controller) { - newBranch := "auto-pr-" + env["PR_NUMBER"] - oldBranch := "auto-pr-" + env["PR_NUMBER"] + "-old" +func execGenerateComment(prNumber int, ghTokenMagicModules, buildId, buildStep, projectId, commitSha string, gh GithubClient, rnr ExecRunner, ctlr *source.Controller) { + newBranch := fmt.Sprintf("auto-pr-%d", prNumber) + oldBranch := fmt.Sprintf("auto-pr-%d-old", prNumber) wd := rnr.GetCWD() mmLocalPath := filepath.Join(wd, "..", "..") - tpgRepoName := "terraform-provider-google" - tpgLocalPath := filepath.Join(mmLocalPath, "..", "tpg") - tpgbRepoName := "terraform-provider-google-beta" - tpgbLocalPath := filepath.Join(mmLocalPath, "..", "tpgb") - tfoicsRepoName := "docs-examples" - tfoicsLocalPath := filepath.Join(mmLocalPath, "..", "tfoics") - // For backwards compatibility until at least Nov 15 2021 - tfcRepoName := "terraform-google-conversion" - tfcLocalPath := filepath.Join(mmLocalPath, "..", "tfc") - - var diffs string - for _, repo := range []*source.Repo{ - { - Name: tpgRepoName, - Title: "Terraform GA", - Path: tpgLocalPath, - }, - { - Name: tpgbRepoName, - Title: "Terraform Beta", - Path: tpgbLocalPath, - }, - { - Name: tfcRepoName, - Title: "TF Conversion", - Path: tfcLocalPath, - DiffCanFail: true, - }, - { - Name: tfoicsRepoName, - Title: "TF OiCS", - Path: tfoicsLocalPath, - }, - } { - // TPG/TPGB difference - repoDiffs, err := cloneAndDiff(repo, oldBranch, newBranch, ctlr) + + tpgRepo := source.Repo{ + Name: "terraform-provider-google", + Title: "`google` provider", + Path: filepath.Join(mmLocalPath, "..", "tpg"), + Version: provider.GA, + } + tpgbRepo := source.Repo{ + Name: "terraform-provider-google-beta", + Title: "`google-beta` provider", + Path: filepath.Join(mmLocalPath, "..", "tpgb"), + Version: provider.Beta, + } + tgcRepo := source.Repo{ + Name: "terraform-google-conversion", + Title: "`terraform-google-conversion`", + Path: filepath.Join(mmLocalPath, "..", "tgc"), + Version: provider.Beta, + } + tfoicsRepo := source.Repo{ + Name: "docs-examples", + Title: "Open in Cloud Shell", + Path: filepath.Join(mmLocalPath, "..", "tfoics"), + } + + // Initialize repos + data := diffCommentData{ + PrNumber: prNumber, + } + errors := map[string][]string{"Other": []string{}} + var err error + for _, repo := range []*source.Repo{&tpgRepo, &tpgbRepo, &tgcRepo, &tfoicsRepo} { + errors[repo.Title] = []string{} + repo.Branch = newBranch + if err := ctlr.Clone(repo); err != nil { + fmt.Println("Failed to clone repo: ", err) + errors[repo.Title] = append(errors[repo.Title], "Failed to clone repo") + } else { + repo.Cloned = true + } + } + + diffs := []Diff{} + for _, repo := range []source.Repo{tpgRepo, tpgbRepo, tgcRepo, tfoicsRepo} { + if !repo.Cloned { + fmt.Println("Skipping diff; repo failed to clone: ", repo.Name) + continue + } + diffStats, err := computeDiff(&repo, oldBranch, ctlr) if err != nil { - fmt.Printf("Error cloning and diffing tpg repo: %v\n", err) - if !repo.DiffCanFail { - os.Exit(1) - } + fmt.Println("diffing repo: ", err) + errors[repo.Title] = append(errors[repo.Title], "Failed to compute repo diff stats") } - if repoDiffs != "" { - diffs += "\n" + repoDiffs + if diffStats != "" { + diffs = append(diffs, Diff{ + Title: repo.Title, + Repo: repo.Name, + DiffStats: diffStats, + }) } } + data.Diffs = diffs - var showBreakingChangesFailed bool - var err error + // The breaking changes are unique across both provider versions + uniqueBreakingChanges := map[string]struct{}{} diffProcessorPath := filepath.Join(mmLocalPath, "tools", "diff-processor") - // versionedBreakingChanges is a map of breaking change output by provider version. - versionedBreakingChanges := make(map[provider.Version]string, 2) - - env["OLD_REF"] = oldBranch - env["NEW_REF"] = newBranch - for _, repo := range []*source.Repo{ - { - Title: "TPG", - Path: tpgLocalPath, - Version: provider.GA, - }, - { - Title: "TPGB", - Path: tpgbLocalPath, - Version: provider.Beta, - }, - } { - // TPG(B) diff processor - err = buildDiffProcessor(diffProcessorPath, repo.Path, env, rnr) + diffProcessorEnv := map[string]string{ + "OLD_REF": oldBranch, + "NEW_REF": newBranch, + // Passthrough vars required for a valid build environment. + "PATH": os.Getenv("PATH"), + "GOPATH": os.Getenv("GOPATH"), + "HOME": os.Getenv("HOME"), + } + for _, repo := range []source.Repo{tpgRepo, tpgbRepo} { + if !repo.Cloned { + fmt.Println("Skipping breaking changes; repo failed to clone: ", repo.Name) + continue + } + err = buildDiffProcessor(diffProcessorPath, repo.Path, diffProcessorEnv, rnr) if err != nil { - fmt.Println(err) - os.Exit(1) + fmt.Println("building diff processor: ", err) + errors[repo.Title] = append(errors[repo.Title], "The diff processor failed to build. This is usually due to the downstream provider failing to compile.") + continue } - output, err := computeBreakingChanges(diffProcessorPath, rnr) + + breakingChanges, err := computeBreakingChanges(diffProcessorPath, rnr) if err != nil { - fmt.Println("Error computing TPG breaking changes: ", err) - showBreakingChangesFailed = true + fmt.Println("computing breaking changes: ", err) + errors[repo.Title] = append(errors[repo.Title], "The diff processor crashed while computing breaking changes. This is usually due to the downstream provider failing to compile.") + } + for _, breakingChange := range breakingChanges { + uniqueBreakingChanges[breakingChange] = struct{}{} } - versionedBreakingChanges[repo.Version] = strings.TrimSuffix(output, "\n") - err = addLabels(diffProcessorPath, env, rnr) + + addLabelsEnv := map[string]string{ + "GITHUB_TOKEN_MAGIC_MODULES": ghTokenMagicModules, + } + err = addLabels(prNumber, diffProcessorPath, addLabelsEnv, rnr) if err != nil { - fmt.Println("Error adding TPG labels to PR: ", err) + fmt.Println("adding service labels: ", err) + errors[repo.Title] = append(errors[repo.Title], "The diff processor crashed while adding labels.") } err = cleanDiffProcessor(diffProcessorPath, rnr) if err != nil { - fmt.Println("Error cleaning up diff processor: ", err) - os.Exit(1) + fmt.Println("cleaning up diff processor: ", err) + errors[repo.Title] = append(errors[repo.Title], "The diff processor failed to clean up properly.") } } + breakingChangesSlice := maps.Keys(uniqueBreakingChanges) + sort.Strings(breakingChangesSlice) + data.BreakingChanges = breakingChangesSlice - var breakingChanges string - if showBreakingChangesFailed { - breakingChanges = `## Breaking Change Detection Failed -The breaking change detector crashed during execution. This is usually due to the downstream provider(s) failing to compile. Please investigate or follow up with your reviewer.` - } else { - breakingChanges = combineBreakingChanges(versionedBreakingChanges[provider.GA], versionedBreakingChanges[provider.Beta]) - } - - // Missing test detector - missingTests, err := detectMissingTests(mmLocalPath, tpgbLocalPath, oldBranch, rnr) - if err != nil { - fmt.Println("Error setting up missing test detector: ", err) - os.Exit(1) - } - - message := "Hi there, I'm the Modular magician. I've detected the following information about your changes:\n\n" + // Update breaking changes status on PR breakingState := "success" - if breakingChanges != "" { - message += breakingChanges + "\n\n" + if len(uniqueBreakingChanges) > 0 { + breakingState = "failure" - pullRequest, err := gh.GetPullRequest(env["PR_NUMBER"]) + pullRequest, err := gh.GetPullRequest(strconv.Itoa(prNumber)) if err != nil { fmt.Printf("Error getting pull request: %v\n", err) - os.Exit(1) - } - - breakingChangesAllowed := false - for _, label := range pullRequest.Labels { - if label.Name == allowBreakingChangesLabel { - breakingChangesAllowed = true - break + errors["Other"] = append(errors["Other"], "Failed to check for `override-breaking-change` label") + } else { + for _, label := range pullRequest.Labels { + if label.Name == allowBreakingChangesLabel { + breakingState = "success" + break + } } } - if !breakingChangesAllowed { - breakingState = "failure" - } } - - if diffs == "" { - message += "## Diff report\nYour PR hasn't generated any diffs, but I'll let you know if a future commit does." - } else { - message += "## Diff report\nYour PR generated some diffs in downstreams - here they are.\n" + diffs + "\n" - if missingTests != "" { - message += "\n" + missingTests + "\n" - } + targetURL := fmt.Sprintf("https://console.cloud.google.com/cloud-build/builds;region=global/%s;step=%s?project=%s", buildId, buildStep, projectId) + if err = gh.PostBuildStatus(strconv.Itoa(prNumber), "terraform-provider-breaking-change-test", breakingState, targetURL, commitSha); err != nil { + fmt.Printf("Error posting build status for pr %d commit %s: %v\n", prNumber, commitSha, err) + errors["Other"] = append(errors["Other"], "Failed to update breaking-change status check with state: "+breakingState) } - if err := gh.PostComment(env["PR_NUMBER"], message); err != nil { - fmt.Printf("Error posting comment to PR %s: %v\n", env["PR_NUMBER"], err) + // Run missing test detector (currently only for beta) + missingTestsPath := mmLocalPath + for _, repo := range []source.Repo{tpgbRepo} { + if !repo.Cloned { + fmt.Println("Skipping missing tests; repo failed to clone: ", repo.Name) + continue + } + missingTests, err := detectMissingTests(missingTestsPath, repo.Path, oldBranch, rnr) + if err != nil { + fmt.Println("Error running missing test detector: ", err) + errors[repo.Title] = append(errors[repo.Title], "The missing test detector failed to run.") + } + data.MissingTests = missingTests + } + + // Run unit tests for missing test detector + if err = runMissingTestUnitTests( + mmLocalPath, + tpgbRepo.Path, + targetURL, + commitSha, + prNumber, + gh, + rnr, + ); err != nil { + fmt.Println("Error running missing test detector unit tests: ", err) + errors["Other"] = append(errors["Other"], "Missing test detector unit tests failed to run.") + } + + // Add errors to data as an ordered list + errorsList := []Errors{} + for _, repo := range []source.Repo{tpgRepo, tpgbRepo, tgcRepo, tfoicsRepo} { + if len(errors[repo.Title]) > 0 { + errorsList = append(errorsList, Errors{ + Title: repo.Title, + Errors: errors[repo.Title], + }) + } } - - targetURL := fmt.Sprintf("https://console.cloud.google.com/cloud-build/builds;region=global/%s;step=%s?project=%s", env["BUILD_ID"], env["BUILD_STEP"], env["PROJECT_ID"]) - if err := gh.PostBuildStatus(env["PR_NUMBER"], "terraform-provider-breaking-change-test", breakingState, targetURL, env["COMMIT_SHA"]); err != nil { - fmt.Printf("Error posting build status for pr %s commit %s: %v\n", env["PR_NUMBER"], env["COMMIT_SHA"], err) - os.Exit(1) + if len(errors["Other"]) > 0 { + errorsList = append(errorsList, Errors{ + Title: "Other", + Errors: errors["Other"], + }) } + data.Errors = errorsList - if err := rnr.PushDir(mmLocalPath); err != nil { - fmt.Println(err) + // Post diff comment + message, err := formatDiffComment(data) + if err != nil { + fmt.Println("Error formatting message: ", err) + fmt.Printf("Data: %v\n", data) os.Exit(1) } - if diffs := rnr.MustRun("git", []string{"diff", "HEAD", "origin/main", "tools/missing-test-detector"}, nil); diffs != "" { - fmt.Printf("Found diffs in missing test detector:\n%s\nRunning tests.\n", diffs) - if err := testTools(mmLocalPath, tpgbLocalPath, env, gh, rnr); err != nil { - fmt.Printf("Error testing tools in %s: %v\n", mmLocalPath, err) - os.Exit(1) - } - } - if err := rnr.PopDir(); err != nil { - fmt.Println(err) + if err := gh.PostComment(strconv.Itoa(prNumber), message); err != nil { + fmt.Printf("Error posting comment to PR %d: %v\n", prNumber, err) + fmt.Println("Comment: ", message) os.Exit(1) } } -func cloneAndDiff(repo *source.Repo, oldBranch, newBranch string, ctlr *source.Controller) (string, error) { - // Clone the repo to the desired repo.Path. - repo.Branch = newBranch - if err := ctlr.Clone(repo); err != nil { - return "", fmt.Errorf("error cloning %s: %v\n", repo.Name, err) - } - +func computeDiff(repo *source.Repo, oldBranch string, ctlr *source.Controller) (string, error) { if err := ctlr.Fetch(repo, oldBranch); err != nil { return "", err } - - // Return summary, if any. - diffs, err := ctlr.Diff(repo, oldBranch, newBranch) + // Get shortstat summary of the diff + diff, err := ctlr.Diff(repo, oldBranch, repo.Branch) if err != nil { return "", err } - if diffs == "" { - return "", nil - } - diffs = strings.TrimSuffix(diffs, "\n") - return fmt.Sprintf("%s: [Diff](https://github.com/modular-magician/%s/compare/%s..%s) (%s)", repo.Title, repo.Name, oldBranch, newBranch, diffs), nil + return strings.TrimSuffix(diff, "\n"), nil } // Build the diff processor for tpg or tpgb @@ -304,22 +371,27 @@ func buildDiffProcessor(diffProcessorPath, providerLocalPath string, env map[str return rnr.PopDir() } -func computeBreakingChanges(diffProcessorPath string, rnr ExecRunner) (string, error) { +func computeBreakingChanges(diffProcessorPath string, rnr ExecRunner) ([]string, error) { if err := rnr.PushDir(diffProcessorPath); err != nil { - return "", err + return nil, err } - breakingChanges, err := rnr.Run("bin/diff-processor", []string{"breaking-changes"}, nil) + output, err := rnr.Run("bin/diff-processor", []string{"breaking-changes"}, nil) if err != nil { - return "", err + return nil, err + } + + if output == "" { + return nil, nil } - return breakingChanges, rnr.PopDir() + + return strings.Split(strings.TrimSuffix(output, "\n"), "\n"), rnr.PopDir() } -func addLabels(diffProcessorPath string, env map[string]string, rnr ExecRunner) error { +func addLabels(prNumber int, diffProcessorPath string, env map[string]string, rnr ExecRunner) error { if err := rnr.PushDir(diffProcessorPath); err != nil { return err } - output, err := rnr.Run("bin/diff-processor", []string{"add-labels", env["PR_NUMBER"]}, env) + output, err := rnr.Run("bin/diff-processor", []string{"add-labels", strconv.Itoa(prNumber)}, env) fmt.Println(output) if err != nil { return err @@ -336,48 +408,6 @@ func cleanDiffProcessor(diffProcessorPath string, rnr ExecRunner) error { return nil } -// Get the breaking change message including the unique tpg messages and all tpgb messages. -func combineBreakingChanges(tpgBreaking, tpgbBreaking string) string { - var allMessages []string - if tpgBreaking == "" { - if tpgbBreaking == "" { - return "" - } - allMessages = strings.Split(tpgbBreaking, "\n") - } else if tpgbBreaking == "" { - allMessages = strings.Split(tpgBreaking, "\n") - } else { - dashExp := regexp.MustCompile("-.*") - tpgMessages := strings.Split(tpgBreaking, "\n") - tpgbMessages := strings.Split(tpgbBreaking, "\n") - tpgbSet := make(map[string]struct{}, len(tpgbMessages)) - var tpgUnique []string - for _, message := range tpgbMessages { - simple := dashExp.ReplaceAllString(message, "") - tpgbSet[simple] = struct{}{} - } - for _, message := range tpgMessages { - simple := dashExp.ReplaceAllString(message, "") - if _, ok := tpgbSet[simple]; !ok { - tpgUnique = append(tpgUnique, message) - } - } - allMessages = append(tpgUnique, tpgbMessages...) - } - if len(allMessages) > 0 { - return `## Breaking Change(s) Detected -The following breaking change(s) were detected within your pull request. - -* ` + strings.Join(allMessages, "\n* ") + ` - -If you believe this detection to be incorrect please raise the concern with your reviewer. -If you intend to make this change you will need to wait for a [major release](https://www.terraform.io/plugin/sdkv2/best-practices/versioning#example-major-number-increments) window. -An ` + "`override-breaking-change`" + ` label can be added to allow merging. -` - } - return "" -} - // Run the missing test detector and return the results. // Returns an empty string unless there are missing tests. // Error will be nil unless an error occurs during setup. @@ -449,9 +479,24 @@ func updatePackageName(name, path string, rnr ExecRunner) error { return rnr.PopDir() } -// Run unit tests for the missing test detector and diff processor. +// Run unit tests for the missing test detector. // Report results using Github API. -func testTools(mmLocalPath, tpgbLocalPath string, env map[string]string, gh GithubClient, rnr ExecRunner) error { +func runMissingTestUnitTests(mmLocalPath, tpgbLocalPath, targetURL, commitSha string, prNumber int, gh GithubClient, rnr ExecRunner) error { + if err := rnr.PushDir(mmLocalPath); err != nil { + return err + } + + diffs, err := rnr.Run("git", []string{"diff", "HEAD", "origin/main", "tools/missing-test-detector"}, nil) + if err != nil { + return err + } + if diffs == "" { + // Short-circuit if there are no changes to the missing test detector + return rnr.PopDir() + } + + fmt.Printf("Found diffs in missing test detector:\n%s\nRunning tests.\n", diffs) + missingTestDetectorPath := filepath.Join(mmLocalPath, "tools", "missing-test-detector") rnr.PushDir(missingTestDetectorPath) if _, err := rnr.Run("go", []string{"mod", "tidy"}, nil); err != nil { @@ -460,20 +505,33 @@ func testTools(mmLocalPath, tpgbLocalPath string, env map[string]string, gh Gith servicesDir := filepath.Join(tpgbLocalPath, "google-beta", "services") state := "success" if _, err := rnr.Run("go", []string{"test"}, map[string]string{ - "GOPATH": env["GOPATH"], - "HOME": env["HOME"], "SERVICES_DIR": servicesDir, + // Passthrough vars required for a valid build environment. + "GOPATH": os.Getenv("GOPATH"), + "HOME": os.Getenv("HOME"), }); err != nil { fmt.Printf("error from running go test in %s: %v\n", missingTestDetectorPath, err) state = "failure" } - targetURL := fmt.Sprintf("https://console.cloud.google.com/cloud-build/builds;region=global/%s;step=%s?project=%s", env["BUILD_ID"], env["BUILD_STEP"], env["PROJECT_ID"]) - if err := gh.PostBuildStatus(env["PR_NUMBER"], "unit-tests-missing-test-detector", state, targetURL, env["COMMIT_SHA"]); err != nil { + if err := gh.PostBuildStatus(strconv.Itoa(prNumber), "unit-tests-missing-test-detector", state, targetURL, commitSha); err != nil { return err } return rnr.PopDir() } +func formatDiffComment(data diffCommentData) (string, error) { + tmpl, err := template.New("DIFF_COMMENT.md").Parse(diffComment) + if err != nil { + panic(fmt.Sprintf("Unable to parse DIFF_COMMENT.md: %s", err)) + } + sb := new(strings.Builder) + err = tmpl.Execute(sb, data) + if err != nil { + return "", err + } + return sb.String(), nil +} + func init() { rootCmd.AddCommand(generateCommentCmd) } diff --git a/.ci/magician/cmd/generate_comment_test.go b/.ci/magician/cmd/generate_comment_test.go index c8a1c14ed487..456267619bb8 100644 --- a/.ci/magician/cmd/generate_comment_test.go +++ b/.ci/magician/cmd/generate_comment_test.go @@ -16,9 +16,12 @@ package cmd import ( - "magician/source" + "os" "reflect" "testing" + + "github.com/stretchr/testify/assert" + "magician/source" ) func TestExecGenerateComment(t *testing.T) { @@ -27,25 +30,27 @@ func TestExecGenerateComment(t *testing.T) { calledMethods: make(map[string][][]any), } ctlr := source.NewController("/mock/dir/go", "modular-magician", "*******", mr) - env := map[string]string{ - "BUILD_ID": "build1", - "BUILD_STEP": "17", - "COMMIT_SHA": "sha1", - "GITHUB_TOKEN_MAGIC_MODULES": "*******", - "PR_NUMBER": "pr1", - "PROJECT_ID": "project1", - } diffProcessorEnv := map[string]string{ - "BUILD_ID": "build1", - "BUILD_STEP": "17", - "COMMIT_SHA": "sha1", + "NEW_REF": "auto-pr-123456", + "OLD_REF": "auto-pr-123456-old", + "PATH": os.Getenv("PATH"), + "GOPATH": os.Getenv("GOPATH"), + "HOME": os.Getenv("HOME"), + } + addLabelsEnv := map[string]string{ "GITHUB_TOKEN_MAGIC_MODULES": "*******", - "NEW_REF": "auto-pr-pr1", - "OLD_REF": "auto-pr-pr1-old", - "PR_NUMBER": "pr1", - "PROJECT_ID": "project1", } - execGenerateComment(env, gh, mr, ctlr) + execGenerateComment( + 123456, + "*******", + "build1", + "17", + "project1", + "sha1", + gh, + mr, + ctlr, + ) for method, expectedCalls := range map[string][]ParameterList{ "Copy": { @@ -64,25 +69,25 @@ func TestExecGenerateComment(t *testing.T) { {"/mock/dir/magic-modules/tools/diff-processor/bin"}, }, "Run": { - {"/mock/dir/magic-modules/.ci/magician", "git", []string{"clone", "-b", "auto-pr-pr1", "https://modular-magician:*******@github.com/modular-magician/terraform-provider-google", "/mock/dir/tpg"}, map[string]string(nil)}, - {"/mock/dir/tpg", "git", []string{"fetch", "origin", "auto-pr-pr1-old"}, map[string]string(nil)}, - {"/mock/dir/tpg", "git", []string{"diff", "origin/auto-pr-pr1-old", "origin/auto-pr-pr1", "--shortstat"}, map[string]string(nil)}, - {"/mock/dir/magic-modules/.ci/magician", "git", []string{"clone", "-b", "auto-pr-pr1", "https://modular-magician:*******@github.com/modular-magician/terraform-provider-google-beta", "/mock/dir/tpgb"}, map[string]string(nil)}, - {"/mock/dir/tpgb", "git", []string{"fetch", "origin", "auto-pr-pr1-old"}, map[string]string(nil)}, - {"/mock/dir/tpgb", "git", []string{"diff", "origin/auto-pr-pr1-old", "origin/auto-pr-pr1", "--shortstat"}, map[string]string(nil)}, - {"/mock/dir/magic-modules/.ci/magician", "git", []string{"clone", "-b", "auto-pr-pr1", "https://modular-magician:*******@github.com/modular-magician/terraform-google-conversion", "/mock/dir/tfc"}, map[string]string(nil)}, - {"/mock/dir/tfc", "git", []string{"fetch", "origin", "auto-pr-pr1-old"}, map[string]string(nil)}, - {"/mock/dir/tfc", "git", []string{"diff", "origin/auto-pr-pr1-old", "origin/auto-pr-pr1", "--shortstat"}, map[string]string(nil)}, - {"/mock/dir/magic-modules/.ci/magician", "git", []string{"clone", "-b", "auto-pr-pr1", "https://modular-magician:*******@github.com/modular-magician/docs-examples", "/mock/dir/tfoics"}, map[string]string(nil)}, - {"/mock/dir/tfoics", "git", []string{"fetch", "origin", "auto-pr-pr1-old"}, map[string]string(nil)}, - {"/mock/dir/tfoics", "git", []string{"diff", "origin/auto-pr-pr1-old", "origin/auto-pr-pr1", "--shortstat"}, map[string]string(nil)}, + {"/mock/dir/magic-modules/.ci/magician", "git", []string{"clone", "-b", "auto-pr-123456", "https://modular-magician:*******@github.com/modular-magician/terraform-provider-google", "/mock/dir/tpg"}, map[string]string(nil)}, + {"/mock/dir/magic-modules/.ci/magician", "git", []string{"clone", "-b", "auto-pr-123456", "https://modular-magician:*******@github.com/modular-magician/terraform-provider-google-beta", "/mock/dir/tpgb"}, map[string]string(nil)}, + {"/mock/dir/magic-modules/.ci/magician", "git", []string{"clone", "-b", "auto-pr-123456", "https://modular-magician:*******@github.com/modular-magician/terraform-google-conversion", "/mock/dir/tgc"}, map[string]string(nil)}, + {"/mock/dir/magic-modules/.ci/magician", "git", []string{"clone", "-b", "auto-pr-123456", "https://modular-magician:*******@github.com/modular-magician/docs-examples", "/mock/dir/tfoics"}, map[string]string(nil)}, + {"/mock/dir/tpg", "git", []string{"fetch", "origin", "auto-pr-123456-old"}, map[string]string(nil)}, + {"/mock/dir/tpg", "git", []string{"diff", "origin/auto-pr-123456-old", "origin/auto-pr-123456", "--shortstat"}, map[string]string(nil)}, + {"/mock/dir/tpgb", "git", []string{"fetch", "origin", "auto-pr-123456-old"}, map[string]string(nil)}, + {"/mock/dir/tpgb", "git", []string{"diff", "origin/auto-pr-123456-old", "origin/auto-pr-123456", "--shortstat"}, map[string]string(nil)}, + {"/mock/dir/tgc", "git", []string{"fetch", "origin", "auto-pr-123456-old"}, map[string]string(nil)}, + {"/mock/dir/tgc", "git", []string{"diff", "origin/auto-pr-123456-old", "origin/auto-pr-123456", "--shortstat"}, map[string]string(nil)}, + {"/mock/dir/tfoics", "git", []string{"fetch", "origin", "auto-pr-123456-old"}, map[string]string(nil)}, + {"/mock/dir/tfoics", "git", []string{"diff", "origin/auto-pr-123456-old", "origin/auto-pr-123456", "--shortstat"}, map[string]string(nil)}, {"/mock/dir/magic-modules/tools/diff-processor", "make", []string{"build"}, diffProcessorEnv}, {"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"breaking-changes"}, map[string]string(nil)}, - {"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"add-labels", "pr1"}, diffProcessorEnv}, + {"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"add-labels", "123456"}, addLabelsEnv}, {"/mock/dir/magic-modules/tools/diff-processor", "make", []string{"build"}, diffProcessorEnv}, {"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"breaking-changes"}, map[string]string(nil)}, - {"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"add-labels", "pr1"}, diffProcessorEnv}, - {"/mock/dir/tpgbold", "git", []string{"checkout", "origin/auto-pr-pr1-old"}, map[string]string(nil)}, + {"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"add-labels", "123456"}, addLabelsEnv}, + {"/mock/dir/tpgbold", "git", []string{"checkout", "origin/auto-pr-123456-old"}, map[string]string(nil)}, {"/mock/dir/tpgbold", "find", []string{".", "-type", "f", "-name", "*.go", "-exec", "sed", "-i.bak", "s~github.com/hashicorp/terraform-provider-google-beta~google/provider/old~g", "{}", "+"}, map[string]string(nil)}, {"/mock/dir/tpgbold", "sed", []string{"-i.bak", "s|github.com/hashicorp/terraform-provider-google-beta|google/provider/old|g", "go.mod"}, map[string]string(nil)}, {"/mock/dir/tpgbold", "sed", []string{"-i.bak", "s|github.com/hashicorp/terraform-provider-google-beta|google/provider/old|g", "go.sum"}, map[string]string(nil)}, @@ -110,8 +115,8 @@ func TestExecGenerateComment(t *testing.T) { } for method, expectedCalls := range map[string][][]any{ - "PostBuildStatus": {{"pr1", "terraform-provider-breaking-change-test", "success", "https://console.cloud.google.com/cloud-build/builds;region=global/build1;step=17?project=project1", "sha1"}}, - "PostComment": {{"pr1", "Hi there, I'm the Modular magician. I've detected the following information about your changes:\n\n## Diff report\nYour PR generated some diffs in downstreams - here they are.\n\nTerraform GA: [Diff](https://github.com/modular-magician/terraform-provider-google/compare/auto-pr-pr1-old..auto-pr-pr1) ( 2 files changed, 40 insertions(+))\nTerraform Beta: [Diff](https://github.com/modular-magician/terraform-provider-google-beta/compare/auto-pr-pr1-old..auto-pr-pr1) ( 2 files changed, 40 insertions(+))\nTF Conversion: [Diff](https://github.com/modular-magician/terraform-google-conversion/compare/auto-pr-pr1-old..auto-pr-pr1) ( 1 file changed, 10 insertions(+))\n\n## Missing test report\nYour PR includes resource fields which are not covered by any test.\n\nResource: `google_folder_access_approval_settings` (3 total tests)\nPlease add an acceptance test which includes these fields. The test should include the following:\n\n```hcl\nresource \"google_folder_access_approval_settings\" \"primary\" {\n uncovered_field = # value needed\n}\n\n```\n\n"}}, + "PostBuildStatus": {{"123456", "terraform-provider-breaking-change-test", "success", "https://console.cloud.google.com/cloud-build/builds;region=global/build1;step=17?project=project1", "sha1"}}, + "PostComment": {{"123456", "Hi there, I'm the Modular magician. I've detected the following information about your changes:\n\n## Diff report\n\nYour PR generated some diffs in downstreams - here they are.\n\n`google` provider: [Diff](https://github.com/modular-magician/terraform-provider-google/compare/auto-pr-123456-old..auto-pr-123456) ( 2 files changed, 40 insertions(+))\n`google-beta` provider: [Diff](https://github.com/modular-magician/terraform-provider-google-beta/compare/auto-pr-123456-old..auto-pr-123456) ( 2 files changed, 40 insertions(+))\n`terraform-google-conversion`: [Diff](https://github.com/modular-magician/terraform-google-conversion/compare/auto-pr-123456-old..auto-pr-123456) ( 1 file changed, 10 insertions(+))\n\n## Missing test report\nYour PR includes resource fields which are not covered by any test.\n\nResource: `google_folder_access_approval_settings` (3 total tests)\nPlease add an acceptance test which includes these fields. The test should include the following:\n\n```hcl\nresource \"google_folder_access_approval_settings\" \"primary\" {\n uncovered_field = # value needed\n}\n\n```\n"}}, } { if actualCalls, ok := gh.calledMethods[method]; !ok { t.Fatalf("Found no calls for %s", method) @@ -126,3 +131,122 @@ func TestExecGenerateComment(t *testing.T) { } } } + +func TestFormatDiffComment(t *testing.T) { + cases := map[string]struct { + data diffCommentData + expectedStrings []string + notExpectedStrings []string + }{ + "basic message": { + data: diffCommentData{}, + expectedStrings: []string{"## Diff report", "hasn't generated any diffs"}, + notExpectedStrings: []string{ + "generated some diffs", + "## Breaking Change(s) Detected", + "## Errors", + "## Missing test report", + }, + }, + "errors are displayed": { + data: diffCommentData{ + Errors: []Errors{ + { + Title: "`google` provider", + Errors: []string{"Provider 1"}, + }, + { + Title: "Other", + Errors: []string{"Error 1", "Error 2"}, + }, + }, + }, + expectedStrings: []string{"## Diff report", "## Errors", "`google` provider:\n- Provider 1\n\nOther:\n- Error 1\n- Error 2\n"}, + notExpectedStrings: []string{ + "generated some diffs", + "## Breaking Change(s) Detected", + "## Missing test report", + }, + }, + "diffs are displayed": { + data: diffCommentData{ + PrNumber: 1234567890, + Diffs: []Diff{ + { + Title: "Repo 1", + Repo: "repo-1", + DiffStats: "+1 added, -1 removed", + }, + { + Title: "Repo 2", + Repo: "repo-2", + DiffStats: "+2 added, -2 removed", + }, + }, + }, + expectedStrings: []string{ + "## Diff report", + "generated some diffs", + "Repo 1: [Diff](https://github.com/modular-magician/repo-1/compare/auto-pr-1234567890-old..auto-pr-1234567890) (+1 added, -1 removed)\nRepo 2: [Diff](https://github.com/modular-magician/repo-2/compare/auto-pr-1234567890-old..auto-pr-1234567890) (+2 added, -2 removed)", + }, + notExpectedStrings: []string{ + "hasn't generated any diffs", + "## Breaking Change(s) Detected", + "## Errors", + "## Missing test report", + }, + }, + "breaking changes are displayed": { + data: diffCommentData{ + BreakingChanges: []string{ + "Breaking change 1", + "Breaking change 2", + }, + }, + expectedStrings: []string{ + "## Diff report", + "## Breaking Change(s) Detected", + "major release", + "`override-breaking-change`", + "- Breaking change 1\n- Breaking change 2\n", + }, + notExpectedStrings: []string{ + "generated some diffs", + "## Errors", + "## Missing test report", + }, + }, + "missing tests are displayed": { + data: diffCommentData{ + MissingTests: "## Missing test report", + }, + expectedStrings: []string{ + "## Diff report", + "## Missing test report", + }, + notExpectedStrings: []string{ + "generated some diffs", + "## Breaking Change(s) Detected", + "## Errors", + }, + }, + } + + for tn, tc := range cases { + tc := tc + t.Run(tn, func(t *testing.T) { + t.Parallel() + + comment, err := formatDiffComment(tc.data) + assert.Nil(t, err) + + for _, s := range tc.expectedStrings { + assert.Contains(t, comment, s) + } + + for _, s := range tc.notExpectedStrings { + assert.NotContains(t, comment, s) + } + }) + } +} diff --git a/.ci/magician/cmd/mock_runner_test.go b/.ci/magician/cmd/mock_runner_test.go index 5789d9878a3f..bc470b13d685 100644 --- a/.ci/magician/cmd/mock_runner_test.go +++ b/.ci/magician/cmd/mock_runner_test.go @@ -41,32 +41,33 @@ func NewMockRunner() MockRunner { return &mockRunner{ calledMethods: make(map[string][]ParameterList), cmdResults: map[string]string{ - "/mock/dir/tfc git [clone -b auto-pr-pr1 https://modular-magician:*******@github.com/modular-magician/docs-examples /mock/dir/tfoics] map[]": "", - "/mock/dir/tpgb git [clone -b auto-pr-pr1 https://modular-magician:*******@github.com/modular-magician/terraform-google-conversion /mock/dir/tfc] map[]": "", - " git [clone -b auto-pr-pr1 https://modular-magician:*******@github.com/modular-magician/terraform-provider-google /mock/dir/tpg] map[]": "", - "/mock/dir/tpg git [clone -b auto-pr-pr1 https://modular-magician:*******@github.com/modular-magician/terraform-provider-google-beta /mock/dir/tpgb] map[]": "", - "/mock/dir/magic-modules git [diff HEAD origin/main tools/missing-test-detector] map[]": "", - "/mock/dir/magic-modules/tools/diff-processor bin/diff-processor [breaking-changes] map[]": "", - "/mock/dir/magic-modules/tools/diff-processor make [build] map[OLD_REF:auto-pr-pr1-old NEW_REF:auto-pr-pr1]": "", - "/mock/dir/magic-modules/tools/missing-test-detector go [mod edit -replace google/provider/new=/mock/dir/tpgb] map[]": "", - "/mock/dir/magic-modules/tools/missing-test-detector go [mod edit -replace google/provider/old=/mock/dir/tpgbold] map[]": "", - "/mock/dir/magic-modules/tools/missing-test-detector go [mod tidy] map[]": "", - "/mock/dir/magic-modules/tools/missing-test-detector go [run . -services-dir=/mock/dir/tpgb/google-beta/services] map[]": "## Missing test report\nYour PR includes resource fields which are not covered by any test.\n\nResource: `google_folder_access_approval_settings` (3 total tests)\nPlease add an acceptance test which includes these fields. The test should include the following:\n\n```hcl\nresource \"google_folder_access_approval_settings\" \"primary\" {\n uncovered_field = # value needed\n}\n\n```\n", - "/mock/dir/tfc git [diff origin/auto-pr-pr1-old origin/auto-pr-pr1 --shortstat] map[]": " 1 file changed, 10 insertions(+)\n", - "/mock/dir/tfc git [fetch origin auto-pr-pr1-old] map[]": "", - "/mock/dir/tfoics git [diff origin/auto-pr-pr1-old origin/auto-pr-pr1 --shortstat] map[]": "", - "/mock/dir/tfoics git [fetch origin auto-pr-pr1-old] map[]": "", - "/mock/dir/tpg git [diff origin/auto-pr-pr1-old origin/auto-pr-pr1 --shortstat] map[]": " 2 files changed, 40 insertions(+)\n", - "/mock/dir/tpg git [fetch origin auto-pr-pr1-old] map[]": "", - "/mock/dir/tpgb find [. -type f -name *.go -exec sed -i.bak s~github.com/hashicorp/terraform-provider-google-beta~google/provider/new~g {} +] map[]": "", - "/mock/dir/tpgb git [diff origin/auto-pr-pr1-old origin/auto-pr-pr1 --shortstat] map[]": " 2 files changed, 40 insertions(+)\n", - "/mock/dir/tpgb git [fetch origin auto-pr-pr1-old] map[]": "", - "/mock/dir/tpgb sed [-i.bak s|github.com/hashicorp/terraform-provider-google-beta|google/provider/new|g go.mod] map[]": "", - "/mock/dir/tpgb sed [-i.bak s|github.com/hashicorp/terraform-provider-google-beta|google/provider/new|g go.sum] map[]": "", - "/mock/dir/tpgbold find [. -type f -name *.go -exec sed -i.bak s~github.com/hashicorp/terraform-provider-google-beta~google/provider/old~g {} +] map[]": "", - "/mock/dir/tpgbold git [checkout origin/auto-pr-pr1-old] map[]": "", - "/mock/dir/tpgbold sed [-i.bak s|github.com/hashicorp/terraform-provider-google-beta|google/provider/old|g go.mod] map[]": "", - "/mock/dir/tpgbold sed [-i.bak s|github.com/hashicorp/terraform-provider-google-beta|google/provider/old|g go.sum] map[]": "", + "/mock/dir/magic-modules/.ci/magician git [clone -b auto-pr-123456 https://modular-magician:*******@github.com/modular-magician/docs-examples /mock/dir/tfoics] map[]": "", + "/mock/dir/magic-modules/.ci/magician git [clone -b auto-pr-123456 https://modular-magician:*******@github.com/modular-magician/terraform-google-conversion /mock/dir/tgc] map[]": "", + "/mock/dir/magic-modules/.ci/magician git [clone -b auto-pr-123456 https://modular-magician:*******@github.com/modular-magician/terraform-provider-google /mock/dir/tpg] map[]": "", + "/mock/dir/magic-modules/.ci/magician git [clone -b auto-pr-123456 https://modular-magician:*******@github.com/modular-magician/terraform-provider-google-beta /mock/dir/tpgb] map[]": "", + "/mock/dir/magic-modules git [diff HEAD origin/main tools/missing-test-detector] map[]": "", + "/mock/dir/magic-modules/tools/diff-processor bin/diff-processor [breaking-changes] map[]": "", + "/mock/dir/magic-modules/tools/diff-processor make [build] map[NEW_REF:auto-pr-123456 OLD_REF:auto-pr-123456-old]": "", + "/mock/dir/magic-modules/tools/diff-processor bin/diff-processor [add-labels 123456] map[GITHUB_TOKEN_MAGIC_MODULES:*******]": "", + "/mock/dir/magic-modules/tools/missing-test-detector go [mod edit -replace google/provider/new=/mock/dir/tpgb] map[]": "", + "/mock/dir/magic-modules/tools/missing-test-detector go [mod edit -replace google/provider/old=/mock/dir/tpgbold] map[]": "", + "/mock/dir/magic-modules/tools/missing-test-detector go [mod tidy] map[]": "", + "/mock/dir/magic-modules/tools/missing-test-detector go [run . -services-dir=/mock/dir/tpgb/google-beta/services] map[]": "## Missing test report\nYour PR includes resource fields which are not covered by any test.\n\nResource: `google_folder_access_approval_settings` (3 total tests)\nPlease add an acceptance test which includes these fields. The test should include the following:\n\n```hcl\nresource \"google_folder_access_approval_settings\" \"primary\" {\n uncovered_field = # value needed\n}\n\n```\n", + "/mock/dir/tgc git [diff origin/auto-pr-123456-old origin/auto-pr-123456 --shortstat] map[]": " 1 file changed, 10 insertions(+)\n", + "/mock/dir/tgc git [fetch origin auto-pr-123456-old] map[]": "", + "/mock/dir/tfoics git [diff origin/auto-pr-123456-old origin/auto-pr-123456 --shortstat] map[]": "", + "/mock/dir/tfoics git [fetch origin auto-pr-123456-old] map[]": "", + "/mock/dir/tpg git [diff origin/auto-pr-123456-old origin/auto-pr-123456 --shortstat] map[]": " 2 files changed, 40 insertions(+)\n", + "/mock/dir/tpg git [fetch origin auto-pr-123456-old] map[]": "", + "/mock/dir/tpgb find [. -type f -name *.go -exec sed -i.bak s~github.com/hashicorp/terraform-provider-google-beta~google/provider/new~g {} +] map[]": "", + "/mock/dir/tpgb git [diff origin/auto-pr-123456-old origin/auto-pr-123456 --shortstat] map[]": " 2 files changed, 40 insertions(+)\n", + "/mock/dir/tpgb git [fetch origin auto-pr-123456-old] map[]": "", + "/mock/dir/tpgb sed [-i.bak s|github.com/hashicorp/terraform-provider-google-beta|google/provider/new|g go.mod] map[]": "", + "/mock/dir/tpgb sed [-i.bak s|github.com/hashicorp/terraform-provider-google-beta|google/provider/new|g go.sum] map[]": "", + "/mock/dir/tpgbold find [. -type f -name *.go -exec sed -i.bak s~github.com/hashicorp/terraform-provider-google-beta~google/provider/old~g {} +] map[]": "", + "/mock/dir/tpgbold git [checkout origin/auto-pr-123456-old] map[]": "", + "/mock/dir/tpgbold sed [-i.bak s|github.com/hashicorp/terraform-provider-google-beta|google/provider/old|g go.mod] map[]": "", + "/mock/dir/tpgbold sed [-i.bak s|github.com/hashicorp/terraform-provider-google-beta|google/provider/old|g go.sum] map[]": "", }, cwd: "/mock/dir/magic-modules/.ci/magician", dirStack: list.New(), diff --git a/.ci/magician/source/repo.go b/.ci/magician/source/repo.go index 2d62d531431f..529cf97200ee 100644 --- a/.ci/magician/source/repo.go +++ b/.ci/magician/source/repo.go @@ -14,7 +14,7 @@ type Repo struct { Owner string // Owner of repo, optional Path string // local Path once cloned, including Name Version provider.Version - DiffCanFail bool // whether to allow the command to continue if cloning or diffing the repo fails + Cloned bool } type Controller struct { diff --git a/.ci/scripts/go-plus/magician/exec.sh b/.ci/scripts/go-plus/magician/exec.sh index a0cb4078e6b6..c438fb69cb22 100755 --- a/.ci/scripts/go-plus/magician/exec.sh +++ b/.ci/scripts/go-plus/magician/exec.sh @@ -1,5 +1,7 @@ #!/bin/bash +set -e + # Get the directory of the current script DIR="$(dirname $(realpath $0))" diff --git a/tools/diff-processor/GNUmakefile b/tools/diff-processor/GNUmakefile index 8e4f18ba9b05..c9e336230d17 100644 --- a/tools/diff-processor/GNUmakefile +++ b/tools/diff-processor/GNUmakefile @@ -56,3 +56,5 @@ endif go mod tidy mkdir -p bin/ go build -o ./bin/ . + +.PHONY: clone clean build