Skip to content

Commit a080fd3

Browse files
trodgetrodge
authored
Magician refactor generate comment (GoogleCloudPlatform#9613)
* Run cassettes on main Revert local testing changes Test: make magician-check-vcr-cassettes the only step Also collect passing and skipping tests collectResult no longer returns an error Remove extra return value gitignore magician binary Print logs Actually set logPaths and cassettePaths Rework management of environment variables Remove extra ] Also print all_tests.log Echo home Include HOME in env Echo path Use GOCACHE instead of HOME add -vet=off Run all tests, skip printing logs Run 24 tests and upload logs Add -r and remove logs Also upload cassettes Run tests for one resource in recording Run tests for one resource in replaying Also capture PATH Run recording again Clone hashicorp provider instead of mm Run all tests Run replaying Move check cassettes to push-downstream Remove echo PATH change to GA in doc (GoogleCloudPlatform#9491) Co-authored-by: Edward Sun <sunedward@google.com> Refactor magician structs (GoogleCloudPlatform#9605) * Refactored github interfaces Fixed bug in overriding breaking changes * gofmt * Removed GetPullRequestLabelIDs Use magician for generate comment Fix formatting of breaking changes Keep diff string empty Add missing newline Add copyright notices Add missing space Revert changes from running generate-comment Run cassettes on main Print logs Rework management of environment variables change to GA in doc (GoogleCloudPlatform#9491) Co-authored-by: Edward Sun <sunedward@google.com> git checkout main gcb-generate-diffs-new.yml * Move Version into provider package and Repo into source package * Remove second walk func * Add , * Add source Runner interface * Remove check cassettes changes * Fix issues with diff processor build * Fix test
1 parent 05f4a8d commit a080fd3

9 files changed

Lines changed: 394 additions & 194 deletions

File tree

‎.ci/magician/cmd/generate_comment.go‎

Lines changed: 119 additions & 118 deletions
Large diffs are not rendered by default.

‎.ci/magician/cmd/generate_comment_test.go‎

Lines changed: 54 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package cmd
1717

1818
import (
19+
"magician/source"
1920
"reflect"
2021
"testing"
2122
)
@@ -25,9 +26,28 @@ func TestExecGenerateComment(t *testing.T) {
2526
gh := &mockGithub{
2627
calledMethods: make(map[string][][]any),
2728
}
28-
execGenerateComment("build1", "project1", "17", "sha1", "pr1", "*******", gh, mr)
29+
ctlr := source.NewController("/mock/dir/go", "modular-magician", "*******", mr)
30+
env := map[string]string{
31+
"BUILD_ID": "build1",
32+
"BUILD_STEP": "17",
33+
"COMMIT_SHA": "sha1",
34+
"GITHUB_TOKEN": "*******",
35+
"PR_NUMBER": "pr1",
36+
"PROJECT_ID": "project1",
37+
}
38+
diffProcessorEnv := map[string]string{
39+
"BUILD_ID": "build1",
40+
"BUILD_STEP": "17",
41+
"COMMIT_SHA": "sha1",
42+
"GITHUB_TOKEN": "*******",
43+
"NEW_REF": "auto-pr-pr1",
44+
"OLD_REF": "auto-pr-pr1-old",
45+
"PR_NUMBER": "pr1",
46+
"PROJECT_ID": "project1",
47+
}
48+
execGenerateComment(env, gh, mr, ctlr)
2949

30-
for method, expectedCalls := range map[string][][]any{
50+
for method, expectedCalls := range map[string][]ParameterList{
3151
"Copy": {
3252
{"/mock/dir/tpg", "/mock/dir/magic-modules/tools/diff-processor/old"},
3353
{"/mock/dir/tpg", "/mock/dir/magic-modules/tools/diff-processor/new"},
@@ -44,38 +64,39 @@ func TestExecGenerateComment(t *testing.T) {
4464
{"/mock/dir/magic-modules/tools/diff-processor/bin"},
4565
},
4666
"Run": {
47-
{"/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"}, []string(nil)},
48-
{"/mock/dir/tpg", "git", []string{"fetch", "origin", "auto-pr-pr1-old"}, []string(nil)},
49-
{"/mock/dir/tpg", "git", []string{"diff", "origin/auto-pr-pr1-old", "origin/auto-pr-pr1", "--shortstat"}, []string(nil)},
50-
{"/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"}, []string(nil)},
51-
{"/mock/dir/tpgb", "git", []string{"fetch", "origin", "auto-pr-pr1-old"}, []string(nil)},
52-
{"/mock/dir/tpgb", "git", []string{"diff", "origin/auto-pr-pr1-old", "origin/auto-pr-pr1", "--shortstat"}, []string(nil)},
53-
{"/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"}, []string(nil)},
54-
{"/mock/dir/tfc", "git", []string{"fetch", "origin", "auto-pr-pr1-old"}, []string(nil)},
55-
{"/mock/dir/tfc", "git", []string{"diff", "origin/auto-pr-pr1-old", "origin/auto-pr-pr1", "--shortstat"}, []string(nil)},
56-
{"/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"}, []string(nil)},
57-
{"/mock/dir/tfoics", "git", []string{"fetch", "origin", "auto-pr-pr1-old"}, []string(nil)},
58-
{"/mock/dir/tfoics", "git", []string{"diff", "origin/auto-pr-pr1-old", "origin/auto-pr-pr1", "--shortstat"}, []string(nil)},
59-
{"/mock/dir/magic-modules/tools/diff-processor", "make", []string{"build"}, []string{"OLD_REF=auto-pr-pr1-old", "NEW_REF=auto-pr-pr1"}},
60-
{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"breaking-changes"}, []string(nil)},
61-
{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"add-labels", "pr1"}, []string{"GITHUB_TOKEN=*******"}},
62-
{"/mock/dir/magic-modules/tools/diff-processor", "make", []string{"build"}, []string{"OLD_REF=auto-pr-pr1-old", "NEW_REF=auto-pr-pr1"}},
63-
{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"breaking-changes"}, []string(nil)},
64-
{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"add-labels", "pr1"}, []string{"GITHUB_TOKEN=*******"}},
65-
{"/mock/dir/tpgbold", "git", []string{"checkout", "origin/auto-pr-pr1-old"}, []string(nil)},
66-
{"/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)},
67-
{"/mock/dir/tpgbold", "sed", []string{"-i.bak", "s|github.com/hashicorp/terraform-provider-google-beta|google/provider/old|g", "go.mod"}, []string(nil)},
68-
{"/mock/dir/tpgbold", "sed", []string{"-i.bak", "s|github.com/hashicorp/terraform-provider-google-beta|google/provider/old|g", "go.sum"}, []string(nil)},
69-
{"/mock/dir/tpgb", "find", []string{".", "-type", "f", "-name", "*.go", "-exec", "sed", "-i.bak", "s~github.com/hashicorp/terraform-provider-google-beta~google/provider/new~g", "{}", "+"}, []string(nil)},
70-
{"/mock/dir/tpgb", "sed", []string{"-i.bak", "s|github.com/hashicorp/terraform-provider-google-beta|google/provider/new|g", "go.mod"}, []string(nil)},
71-
{"/mock/dir/tpgb", "sed", []string{"-i.bak", "s|github.com/hashicorp/terraform-provider-google-beta|google/provider/new|g", "go.sum"}, []string(nil)},
72-
{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"mod", "edit", "-replace", "google/provider/new=/mock/dir/tpgb"}, []string(nil)},
73-
{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"mod", "edit", "-replace", "google/provider/old=/mock/dir/tpgbold"}, []string(nil)},
74-
{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"mod", "tidy"}, []string(nil)},
75-
{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"run", ".", "-services-dir=/mock/dir/tpgb/google-beta/services"}, []string(nil)},
76-
{"/mock/dir/magic-modules", "git", []string{"diff", "HEAD", "origin/main", "tools/missing-test-detector"}, []string(nil)}},
67+
{"/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)},
68+
{"/mock/dir/tpg", "git", []string{"fetch", "origin", "auto-pr-pr1-old"}, map[string]string(nil)},
69+
{"/mock/dir/tpg", "git", []string{"diff", "origin/auto-pr-pr1-old", "origin/auto-pr-pr1", "--shortstat"}, map[string]string(nil)},
70+
{"/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)},
71+
{"/mock/dir/tpgb", "git", []string{"fetch", "origin", "auto-pr-pr1-old"}, map[string]string(nil)},
72+
{"/mock/dir/tpgb", "git", []string{"diff", "origin/auto-pr-pr1-old", "origin/auto-pr-pr1", "--shortstat"}, map[string]string(nil)},
73+
{"/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)},
74+
{"/mock/dir/tfc", "git", []string{"fetch", "origin", "auto-pr-pr1-old"}, map[string]string(nil)},
75+
{"/mock/dir/tfc", "git", []string{"diff", "origin/auto-pr-pr1-old", "origin/auto-pr-pr1", "--shortstat"}, map[string]string(nil)},
76+
{"/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)},
77+
{"/mock/dir/tfoics", "git", []string{"fetch", "origin", "auto-pr-pr1-old"}, map[string]string(nil)},
78+
{"/mock/dir/tfoics", "git", []string{"diff", "origin/auto-pr-pr1-old", "origin/auto-pr-pr1", "--shortstat"}, map[string]string(nil)},
79+
{"/mock/dir/magic-modules/tools/diff-processor", "make", []string{"build"}, diffProcessorEnv},
80+
{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"breaking-changes"}, map[string]string(nil)},
81+
{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"add-labels", "pr1"}, diffProcessorEnv},
82+
{"/mock/dir/magic-modules/tools/diff-processor", "make", []string{"build"}, diffProcessorEnv},
83+
{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"breaking-changes"}, map[string]string(nil)},
84+
{"/mock/dir/magic-modules/tools/diff-processor", "bin/diff-processor", []string{"add-labels", "pr1"}, diffProcessorEnv},
85+
{"/mock/dir/tpgbold", "git", []string{"checkout", "origin/auto-pr-pr1-old"}, map[string]string(nil)},
86+
{"/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)},
87+
{"/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)},
88+
{"/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)},
89+
{"/mock/dir/tpgb", "find", []string{".", "-type", "f", "-name", "*.go", "-exec", "sed", "-i.bak", "s~github.com/hashicorp/terraform-provider-google-beta~google/provider/new~g", "{}", "+"}, map[string]string(nil)},
90+
{"/mock/dir/tpgb", "sed", []string{"-i.bak", "s|github.com/hashicorp/terraform-provider-google-beta|google/provider/new|g", "go.mod"}, map[string]string(nil)},
91+
{"/mock/dir/tpgb", "sed", []string{"-i.bak", "s|github.com/hashicorp/terraform-provider-google-beta|google/provider/new|g", "go.sum"}, map[string]string(nil)},
92+
{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"mod", "edit", "-replace", "google/provider/new=/mock/dir/tpgb"}, map[string]string(nil)},
93+
{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"mod", "edit", "-replace", "google/provider/old=/mock/dir/tpgbold"}, map[string]string(nil)},
94+
{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"mod", "tidy"}, map[string]string(nil)},
95+
{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"run", ".", "-services-dir=/mock/dir/tpgb/google-beta/services"}, map[string]string(nil)},
96+
{"/mock/dir/magic-modules", "git", []string{"diff", "HEAD", "origin/main", "tools/missing-test-detector"}, map[string]string(nil)},
97+
},
7798
} {
78-
if actualCalls, ok := mr.calledMethods[method]; !ok {
99+
if actualCalls, ok := mr.Calls(method); !ok {
79100
t.Fatalf("Found no calls for %s", method)
80101
} else if len(actualCalls) != len(expectedCalls) {
81102
t.Fatalf("Unexpected number of calls for %s, got %d, expected %d", method, len(actualCalls), len(expectedCalls))

‎.ci/magician/cmd/interfaces.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,6 @@ type ExecRunner interface {
4444
RemoveAll(path string) error
4545
PushDir(path string) error
4646
PopDir() error
47-
Run(name string, args, env []string) (string, error)
48-
MustRun(name string, args, env []string) string
47+
Run(name string, args []string, env map[string]string) (string, error)
48+
MustRun(name string, args []string, env map[string]string) string
4949
}

0 commit comments

Comments
 (0)