From 455b727ff2cf1d5ef41c4f65c56dfc5b74c2d513 Mon Sep 17 00:00:00 2001 From: Thomas Rodgers Date: Fri, 16 Aug 2024 17:22:56 +0000 Subject: [PATCH] Revert "chore(ci): move ExecRunner interface to exec package" (#11452) --- .ci/magician/cmd/generate_comment.go | 10 +++++----- .ci/magician/cmd/generate_downstream.go | 14 +++++++------- .ci/magician/cmd/interfaces.go | 12 ++++++++++++ .ci/magician/cmd/mock_runner_test.go | 3 +-- .ci/magician/cmd/test_terraform_vcr.go | 2 +- .ci/magician/cmd/test_tgc_integration.go | 2 +- .ci/magician/cmd/vcr_cassette_update.go | 8 ++++---- .ci/magician/{exec => vcr}/interfaces.go | 6 +++--- .ci/magician/vcr/tester.go | 5 ++--- 9 files changed, 36 insertions(+), 26 deletions(-) rename .ci/magician/{exec => vcr}/interfaces.go (97%) diff --git a/.ci/magician/cmd/generate_comment.go b/.ci/magician/cmd/generate_comment.go index 52b450a63571..0efc3fc65ba8 100644 --- a/.ci/magician/cmd/generate_comment.go +++ b/.ci/magician/cmd/generate_comment.go @@ -151,7 +151,7 @@ func listGCEnvironmentVariables() string { return result } -func execGenerateComment(prNumber int, ghTokenMagicModules, buildId, buildStep, projectId, commitSha string, gh GithubClient, rnr exec.ExecRunner, ctlr *source.Controller) error { +func execGenerateComment(prNumber int, ghTokenMagicModules, buildId, buildStep, projectId, commitSha string, gh GithubClient, rnr ExecRunner, ctlr *source.Controller) error { errors := map[string][]string{"Other": []string{}} // TODO(ScottSuarez) - temporary fix to ensure the label is removed. @@ -420,7 +420,7 @@ func execGenerateComment(prNumber int, ghTokenMagicModules, buildId, buildStep, } // Build the diff processor for tpg or tpgb -func buildDiffProcessor(diffProcessorPath, providerLocalPath string, env map[string]string, rnr exec.ExecRunner) error { +func buildDiffProcessor(diffProcessorPath, providerLocalPath string, env map[string]string, rnr ExecRunner) error { for _, path := range []string{"old", "new", "bin"} { if err := rnr.RemoveAll(filepath.Join(diffProcessorPath, path)); err != nil { return err @@ -440,7 +440,7 @@ func buildDiffProcessor(diffProcessorPath, providerLocalPath string, env map[str return rnr.PopDir() } -func computeBreakingChanges(diffProcessorPath string, rnr exec.ExecRunner) ([]BreakingChange, error) { +func computeBreakingChanges(diffProcessorPath string, rnr ExecRunner) ([]BreakingChange, error) { if err := rnr.PushDir(diffProcessorPath); err != nil { return nil, err } @@ -460,7 +460,7 @@ func computeBreakingChanges(diffProcessorPath string, rnr exec.ExecRunner) ([]Br return changes, rnr.PopDir() } -func changedSchemaResources(diffProcessorPath string, rnr exec.ExecRunner) ([]string, error) { +func changedSchemaResources(diffProcessorPath string, rnr ExecRunner) ([]string, error) { if err := rnr.PushDir(diffProcessorPath); err != nil { return nil, err } @@ -486,7 +486,7 @@ func changedSchemaResources(diffProcessorPath string, rnr exec.ExecRunner) ([]st // 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(diffProcessorPath, tpgbLocalPath string, rnr exec.ExecRunner) (map[string]*MissingTestInfo, error) { +func detectMissingTests(diffProcessorPath, tpgbLocalPath string, rnr ExecRunner) (map[string]*MissingTestInfo, error) { if err := rnr.PushDir(diffProcessorPath); err != nil { return nil, err } diff --git a/.ci/magician/cmd/generate_downstream.go b/.ci/magician/cmd/generate_downstream.go index 3159e5745d6e..5a056dafef55 100644 --- a/.ci/magician/cmd/generate_downstream.go +++ b/.ci/magician/cmd/generate_downstream.go @@ -93,7 +93,7 @@ func listGDEnvironmentVariables() string { return result } -func execGenerateDownstream(baseBranch, command, repo, version, ref string, gh GithubClient, rnr exec.ExecRunner, ctlr *source.Controller) error { +func execGenerateDownstream(baseBranch, command, repo, version, ref string, gh GithubClient, rnr ExecRunner, ctlr *source.Controller) error { if baseBranch == "" { baseBranch = "main" } @@ -177,7 +177,7 @@ func execGenerateDownstream(baseBranch, command, repo, version, ref string, gh G return nil } -func cloneRepo(mmRepo *source.Repo, baseBranch, repo, version, command, ref string, rnr exec.ExecRunner, ctlr *source.Controller) (*source.Repo, *source.Repo, string, error) { +func cloneRepo(mmRepo *source.Repo, baseBranch, repo, version, command, ref string, rnr ExecRunner, ctlr *source.Controller) (*source.Repo, *source.Repo, string, error) { downstreamRepo := &source.Repo{ Title: repo, Branch: baseBranch, @@ -244,7 +244,7 @@ func cloneRepo(mmRepo *source.Repo, baseBranch, repo, version, command, ref stri return downstreamRepo, scratchRepo, commitMessage, nil } -func setGitConfig(rnr exec.ExecRunner) error { +func setGitConfig(rnr ExecRunner) error { if _, err := rnr.Run("git", []string{"config", "--local", "user.name", "Modular Magician"}, nil); err != nil { return err } @@ -254,7 +254,7 @@ func setGitConfig(rnr exec.ExecRunner) error { return nil } -func runMake(downstreamRepo *source.Repo, command string, rnr exec.ExecRunner) error { +func runMake(downstreamRepo *source.Repo, command string, rnr ExecRunner) error { switch downstreamRepo.Title { case "terraform-google-conversion": if _, err := rnr.Run("make", []string{"clean-tgc", "OUTPUT_PATH=" + downstreamRepo.Path}, nil); err != nil { @@ -308,7 +308,7 @@ func getPullRequest(baseBranch, ref string, gh GithubClient) (*github.PullReques return nil, fmt.Errorf("no pr found with merge commit sha %s and base branch %s", ref, baseBranch) } -func createCommit(scratchRepo *source.Repo, commitMessage string, rnr exec.ExecRunner) (string, error) { +func createCommit(scratchRepo *source.Repo, commitMessage string, rnr ExecRunner) (string, error) { if err := rnr.PushDir(scratchRepo.Path); err != nil { return "", err } @@ -346,7 +346,7 @@ func createCommit(scratchRepo *source.Repo, commitMessage string, rnr exec.ExecR return commitSha, err } -func addChangelogEntry(downstreamRepo *source.Repo, pullRequest *github.PullRequest, rnr exec.ExecRunner) error { +func addChangelogEntry(downstreamRepo *source.Repo, pullRequest *github.PullRequest, rnr ExecRunner) error { if err := rnr.PushDir(downstreamRepo.Path); err != nil { return err } @@ -357,7 +357,7 @@ func addChangelogEntry(downstreamRepo *source.Repo, pullRequest *github.PullRequ return rnr.PopDir() } -func mergePullRequest(downstreamRepo, scratchRepo *source.Repo, scratchRepoSha string, pullRequest *github.PullRequest, rnr exec.ExecRunner, gh GithubClient) error { +func mergePullRequest(downstreamRepo, scratchRepo *source.Repo, scratchRepoSha string, pullRequest *github.PullRequest, rnr ExecRunner, gh GithubClient) error { fmt.Printf(`Base: %s:%s Head: %s:%s `, downstreamRepo.Owner, downstreamRepo.Branch, scratchRepo.Owner, scratchRepo.Branch) diff --git a/.ci/magician/cmd/interfaces.go b/.ci/magician/cmd/interfaces.go index 13ec1b16375a..948ab794c907 100644 --- a/.ci/magician/cmd/interfaces.go +++ b/.ci/magician/cmd/interfaces.go @@ -39,3 +39,15 @@ type CloudbuildClient interface { ApproveCommunityChecker(prNumber, commitSha string) error TriggerMMPresubmitRuns(commitSha string, substitutions map[string]string) error } + +type ExecRunner interface { + GetCWD() string + Copy(src, dest string) error + Mkdir(path string) error + RemoveAll(path string) error + PushDir(path string) error + PopDir() error + WriteFile(name, data string) error + Run(name string, args []string, env map[string]string) (string, error) + MustRun(name string, args []string, env map[string]string) string +} diff --git a/.ci/magician/cmd/mock_runner_test.go b/.ci/magician/cmd/mock_runner_test.go index ae354d7f488f..c3a1abccbde3 100644 --- a/.ci/magician/cmd/mock_runner_test.go +++ b/.ci/magician/cmd/mock_runner_test.go @@ -20,7 +20,6 @@ import ( "errors" "fmt" "log" - "magician/exec" "os" "path/filepath" "sort" @@ -32,7 +31,7 @@ import ( type ParameterList []any type MockRunner interface { - exec.ExecRunner + ExecRunner Calls(method string) ([]ParameterList, bool) } diff --git a/.ci/magician/cmd/test_terraform_vcr.go b/.ci/magician/cmd/test_terraform_vcr.go index 18c3a2972c20..3b976fece0d4 100644 --- a/.ci/magician/cmd/test_terraform_vcr.go +++ b/.ci/magician/cmd/test_terraform_vcr.go @@ -133,7 +133,7 @@ var testTerraformVCRCmd = &cobra.Command{ }, } -func execTestTerraformVCR(prNumber, mmCommitSha, buildID, projectID, buildStep, baseBranch string, gh GithubClient, rnr exec.ExecRunner, ctlr *source.Controller, vt *vcr.Tester) error { +func execTestTerraformVCR(prNumber, mmCommitSha, buildID, projectID, buildStep, baseBranch string, gh GithubClient, rnr ExecRunner, ctlr *source.Controller, vt *vcr.Tester) error { newBranch := "auto-pr-" + prNumber oldBranch := newBranch + "-old" diff --git a/.ci/magician/cmd/test_tgc_integration.go b/.ci/magician/cmd/test_tgc_integration.go index b518f81d1310..f5b6dfb68e0b 100644 --- a/.ci/magician/cmd/test_tgc_integration.go +++ b/.ci/magician/cmd/test_tgc_integration.go @@ -44,7 +44,7 @@ var testTGCIntegrationCmd = &cobra.Command{ }, } -func execTestTGCIntegration(prNumber, mmCommit, buildID, projectID, buildStep, ghRepo, githubUsername string, rnr exec.ExecRunner, ctlr *source.Controller, gh GithubClient) error { +func execTestTGCIntegration(prNumber, mmCommit, buildID, projectID, buildStep, ghRepo, githubUsername string, rnr ExecRunner, ctlr *source.Controller, gh GithubClient) error { newBranch := "auto-pr-" + prNumber repo := &source.Repo{ Name: ghRepo, diff --git a/.ci/magician/cmd/vcr_cassette_update.go b/.ci/magician/cmd/vcr_cassette_update.go index 2ff3abe10dbf..4e59b1cfed81 100644 --- a/.ci/magician/cmd/vcr_cassette_update.go +++ b/.ci/magician/cmd/vcr_cassette_update.go @@ -100,7 +100,7 @@ var vcrCassetteUpdateCmd = &cobra.Command{ }, } -func execVCRCassetteUpdate(buildID, today string, rnr exec.ExecRunner, ctlr *source.Controller, vt *vcr.Tester) error { +func execVCRCassetteUpdate(buildID, today string, rnr ExecRunner, ctlr *source.Controller, vt *vcr.Tester) error { if err := vt.FetchCassettes(provider.Beta, "main", ""); err != nil { return fmt.Errorf("error fetching cassettes: %w", err) } @@ -199,15 +199,15 @@ func execVCRCassetteUpdate(buildID, today string, rnr exec.ExecRunner, ctlr *sou return nil } -func uploadLogsToGCS(src, dest string, rnr exec.ExecRunner) (string, error) { +func uploadLogsToGCS(src, dest string, rnr ExecRunner) (string, error) { return uploadToGCS(src, dest, []string{"-h", "Content-Type:text/plain", "-q", "cp", "-r"}, rnr) } -func uploadCassettesToGCS(src, dest string, rnr exec.ExecRunner) (string, error) { +func uploadCassettesToGCS(src, dest string, rnr ExecRunner) (string, error) { return uploadToGCS(src, dest, []string{"-m", "-q", "cp"}, rnr) } -func uploadToGCS(src, dest string, opts []string, rnr exec.ExecRunner) (string, error) { +func uploadToGCS(src, dest string, opts []string, rnr ExecRunner) (string, error) { fmt.Printf("uploading from %s to %s\n", src, dest) args := append(opts, src, dest) fmt.Println("gsutil", args) diff --git a/.ci/magician/exec/interfaces.go b/.ci/magician/vcr/interfaces.go similarity index 97% rename from .ci/magician/exec/interfaces.go rename to .ci/magician/vcr/interfaces.go index 4f0b40d5e170..5fefc5214e68 100644 --- a/.ci/magician/exec/interfaces.go +++ b/.ci/magician/vcr/interfaces.go @@ -1,4 +1,4 @@ -package exec +package vcr import "path/filepath" @@ -9,9 +9,9 @@ type ExecRunner interface { RemoveAll(path string) error PushDir(path string) error PopDir() error + ReadFile(name string) (string, error) WriteFile(name, data string) error + Walk(root string, fn filepath.WalkFunc) error Run(name string, args []string, env map[string]string) (string, error) MustRun(name string, args []string, env map[string]string) string - Walk(root string, fn filepath.WalkFunc) error - ReadFile(name string) (string, error) } diff --git a/.ci/magician/vcr/tester.go b/.ci/magician/vcr/tester.go index d55807dc6be4..cb3a4c593130 100644 --- a/.ci/magician/vcr/tester.go +++ b/.ci/magician/vcr/tester.go @@ -3,7 +3,6 @@ package vcr import ( "fmt" "io/fs" - "magician/exec" "magician/provider" "path/filepath" "regexp" @@ -50,7 +49,7 @@ type logKey struct { type Tester struct { env map[string]string // shared environment variables for running tests - rnr exec.ExecRunner // for running commands and manipulating files + rnr ExecRunner // for running commands and manipulating files baseDir string // the directory in which this tester was created saKeyPath string // where sa_key.json is relative to baseDir cassettePaths map[provider.Version]string // where cassettes are relative to baseDir by version @@ -68,7 +67,7 @@ var testResultsExpression = regexp.MustCompile(`(?m:^--- (PASS|FAIL|SKIP): (Test var testPanicExpression = regexp.MustCompile(`^panic: .*`) // Create a new tester in the current working directory and write the service account key file. -func NewTester(env map[string]string, rnr exec.ExecRunner) (*Tester, error) { +func NewTester(env map[string]string, rnr ExecRunner) (*Tester, error) { saKeyPath := "sa_key.json" if err := rnr.WriteFile(saKeyPath, env["SA_KEY"]); err != nil { return nil, err