Made breaking change detector own its own setup and build process (Go…

…ogleCloudPlatform#8705) * Made breaking change detector own its own setup and build process * Moved breaking change detector unit tests to github action * Corrected breaking change detector unit test setup * Added back package name updates for tpgb new * made unit tests use a shallow clone * Limit breaking change detector unit tests to runs that modify the tool Co-authored-by: Scott Suarez <[email protected]> * Update .ci/scripts/go-plus/github-differ/generate_comment.sh Co-authored-by: Scott Suarez <[email protected]> * Minor Cleanup * Intentionally broke breaking change detector * Revert "Intentionally broke breaking change detector" This reverts commit bcb6ba8. * Intentionally caused panic in breaking change detector at runtime * Made a breaking change * Added additional logging * Removed export in generate_comment.sh export hides the exit code of the command being run; assignment on its own does not. Export is not required in the context of a shell script * Made failure get set to 1 instead of $? * Added bin/ cleanup * Revert "Intentionally caused panic in breaking change detector at runtime" This reverts commit a16c0cd. * Fixed package name replacement for google-beta * Re-added export of TPG/TPGB BREAKING * Added comment explaining the export location * Revert "Made a breaking change" This reverts commit 2deecd7. --------- Co-authored-by: Scott Suarez <[email protected]>
nevzheng · Aug 25, 2023 · a0178b9 · a0178b9
1 parent af7728a
commit a0178b9
Show file tree

Hide file tree

Showing 9 changed files with 305 additions and 673 deletions.
diff --git a/.ci/scripts/go-plus/github-differ/generate_comment.sh b/.ci/scripts/go-plus/github-differ/generate_comment.sh
@@ -36,7 +36,7 @@ TFC_LOCAL_PATH=$PWD/../tfc
 DIFFS=""
 NEWLINE=$'\n'
 
-# TPG difference
+# TPG/TPGB difference
 mkdir -p $TPG_LOCAL_PATH
 git clone -b $NEW_BRANCH $TPG_SCRATCH_PATH $TPG_LOCAL_PATH
 pushd $TPG_LOCAL_PATH
@@ -45,82 +45,79 @@ if ! git diff --exit-code origin/$OLD_BRANCH origin/$NEW_BRANCH; then
     SUMMARY=`git diff origin/$OLD_BRANCH origin/$NEW_BRANCH --shortstat`
     DIFFS="${DIFFS}${NEWLINE}Terraform GA: [Diff](https://github.com/modular-magician/terraform-provider-google/compare/$OLD_BRANCH..$NEW_BRANCH) ($SUMMARY)"
 fi
-TPG_LOCAL_PATH_OLD="${TPG_LOCAL_PATH}old"
-mkdir -p $TPG_LOCAL_PATH_OLD
-cp -r $TPG_LOCAL_PATH/. $TPG_LOCAL_PATH_OLD
+popd
 
-git checkout origin/$NEW_BRANCH
-update_package_name "github.com/hashicorp/terraform-provider-google" "google/provider/new"
+mkdir -p $TPGB_LOCAL_PATH
+git clone -b $NEW_BRANCH $TPGB_SCRATCH_PATH $TPGB_LOCAL_PATH
+pushd $TPGB_LOCAL_PATH
+git fetch origin $OLD_BRANCH
+if ! git diff --exit-code origin/$OLD_BRANCH origin/$NEW_BRANCH; then
+    SUMMARY=`git diff origin/$OLD_BRANCH origin/$NEW_BRANCH --shortstat`
+    DIFFS="${DIFFS}${NEWLINE}Terraform Beta: [Diff](https://github.com/modular-magician/terraform-provider-google-beta/compare/$OLD_BRANCH..$NEW_BRANCH) ($SUMMARY)"
+fi
 popd
 
 ## Breaking change setup and execution
-pushd $TPG_LOCAL_PATH_OLD
-git checkout origin/$OLD_BRANCH
-update_package_name "github.com/hashicorp/terraform-provider-google" "google/provider/old"
-popd
 set +e
 pushd $MM_LOCAL_PATH/tools/breaking-change-detector
-sed -i.bak -E "s~google/provider/(.*)/([0-9A-Za-z-]*)~google/provider/\1/google~" comparison.go
-go mod edit -replace google/provider/new=$(realpath $TPG_LOCAL_PATH)
-go mod edit -replace google/provider/old=$(realpath $TPG_LOCAL_PATH_OLD)
-go mod tidy
-export TPG_BREAKING="$(go run .)"
+cp -r $TPG_LOCAL_PATH old/
+cp -r $TPG_LOCAL_PATH new/
+make build OLD_REF=$OLD_BRANCH NEW_REF=$NEW_BRANCH
+TPG_BREAKING="$(bin/breaking-change-detector)"
 retVal=$?
 if [ $retVal -ne 0 ]; then
-    export TPG_BREAKING=""
-    BREAKING_CHANGE_BUILD_FAILURE=$?
+    TPG_BREAKING=""
+    BREAKING_CHANGE_BUILD_FAILURE=1
 fi
-set -e
-popd
 
-# TPGB
-mkdir -p $TPGB_LOCAL_PATH
-git clone -b $NEW_BRANCH $TPGB_SCRATCH_PATH $TPGB_LOCAL_PATH
-pushd $TPGB_LOCAL_PATH
-git fetch origin $OLD_BRANCH
-if ! git diff --exit-code origin/$OLD_BRANCH origin/$NEW_BRANCH; then
-    SUMMARY=`git diff origin/$OLD_BRANCH origin/$NEW_BRANCH --shortstat`
-    DIFFS="${DIFFS}${NEWLINE}Terraform Beta: [Diff](https://github.com/modular-magician/terraform-provider-google-beta/compare/$OLD_BRANCH..$NEW_BRANCH) ($SUMMARY)"
+rm -rf ./old/ ./new/ ./bin/
+cp -r $TPGB_LOCAL_PATH old/
+cp -r $TPGB_LOCAL_PATH new/
+make build OLD_REF=$OLD_BRANCH NEW_REF=$NEW_BRANCH
+TPGB_BREAKING="$(bin/breaking-change-detector)"
+retVal=$?
+if [ $retVal -ne 0 ]; then
+    TPGB_BREAKING=""
+    BREAKING_CHANGE_BUILD_FAILURE=1
+fi
+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
+    # exporting earlier would cause the retvals to not be calculated properly.
+    export TPG_BREAKING=$TPG_BREAKING
+    export TPGB_BREAKING=$TPGB_BREAKING
+    BREAKINGCHANGES="$($script_dir/compare_breaking_changes.sh)"
+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
 TPGB_LOCAL_PATH_OLD="${TPGB_LOCAL_PATH}old"
 mkdir -p $TPGB_LOCAL_PATH_OLD
 cp -r $TPGB_LOCAL_PATH/. $TPGB_LOCAL_PATH_OLD
 
+pushd $TPGB_LOCAL_PATH
 git checkout origin/$NEW_BRANCH
 update_package_name "github.com/hashicorp/terraform-provider-google-beta" "google/provider/new"
 popd
 
-
 pushd $TPGB_LOCAL_PATH_OLD
 git checkout origin/$OLD_BRANCH
 update_package_name "github.com/hashicorp/terraform-provider-google-beta" "google/provider/old"
 popd
-set +e
-pushd $MM_LOCAL_PATH/tools/breaking-change-detector
-sed -i.bak -E "s~google/provider/(.*)/([0-9A-Za-z-]*)~google/provider/\1/google-beta~" comparison.go
-go mod edit -replace google/provider/new=$(realpath $TPGB_LOCAL_PATH)
-go mod edit -replace google/provider/old=$(realpath $TPGB_LOCAL_PATH_OLD)
-go mod tidy
-export TPGB_BREAKING="$(go run .)"
-retVal=$?
-if [ $retVal -ne 0 ]; then
-    export TPGB_BREAKING=""
-    BREAKING_CHANGE_BUILD_FAILURE=$?
-fi
-BREAKINGCHANGES="$($script_dir/compare_breaking_changes.sh)"
-set -e
-popd
 
-## Missing test setup and execution
 set +e
 pushd $MM_LOCAL_PATH/tools/missing-test-detector
 go mod edit -replace google/provider/new=$(realpath $TPGB_LOCAL_PATH)
 go mod edit -replace google/provider/old=$(realpath $TPGB_LOCAL_PATH_OLD)
 go mod tidy
-export MISSINGTESTS="$(go run . -services-dir=$TPGB_LOCAL_PATH/google-beta/services)"
+MISSINGTESTS="$(go run . -services-dir=$TPGB_LOCAL_PATH/google-beta/services)"
 retVal=$?
 if [ $retVal -ne 0 ]; then
-    export MISSINGTESTS=""
+    MISSINGTESTS=""
 fi
 set -e
 popd
@@ -198,7 +195,7 @@ curl -H "Authorization: token ${GITHUB_TOKEN}" \
       -d "$(jq -r --arg diffs "$MESSAGE" -n "{body: \$diffs}")" \
       "https://api.github.com/repos/GoogleCloudPlatform/magic-modules/issues/${PR_NUMBER}/comments"
 
-if ! git diff --exit-code origin/main tools || [ "$BREAKING_CHANGE_BUILD_FAILURE" -ne 0 ]; then
+if ! git diff --exit-code origin/main tools; then
     ## Run unit tests for breaking change and missing test detector.
     "$script_dir/test_tools.sh" "$MM_LOCAL_PATH" "$TPGB_LOCAL_PATH" "$COMMIT_SHA" "$BUILD_ID" "$BUILD_STEP" "$PROJECT_ID"
 fi
diff --git a/.ci/scripts/go-plus/github-differ/test_tools.sh b/.ci/scripts/go-plus/github-differ/test_tools.sh
@@ -9,35 +9,9 @@ project_id=$6
 
 github_username=modular-magician
 
-set +e
-pushd $MM_LOCAL_PATH/tools/breaking-change-detector
-go test ./...
-exit_code=$?
-popd
-set -e
-
-
-if [ $exit_code -ne 0 ]; then
-	state="failure"
-else
-	state="success"
-fi
-
-post_body=$( jq -n \
-	--arg context "unit-tests-breaking-change-detector" \
-	--arg target_url "https://console.cloud.google.com/cloud-build/builds;region=global/${build_id};step=${build_step}?project=${project_id}" \
-	--arg state "${state}" \
-	'{context: $context, target_url: $target_url, state: $state}')
-
-curl \
-  -X POST \
-  -u "$github_username:$GITHUB_TOKEN" \
-  -H "Accept: application/vnd.github.v3+json" \
-  "https://api.github.com/repos/GoogleCloudPlatform/magic-modules/statuses/$mm_commit_sha" \
-  -d "$post_body"
-
 set +e
 pushd $MM_LOCAL_PATH/tools/missing-test-detector
+go mod tidy
 SERVICES_DIR=$TPGB_LOCAL_PATH/google-beta/services go test
 exit_code=$?
 popd

diff --git a/.github/workflows/unit-tests-breaking-change-detector.yml b/.github/workflows/unit-tests-breaking-change-detector.yml
@@ -0,0 +1,30 @@
+name: unit-tests-breaking-change-detector
+
+permissions: read-all
+
+on:
+  pull_request:
+    paths:
+      - 'tools/breaking-change-detector/**'
+
+jobs:
+  test:
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@v3
+
+      - name: Set up Go
+        uses: actions/setup-go@v4
+        with:
+          go-version: '^1.19.1'
+
+      - name: Build
+        run: |
+          cd tools/breaking-change-detector
+          make clone OWNER_REPO=hashicorp/terraform-provider-google DEPTH=1
+          make build OLD_REF=main NEW_REF=main
+
+      - name: Test
+        run: |
+          cd tools/breaking-change-detector
+          go test -v ./...
diff --git a/tools/breaking-change-detector/.gitignore b/tools/breaking-change-detector/.gitignore
@@ -0,0 +1,3 @@
+old
+new
+bin
diff --git a/tools/breaking-change-detector/GNUmakefile b/tools/breaking-change-detector/GNUmakefile
@@ -1,9 +1,58 @@
-localsetup:
-	sed -i bak -E "s~google/provider/(.*)/([0-9A-Za-z-]*)~google/provider/\1/$(PROVIDER_VERSION)~" comparison.go
+clone:
+ifndef OWNER_REPO
+	$(error OWNER_REPO is undefined)
+endif
+	rm -rf ./old ./new
+ifdef DEPTH
+	git clone --depth=$(DEPTH) https://github.com/$(OWNER_REPO).git old
+else
+	git clone https://github.com/$(OWNER_REPO).git old
+endif
+	cp -r old/ new/
 
-ifneq ($(REPLACE_OLD),)
-	go mod edit -replace google/provider/old=$(realpath ${REPLACE_OLD})
+clean:
+	cd old/; \
+	    rm -rf google/ google-beta; \
+		git checkout -- .; \
+		git clean -f .
+	cd new/; \
+	    rm -rf google/ google-beta; \
+		git checkout -- .; \
+		git clean -f .
+	rm -rf bin/
+
+build:
+ifndef OLD_REF
+	$(error OLD_REF is undefined)
+endif
+ifndef NEW_REF
+	$(error NEW_REF is undefined)
 endif
-ifneq ($(REPLACE_NEW),)
-	go mod edit -replace google/provider/new=$(realpath ${REPLACE_NEW})
-endif
+	$(MAKE) clean
+	cd old/; \
+		git checkout $(OLD_REF); \
+		real_package_name=github.com/hashicorp/terraform-provider-google; \
+		real_folder_name=google; \
+		fake_package_name=google/provider/old; \
+		if [ -d "google-beta" ]; then \
+			mv google-beta google; \
+			real_package_name=github.com/hashicorp/terraform-provider-google-beta; \
+			real_folder_name=google-beta; \
+		fi; \
+		find . -type f -name "*.go" -exec sed -i.bak "s~$$real_package_name/$$real_folder_name~$$fake_package_name/google~g" {} +; \
+		sed -i.bak "s|$$real_package_name|$$fake_package_name|g" go.mod
+	cd new/; \
+		git checkout $(NEW_REF); \
+		real_package_name=github.com/hashicorp/terraform-provider-google; \
+		real_folder_name=google; \
+		fake_package_name=google/provider/new; \
+		if [ -d "google-beta" ]; then \
+			mv google-beta google; \
+			real_package_name=github.com/hashicorp/terraform-provider-google-beta; \
+			real_folder_name=google-beta; \
+		fi; \
+		find . -type f -name "*.go" -exec sed -i.bak "s~$$real_package_name/$$real_folder_name~$$fake_package_name/google~g" {} +; \
+		sed -i.bak "s|$$real_package_name|$$fake_package_name|g" go.mod
+	go mod tidy
+	mkdir -p bin/
+	go build -o ./bin/ .
diff --git a/tools/breaking-change-detector/README.md b/tools/breaking-change-detector/README.md
@@ -1,28 +1,24 @@
 # Breaking change detector
 
-## Purpose
 Detects breaking changes between provider versions.
 
 Specifically protects customer expectations between [minor version](https://www.terraform.io/plugin/sdkv2/best-practices/versioning#example-minor-number-increments).
 
 
-## Execution of
+## Run
 
-### Program:mode-default
 ```bash
-go run .
-```
+# set up old / new dirs
+make clone OWNER_REPO=modular-magician/terraform-provider-google
 
-### Tests
-```bash
-go test ./...
-```
+# build based on old / new dirs
+make build OLD_REF=branch_or_commit NEW_REF=branch_or_commit
 
+# Run the binary
+bin/breaking-change-detector
+```
 
-## Misc
+## Test
 ```bash
-# getting the go version label from git log
-TZ=UTC0 git log --since="jan 1 2019" --format=%cd-%H --date=format-local:%Y%m%d%H%M%S | sed -E "s/(.*-.{12}).*/\1/"
+go test ./...
 ```
-
-
diff --git a/tools/breaking-change-detector/comparison.go b/tools/breaking-change-detector/comparison.go
@@ -1,8 +1,8 @@
 package main
 
 import (
-	newProvider "google/provider/new/google-beta"
-	oldProvider "google/provider/old/google-beta"
+	newProvider "google/provider/new/google"
+	oldProvider "google/provider/old/google"
 	"strings"
 
 	"github.com/GoogleCloudPlatform/magic-modules/.ci/breaking-change-detector/rules"