-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor magician structs #9605
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,8 @@ | ||
| package cloudbuild | ||
|
|
||
| type cloudBuild bool | ||
|
|
||
| type CloudBuild interface { | ||
| ApproveCommunityChecker(prNumber, commitSha string) error | ||
| GetAwaitingApprovalBuildLink(prNumber, commitSha string) (string, error) | ||
| TriggerMMPresubmitRuns(commitSha string, substitutions map[string]string) error | ||
| type Client struct { | ||
| } | ||
|
|
||
| func NewCloudBuildService() CloudBuild { | ||
| var x cloudBuild = true | ||
| return x | ||
| func NewClient() *Client { | ||
| return &Client{} | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,23 +12,7 @@ import ( | |
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| const allowBreakingChangesLabel = 4598495472 | ||
|
|
||
| type gcGithub interface { | ||
| GetPullRequestLabelIDs(prNumber string) (map[int]struct{}, error) | ||
| PostBuildStatus(prNumber, title, state, targetURL, commitSha string) error | ||
| PostComment(prNumber, comment string) error | ||
| } | ||
|
|
||
| type gcRunner interface { | ||
| GetCWD() string | ||
| Copy(src, dest string) error | ||
| RemoveAll(path string) error | ||
| PushDir(path string) error | ||
| PopDir() error | ||
| Run(name string, args, env []string) (string, error) | ||
| MustRun(name string, args, env []string) string | ||
| } | ||
| const allowBreakingChangesLabel = "override-breaking-change" | ||
|
|
||
| type ProviderVersion string | ||
|
|
||
|
|
@@ -74,28 +58,28 @@ var generateCommentCmd = &cobra.Command{ | |
| commit := os.Getenv("COMMIT_SHA") | ||
| fmt.Println("Commit SHA: ", commit) | ||
|
|
||
| pr := os.Getenv("PR_NUMBER") | ||
| fmt.Println("PR Number: ", pr) | ||
| prNumber := os.Getenv("PR_NUMBER") | ||
| fmt.Println("PR Number: ", prNumber) | ||
|
|
||
| githubToken, ok := os.LookupEnv("GITHUB_TOKEN") | ||
| if !ok { | ||
| fmt.Println("Did not provide GITHUB_TOKEN environment variable") | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| gh := github.NewGithubService() | ||
| gh := github.NewClient() | ||
| rnr, err := exec.NewRunner() | ||
| if err != nil { | ||
| fmt.Println("Error creating a runner: ", err) | ||
| os.Exit(1) | ||
| } | ||
| execGenerateComment(buildID, projectID, buildStep, commit, pr, githubToken, gh, rnr) | ||
| execGenerateComment(buildID, projectID, buildStep, commit, prNumber, githubToken, gh, rnr) | ||
| }, | ||
| } | ||
|
|
||
| func execGenerateComment(buildID, projectID, buildStep, commit, pr, githubToken string, gh gcGithub, r gcRunner) { | ||
| newBranch := "auto-pr-" + pr | ||
| oldBranch := "auto-pr-" + pr + "-old" | ||
| func execGenerateComment(buildID, projectID, buildStep, commit, prNumber, githubToken string, gh GithubClient, r ExecRunner) { | ||
| newBranch := "auto-pr-" + prNumber | ||
| oldBranch := "auto-pr-" + prNumber + "-old" | ||
| wd := r.GetCWD() | ||
| mmLocalPath := filepath.Join(wd, "..", "..") | ||
| tpgRepoName := "terraform-provider-google" | ||
|
|
@@ -173,7 +157,7 @@ func execGenerateComment(buildID, projectID, buildStep, commit, pr, githubToken | |
| showBreakingChangesFailed = true | ||
| } | ||
| versionedBreakingChanges[repo.Version] = output | ||
| err = addLabels(diffProcessorPath, githubToken, pr, r) | ||
| err = addLabels(diffProcessorPath, githubToken, prNumber, r) | ||
| if err != nil { | ||
| fmt.Println("Error adding TPG labels to PR: ", err) | ||
| } | ||
|
|
@@ -204,12 +188,20 @@ The breaking change detector crashed during execution. This is usually due to th | |
| if breakingChanges != "" { | ||
| message += breakingChanges + "\n\n" | ||
|
|
||
| labels, err := gh.GetPullRequestLabelIDs(pr) | ||
| pullRequest, err := gh.GetPullRequest(prNumber) | ||
| if err != nil { | ||
| fmt.Printf("Error getting pull request labels: %v\n", err) | ||
| fmt.Printf("Error getting pull request: %v\n", err) | ||
| os.Exit(1) | ||
| } | ||
| if _, ok := labels[allowBreakingChangesLabel]; !ok { | ||
|
|
||
| breakingChangesAllowed := false | ||
| for _, label := range pullRequest.Labels { | ||
| if label.Name == allowBreakingChangesLabel { | ||
| breakingChangesAllowed = true | ||
| break | ||
| } | ||
| } | ||
| if !breakingChangesAllowed { | ||
| breakingState = "failure" | ||
| } | ||
| } | ||
|
|
@@ -223,13 +215,13 @@ The breaking change detector crashed during execution. This is usually due to th | |
| } | ||
| } | ||
|
|
||
| if err := gh.PostComment(pr, message); err != nil { | ||
| fmt.Printf("Error posting comment to PR %s: %v\n", pr, err) | ||
| if err := gh.PostComment(prNumber, message); err != nil { | ||
| fmt.Printf("Error posting comment to PR %s: %v\n", prNumber, err) | ||
| } | ||
|
|
||
| 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(pr, "terraform-provider-breaking-change-test", breakingState, targetURL, commit); err != nil { | ||
| fmt.Printf("Error posting build status for pr %s commit %s: %v\n", pr, commit, err) | ||
| if err := gh.PostBuildStatus(prNumber, "terraform-provider-breaking-change-test", breakingState, targetURL, commit); err != nil { | ||
| fmt.Printf("Error posting build status for pr %s commit %s: %v\n", prNumber, commit, err) | ||
| os.Exit(1) | ||
| } | ||
|
|
||
|
|
@@ -239,7 +231,7 @@ The breaking change detector crashed during execution. This is usually due to th | |
| } | ||
| if diffs := r.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, pr, commit, buildID, buildStep, projectID, gh, r); err != nil { | ||
| if err := testTools(mmLocalPath, tpgbLocalPath, prNumber, commit, buildID, buildStep, projectID, gh, r); err != nil { | ||
| fmt.Printf("Error testing tools in %s: %v\n", mmLocalPath, err) | ||
| os.Exit(1) | ||
| } | ||
|
|
@@ -250,7 +242,7 @@ The breaking change detector crashed during execution. This is usually due to th | |
| } | ||
| } | ||
|
|
||
| func cloneAndDiff(repo Repository, oldBranch, newBranch, githubToken string, r gcRunner) (string, error) { | ||
| func cloneAndDiff(repo Repository, oldBranch, newBranch, githubToken string, r ExecRunner) (string, error) { | ||
| // Clone the repo to the desired repo.Path. | ||
| url := fmt.Sprintf("https://modular-magician:%[email protected]/modular-magician/%s", githubToken, repo.Name) | ||
| if _, err := r.Run("git", []string{"clone", "-b", newBranch, url, repo.Path}, nil); err != nil { | ||
|
|
@@ -276,7 +268,7 @@ func cloneAndDiff(repo Repository, oldBranch, newBranch, githubToken string, r g | |
| } | ||
|
|
||
| // Build the diff processor for tpg or tpgb | ||
| func buildDiffProcessor(diffProcessorPath, providerLocalPath, oldBranch, newBranch string, r gcRunner) error { | ||
| func buildDiffProcessor(diffProcessorPath, providerLocalPath, oldBranch, newBranch string, r ExecRunner) error { | ||
| if err := r.PushDir(diffProcessorPath); err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -291,7 +283,7 @@ func buildDiffProcessor(diffProcessorPath, providerLocalPath, oldBranch, newBran | |
| return r.PopDir() | ||
| } | ||
|
|
||
| func computeBreakingChanges(diffProcessorPath string, r gcRunner) (string, error) { | ||
| func computeBreakingChanges(diffProcessorPath string, r ExecRunner) (string, error) { | ||
| if err := r.PushDir(diffProcessorPath); err != nil { | ||
| return "", err | ||
| } | ||
|
|
@@ -302,19 +294,19 @@ func computeBreakingChanges(diffProcessorPath string, r gcRunner) (string, error | |
| return breakingChanges, r.PopDir() | ||
| } | ||
|
|
||
| func addLabels(diffProcessorPath, githubToken, pr string, r gcRunner) error { | ||
| func addLabels(diffProcessorPath, githubToken, prNumber string, r ExecRunner) error { | ||
| if err := r.PushDir(diffProcessorPath); err != nil { | ||
| return err | ||
| } | ||
| output, err := r.Run("bin/diff-processor", []string{"add-labels", pr}, []string{fmt.Sprintf("GITHUB_TOKEN=%s", githubToken)}) | ||
| output, err := r.Run("bin/diff-processor", []string{"add-labels", prNumber}, []string{fmt.Sprintf("GITHUB_TOKEN=%s", githubToken)}) | ||
| fmt.Println(output) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| return r.PopDir() | ||
| } | ||
|
|
||
| func cleanDiffProcessor(diffProcessorPath string, r gcRunner) error { | ||
| func cleanDiffProcessor(diffProcessorPath string, r ExecRunner) error { | ||
| for _, path := range []string{"old", "new", "bin"} { | ||
| if err := r.RemoveAll(filepath.Join(diffProcessorPath, path)); err != nil { | ||
| return err | ||
|
|
@@ -368,7 +360,7 @@ An ` + "`override-breaking-change`" + `label can be added to allow merging. | |
| // 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. | ||
| func detectMissingTests(mmLocalPath, tpgbLocalPath, oldBranch string, r gcRunner) (string, error) { | ||
| func detectMissingTests(mmLocalPath, tpgbLocalPath, oldBranch string, r ExecRunner) (string, error) { | ||
| tpgbLocalPathOld := tpgbLocalPath + "old" | ||
|
|
||
| if err := r.Copy(tpgbLocalPath, tpgbLocalPathOld); err != nil { | ||
|
|
@@ -417,7 +409,7 @@ func detectMissingTests(mmLocalPath, tpgbLocalPath, oldBranch string, r gcRunner | |
|
|
||
| // Update the provider package name to the given name in the given path. | ||
| // name should be either "old" or "new". | ||
| func updatePackageName(name, path string, r gcRunner) error { | ||
| func updatePackageName(name, path string, r ExecRunner) error { | ||
| oldPackageName := "github.com/hashicorp/terraform-provider-google-beta" | ||
| newPackageName := "google/provider/" + name | ||
| fmt.Printf("Updating package name in %s from %s to %s\n", path, oldPackageName, newPackageName) | ||
|
|
@@ -438,7 +430,7 @@ func updatePackageName(name, path string, r gcRunner) error { | |
|
|
||
| // Run unit tests for the missing test detector and diff processor. | ||
| // Report results using Github API. | ||
| func testTools(mmLocalPath, tpgbLocalPath, pr, commit, buildID, buildStep, projectID string, gh gcGithub, r gcRunner) error { | ||
| func testTools(mmLocalPath, tpgbLocalPath, prNumber, commit, buildID, buildStep, projectID string, gh GithubClient, r ExecRunner) error { | ||
| missingTestDetectorPath := filepath.Join(mmLocalPath, "tools", "missing-test-detector") | ||
| r.PushDir(missingTestDetectorPath) | ||
| if _, err := r.Run("go", []string{"mod", "tidy"}, nil); err != nil { | ||
|
|
@@ -451,7 +443,7 @@ func testTools(mmLocalPath, tpgbLocalPath, pr, commit, buildID, buildStep, proje | |
| state = "failure" | ||
| } | ||
| 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(pr, "unit-tests-missing-test-detector", state, targetURL, commit); err != nil { | ||
| if err := gh.PostBuildStatus(prNumber, "unit-tests-missing-test-detector", state, targetURL, commit); err != nil { | ||
| return err | ||
| } | ||
| return r.PopDir() | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not strictly necessary but I think it might be useful to disambiguate since this is inside the cloudbuild package.