diff --git a/.ci/magician/cmd/generate_comment.go b/.ci/magician/cmd/generate_comment.go index 04a4c26fdb97..4882435ad574 100644 --- a/.ci/magician/cmd/generate_comment.go +++ b/.ci/magician/cmd/generate_comment.go @@ -136,12 +136,58 @@ func execGenerateComment(buildID, projectID, buildStep, commit, pr, githubToken diffs += repoDiffs } - breakingChanges, err := detectBreakingChanges(mmLocalPath, tpgLocalPath, tpgbLocalPath, oldBranch, newBranch, r) + // TPG diff processor + showBreakingChangesFailed := false + err = buildDiffProcessor(mmLocalPath, tpgLocalPath, oldBranch, newBranch, r) if err != nil { - fmt.Println("Error setting up breaking change detector: ", err) + fmt.Println(err) + os.Exit(1) + } + tpgBreaking, err := computeBreakingChanges(mmLocalPath, r) + if err != nil { + fmt.Println("Error computing TPG breaking changes: ", err) + showBreakingChangesFailed = true + } + err = addLabels(mmLocalPath, githubToken, pr, r) + if err != nil { + fmt.Println("Error adding TPG labels to PR: ", err) + } + err = cleanDiffProcessor(mmLocalPath, r) + if err != nil { + fmt.Println("Error cleaning up diff processor: ", err) os.Exit(1) } + // TPGB diff processor + err = buildDiffProcessor(mmLocalPath, tpgbLocalPath, oldBranch, newBranch, r) + if err != nil { + fmt.Println(err) + os.Exit(1) + } + tpgbBreaking, err := computeBreakingChanges(mmLocalPath, r) + if err != nil { + fmt.Println("Error computing TPGB breaking changes: ", err) + showBreakingChangesFailed = true + } + err = addLabels(mmLocalPath, githubToken, pr, r) + if err != nil { + fmt.Println("Error adding TPGB labels to PR: ", err) + } + err = cleanDiffProcessor(mmLocalPath, r) + if err != nil { + fmt.Println("Error cleaning up diff processor: ", err) + os.Exit(1) + } + + 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(tpgBreaking, tpgbBreaking) + } + + // Missing test detector missingTests, err := detectMissingTests(mmLocalPath, tpgbLocalPath, oldBranch, r) if err != nil { fmt.Println("Error setting up missing test detector: ", err) @@ -210,54 +256,51 @@ func cloneAndDiff(repoName, path, oldBranch, newBranch, diffTitle, githubToken s return "", nil } -// Run the breaking change detector and return the results. -// Returns an empty string unless there are breaking changes or the detector failed. -// Error will be nil unless an error occurs manipulating files. -func detectBreakingChanges(mmLocalPath, tpgLocalPath, tpgbLocalPath, oldBranch, newBranch string, r gcRunner) (string, error) { - // Breaking change setup and execution +// Build the diff processor for tpg or tpgb +func buildDiffProcessor(mmLocalPath, providerLocalPath, oldBranch, newBranch string, r gcRunner) error { diffProcessorPath := filepath.Join(mmLocalPath, "tools", "diff-processor") + r.Chdir(diffProcessorPath) for _, path := range []string{"old", "new"} { - if err := r.Copy(tpgLocalPath, filepath.Join(diffProcessorPath, path)); err != nil { - return "", err + if err := r.Copy(providerLocalPath, filepath.Join(diffProcessorPath, path)); err != nil { + return err } } - var tpgBreaking, tpgbBreaking, breakingChanges string - var diffProccessorErr error - r.Chdir(diffProcessorPath) if _, err := r.Run("make", []string{"build"}, []string{"OLD_REF=" + oldBranch, "NEW_REF=" + newBranch}); err != nil { - fmt.Printf("Error running make build in %s: %v\n", diffProcessorPath, err) - diffProccessorErr = err - } else { - tpgBreaking, err = r.Run("bin/diff-processor", []string{"breaking-changes"}, nil) - if err != nil { - fmt.Println("Diff processor error: ", err) - diffProccessorErr = err - } + return fmt.Errorf("Error running make build in %s: %v\n", diffProcessorPath, err) + } + return nil +} + +func computeBreakingChanges(mmLocalPath string, r gcRunner) (string, error) { + diffProcessorPath := filepath.Join(mmLocalPath, "tools", "diff-processor") + r.Chdir(diffProcessorPath) + breakingChanges, err := r.Run("bin/diff-processor", []string{"breaking-changes"}, nil) + if err != nil { + return "", err } + return breakingChanges, nil +} + +func addLabels(mmLocalPath, githubToken, pr string, r gcRunner) error { + diffProcessorPath := filepath.Join(mmLocalPath, "tools", "diff-processor") + r.Chdir(diffProcessorPath) + output, err := r.Run("bin/diff-processor", []string{"add-labels", pr}, []string{fmt.Sprintf("GITHUB_TOKEN=%s", githubToken)}) + fmt.Println(output) + return err +} + +func cleanDiffProcessor(mmLocalPath string, r gcRunner) error { + diffProcessorPath := filepath.Join(mmLocalPath, "tools", "diff-processor") for _, path := range []string{"old", "new", "bin"} { if err := r.RemoveAll(filepath.Join(diffProcessorPath, path)); err != nil { - return "", err + return err } } - for _, path := range []string{"old", "new"} { - if err := r.Copy(tpgbLocalPath, filepath.Join(diffProcessorPath, path)); err != nil { - return "", err - } - } - - if diffProccessorErr != nil { - fmt.Println("Breaking changes failed") - 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 { - fmt.Println("Breaking changes succeeded") - breakingChanges = compareBreakingChanges(tpgBreaking, tpgbBreaking) - } - return breakingChanges, nil + return nil } // Get the breaking change message including the unique tpg messages and all tpgb messages. -func compareBreakingChanges(tpgBreaking, tpgbBreaking string) string { +func combineBreakingChanges(tpgBreaking, tpgbBreaking string) string { var allMessages []string if tpgBreaking == "" { if tpgbBreaking == "" { diff --git a/.ci/magician/cmd/generate_comment_test.go b/.ci/magician/cmd/generate_comment_test.go index a480217fd1a0..fe3ab5646624 100644 --- a/.ci/magician/cmd/generate_comment_test.go +++ b/.ci/magician/cmd/generate_comment_test.go @@ -54,6 +54,9 @@ func TestExecGenerateComment(t *testing.T) { {"/mock/dir/magic-modules/tools/diff-processor/old"}, {"/mock/dir/magic-modules/tools/diff-processor/new"}, {"/mock/dir/magic-modules/tools/diff-processor/bin"}, + {"/mock/dir/magic-modules/tools/diff-processor/old"}, + {"/mock/dir/magic-modules/tools/diff-processor/new"}, + {"/mock/dir/magic-modules/tools/diff-processor/bin"}, }, "Run": { {"", "git", []string{"clone", "-b", "auto-pr-pr1", "https://modular-magician:*******@github.com/modular-magician/terraform-provider-google", "/mock/dir/tpg"}, []string(nil)}, @@ -70,6 +73,10 @@ func TestExecGenerateComment(t *testing.T) { {"/mock/dir/tfoics", "git", []string{"diff", "origin/auto-pr-pr1-old", "origin/auto-pr-pr1", "--shortstat"}, []string(nil)}, {"/mock/dir/magic-modules/tools/diff-processor", "make", []string{"build"}, []string{"OLD_REF=auto-pr-pr1-old", "NEW_REF=auto-pr-pr1"}}, {"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"breaking-changes"}, []string(nil)}, + {"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"add-labels", "pr1"}, []string{"GITHUB_TOKEN=*******"}}, + {"/mock/dir/magic-modules/tools/diff-processor", "make", []string{"build"}, []string{"OLD_REF=auto-pr-pr1-old", "NEW_REF=auto-pr-pr1"}}, + {"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"breaking-changes"}, []string(nil)}, + {"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"add-labels", "pr1"}, []string{"GITHUB_TOKEN=*******"}}, {"/mock/dir/tpgbold", "git", []string{"checkout", "origin/auto-pr-pr1-old"}, []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", "{}", "+"}, []string(nil)}, {"/mock/dir/tpgbold", "sed", []string{"-i.bak", "s|github.com/hashicorp/terraform-provider-google-beta|google/provider/old|g", "go.mod"}, []string(nil)}, diff --git a/.ci/scripts/go-plus/github-differ/generate_comment.sh b/.ci/scripts/go-plus/github-differ/generate_comment.sh index 455d6fa63aa6..191dde497fb4 100755 --- a/.ci/scripts/go-plus/github-differ/generate_comment.sh +++ b/.ci/scripts/go-plus/github-differ/generate_comment.sh @@ -57,12 +57,14 @@ if ! git diff --exit-code origin/$OLD_BRANCH origin/$NEW_BRANCH; then fi popd -## Breaking change setup and execution +## Diff processor - TPG set +e pushd $MM_LOCAL_PATH/tools/diff-processor cp -r $TPG_LOCAL_PATH old/ cp -r $TPG_LOCAL_PATH new/ make build OLD_REF=$OLD_BRANCH NEW_REF=$NEW_BRANCH + +### Breaking changes TPG_BREAKING="$(bin/diff-processor breaking-changes)" retVal=$? if [ $retVal -ne 0 ]; then @@ -70,16 +72,35 @@ if [ $retVal -ne 0 ]; then BREAKING_CHANGE_BUILD_FAILURE=1 fi +### Add labels +GITHUB_TOKEN=$GITHUB_TOKEN $MM_LOCAL_PATH/tools/diff-processor/bin/diff-processor add-labels $PR_NUMBER rm -rf ./old/ ./new/ ./bin/ +popd +set -e + +## Diff processor - TPGB +set +e +pushd $MM_LOCAL_PATH/tools/diff-processor cp -r $TPGB_LOCAL_PATH old/ cp -r $TPGB_LOCAL_PATH new/ make build OLD_REF=$OLD_BRANCH NEW_REF=$NEW_BRANCH + +### Breaking changes TPGB_BREAKING="$(bin/diff-processor breaking-changes)" retVal=$? if [ $retVal -ne 0 ]; then TPGB_BREAKING="" BREAKING_CHANGE_BUILD_FAILURE=1 fi + +### Add labels +GITHUB_TOKEN=$GITHUB_TOKEN $MM_LOCAL_PATH/tools/diff-processor/bin/diff-processor add-labels $PR_NUMBER +rm -rf ./old/ ./new/ ./bin/ +popd +set -e + +## Report breaking change failures +set +e if [ $BREAKING_CHANGE_BUILD_FAILURE -eq 0 ]; then echo "Breaking changes succeeded" # Export variables here so that they can be used in compare_breaking_changes @@ -91,7 +112,6 @@ else echo "Breaking changes failed" BREAKINGCHANGES="## Breaking Change Detection Failed${NEWLINE}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." fi -popd set -e ## Missing test setup and execution diff --git a/tools/issue-labeler/labeler/backfill.go b/tools/issue-labeler/labeler/backfill.go index dd19ebc00091..12db0cc04238 100644 --- a/tools/issue-labeler/labeler/backfill.go +++ b/tools/issue-labeler/labeler/backfill.go @@ -146,7 +146,7 @@ func UpdateIssues(repository string, issueUpdates []IssueUpdate, dryRun bool) { } fmt.Printf("Existing labels: %v\n", issueUpdate.OldLabels) fmt.Printf("New labels: %v\n", issueUpdate.Labels) - fmt.Printf("%s %s (https://github.com/%s/issues/%d)\n", repository, req.Method, req.URL, issueUpdate.Number) + fmt.Printf("%s %s (https://github.com/%s/issues/%d)\n", req.Method, req.URL, repository, issueUpdate.Number) b, err := json.MarshalIndent(updateBody, "", " ") if err != nil { glog.Errorf("Error marshalling json: %v", err) @@ -167,9 +167,11 @@ func UpdateIssues(repository string, issueUpdates []IssueUpdate, dryRun bool) { var errResp ErrorResponse json.Unmarshal(body, &errResp) if errResp.Message != "" { - glog.Errorf("API error: %s", errResp.Message) + fmt.Printf("API error: %s", errResp.Message) + continue } } + fmt.Printf("GitHub Issue %s %d updated successfully", repository, issueUpdate.Number) } }