Skip to content

Commit

Permalink
Magician refactor generate comment (#9613)
Browse files Browse the repository at this point in the history 
* 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 (#9491)

Co-authored-by: Edward Sun <[email protected]>

Refactor magician structs (#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 (#9491)

Co-authored-by: Edward Sun <[email protected]>

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
  • Loading branch information
@trodge
trodge authored Dec 14, 2023
1 parent 05f4a8d commit a080fd3
Show file tree
Hide file tree
Showing 9 changed files with 394 additions and 194 deletions.
237 changes: 119 additions & 118 deletions .ci/magician/cmd/generate_comment.go
View file

Large diffs are not rendered by default.

87 changes: 54 additions & 33 deletions .ci/magician/cmd/generate_comment_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package cmd

import (
"magician/source"
"reflect"
"testing"
)
Expand All @@ -25,9 +26,28 @@ func TestExecGenerateComment(t *testing.T) {
gh := &mockGithub{
calledMethods: make(map[string][][]any),
}
execGenerateComment("build1", "project1", "17", "sha1", "pr1", "*******", gh, mr)
ctlr := source.NewController("/mock/dir/go", "modular-magician", "*******", mr)
env := map[string]string{
"BUILD_ID": "build1",
"BUILD_STEP": "17",
"COMMIT_SHA": "sha1",
"GITHUB_TOKEN": "*******",
"PR_NUMBER": "pr1",
"PROJECT_ID": "project1",
}
diffProcessorEnv := map[string]string{
"BUILD_ID": "build1",
"BUILD_STEP": "17",
"COMMIT_SHA": "sha1",
"GITHUB_TOKEN": "*******",
"NEW_REF": "auto-pr-pr1",
"OLD_REF": "auto-pr-pr1-old",
"PR_NUMBER": "pr1",
"PROJECT_ID": "project1",
}
execGenerateComment(env, gh, mr, ctlr)

for method, expectedCalls := range map[string][][]any{
for method, expectedCalls := range map[string][]ParameterList{
"Copy": {
{"/mock/dir/tpg", "/mock/dir/magic-modules/tools/diff-processor/old"},
{"/mock/dir/tpg", "/mock/dir/magic-modules/tools/diff-processor/new"},
Expand All @@ -44,38 +64,39 @@ 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"}, []string(nil)},
{"/mock/dir/tpg", "git", []string{"fetch", "origin", "auto-pr-pr1-old"}, []string(nil)},
{"/mock/dir/tpg", "git", []string{"diff", "origin/auto-pr-pr1-old", "origin/auto-pr-pr1", "--shortstat"}, []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"}, []string(nil)},
{"/mock/dir/tpgb", "git", []string{"fetch", "origin", "auto-pr-pr1-old"}, []string(nil)},
{"/mock/dir/tpgb", "git", []string{"diff", "origin/auto-pr-pr1-old", "origin/auto-pr-pr1", "--shortstat"}, []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"}, []string(nil)},
{"/mock/dir/tfc", "git", []string{"fetch", "origin", "auto-pr-pr1-old"}, []string(nil)},
{"/mock/dir/tfc", "git", []string{"diff", "origin/auto-pr-pr1-old", "origin/auto-pr-pr1", "--shortstat"}, []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"}, []string(nil)},
{"/mock/dir/tfoics", "git", []string{"fetch", "origin", "auto-pr-pr1-old"}, []string(nil)},
{"/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)},
{"/mock/dir/tpgbold", "sed", []string{"-i.bak", "s|github.com/hashicorp/terraform-provider-google-beta|google/provider/old|g", "go.sum"}, []string(nil)},
{"/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)},
{"/mock/dir/tpgb", "sed", []string{"-i.bak", "s|github.com/hashicorp/terraform-provider-google-beta|google/provider/new|g", "go.mod"}, []string(nil)},
{"/mock/dir/tpgb", "sed", []string{"-i.bak", "s|github.com/hashicorp/terraform-provider-google-beta|google/provider/new|g", "go.sum"}, []string(nil)},
{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"mod", "edit", "-replace", "google/provider/new=/mock/dir/tpgb"}, []string(nil)},
{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"mod", "edit", "-replace", "google/provider/old=/mock/dir/tpgbold"}, []string(nil)},
{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"mod", "tidy"}, []string(nil)},
{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"run", ".", "-services-dir=/mock/dir/tpgb/google-beta/services"}, []string(nil)},
{"/mock/dir/magic-modules", "git", []string{"diff", "HEAD", "origin/main", "tools/missing-test-detector"}, []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", "/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/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", "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/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)},
{"/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)},
{"/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)},
{"/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)},
{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"mod", "edit", "-replace", "google/provider/new=/mock/dir/tpgb"}, map[string]string(nil)},
{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"mod", "edit", "-replace", "google/provider/old=/mock/dir/tpgbold"}, map[string]string(nil)},
{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"mod", "tidy"}, map[string]string(nil)},
{"/mock/dir/magic-modules/tools/missing-test-detector", "go", []string{"run", ".", "-services-dir=/mock/dir/tpgb/google-beta/services"}, map[string]string(nil)},
{"/mock/dir/magic-modules", "git", []string{"diff", "HEAD", "origin/main", "tools/missing-test-detector"}, map[string]string(nil)},
},
} {
if actualCalls, ok := mr.calledMethods[method]; !ok {
if actualCalls, ok := mr.Calls(method); !ok {
t.Fatalf("Found no calls for %s", method)
} else if len(actualCalls) != len(expectedCalls) {
t.Fatalf("Unexpected number of calls for %s, got %d, expected %d", method, len(actualCalls), len(expectedCalls))
Expand Down
4 changes: 2 additions & 2 deletions .ci/magician/cmd/interfaces.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,6 @@ type ExecRunner interface {
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
Run(name string, args []string, env map[string]string) (string, error)
MustRun(name string, args []string, env map[string]string) string
}
Loading

0 comments on commit a080fd3

Please sign in to comment.