Backport of Limit number of tests in CI comment into release/1.13.x (#…

…21970) * backport of commit dc10489 (#21853) * fix multiline * shellcheck, and success message for builds * add full path * cat the summary * fix and faster * fix if condition * base64 in a separate step * echo * check against empty string * add echo * only use matrix ids * only id * echo matrix * remove wrapping array * tojson * try echo again * use jq to get packages * don't quote * only run binary tests once * only run binary tests once * test what's wrong with the binary * separate file * use matrix file * failed test * update comment on success * correct variable name * bae64 fix * output to file * use multiline * fix * fix formatting * fix newline * fix whitespace * correct body, remove comma * small fixes * shellcheck * another shellcheck fix * fix deprecation checker * only run comments for prs * Update .github/workflows/test-go.yml Co-authored-by: Mike Palmiotto <[email protected]> * Update .github/workflows/test-go.yml Co-authored-by: Mike Palmiotto <[email protected]> * fixes --------- Co-authored-by: Mike Palmiotto <[email protected]> * backport of commit 3b00dde (#21936) * limit test comments * remove unecessary tee * fix go test condition * fix * fail test * remove ailways entirely * fix columns * make a bunch of tests fail * separate line * include Failures: * remove test fails * fix whitespace * backport of commit 2454302 (#21973) * only add binary tests if they exist * shellcheck --------- Co-authored-by: miagilepner <[email protected]> Co-authored-by: Mike Palmiotto <[email protected]>
hashicorp · Jul 20, 2023 · a420deb · a420deb
1 parent f5f8c86
commit a420deb
Show file tree

Hide file tree

Showing 8 changed files with 171 additions and 67 deletions.
diff --git a/.github/scripts/gh_comment.sh b/.github/scripts/gh_comment.sh
@@ -0,0 +1,38 @@
+#!/bin/bash
+
+set -e
+
+
+function update_or_create_comment {
+  REPO=$1
+  PR_NUMBER=$2
+  SEARCH_KEY=$3
+  BODY=$4
+
+  # We only want for the GH bot to place one comment to report build failures
+  # and if we rerun a job, that comment needs to be updated.
+  # Let's try to find if the GH bot has placed a similar comment
+  comment_id=$(gh api \
+                 -H "Accept: application/vnd.github+json" \
+                 -H "X-GitHub-Api-Version: 2022-11-28" \
+                 --paginate \
+                 /repos/hashicorp/"$REPO"/issues/"$PR_NUMBER"/comments | jq -r --arg SEARCH_KEY "$SEARCH_KEY" '.[] | select (.body | contains($SEARCH_KEY)) | .id')
+
+  if [[ "$comment_id" != "" ]]; then
+    # update the comment with the new body
+    gh api \
+      --method PATCH \
+      -H "Accept: application/vnd.github+json" \
+      -H "X-GitHub-Api-Version: 2022-11-28" \
+      /repos/hashicorp/"$REPO"/issues/comments/"$comment_id" \
+      -f body="$BODY"
+  else
+    # create a comment with the new body
+    gh api \
+      --method POST \
+      -H "Accept: application/vnd.github+json" \
+      -H "X-GitHub-Api-Version: 2022-11-28" \
+      /repos/hashicorp/"$REPO"/issues/"$PR_NUMBER"/comments \
+      -f body="$BODY"
+  fi
+}
diff --git a/.github/scripts/report_failed_builds.sh b/.github/scripts/report_failed_builds.sh
@@ -30,30 +30,15 @@ done
 
 # Create a comment to be posted on the PR
 # This comment reports failed jobs and the url to the failed workflow
-new_body="build failed for these jobs: ${failed_jobs[*]}. Please refer to this workflow to learn more: https://github.com/hashicorp/vault/actions/runs/$RUN_ID"
-
-# We only want for the GH bot to place one comment to report build failures
-# and if we rerun a job, that comment needs to be updated.
-# Let's try to find if the GH bot has placed a similar comment
-comment_id=$(gh api \
-               -H "Accept: application/vnd.github+json" \
-               -H "X-GitHub-Api-Version: 2022-11-28" \
-               /repos/hashicorp/"$REPO"/issues/"$PR_NUMBER"/comments | jq -r '.[] | select (.body | contains("build failed for these job")) | .id')
-
-if [[ "$comment_id" != "" ]]; then
-  # update the comment with the new body
-  gh api \
-    --method PATCH \
-    -H "Accept: application/vnd.github+json" \
-    -H "X-GitHub-Api-Version: 2022-11-28" \
-    /repos/hashicorp/"$REPO"/issues/comments/"$comment_id" \
-    -f body="$new_body"
+if [ ${#failed_jobs[@]} -eq 0 ]; then
+  new_body="Build Results:
+All builds succeeded! :white_check_mark:"
 else
-  # create a comment with the new body
-  gh api \
-    --method POST \
-    -H "Accept: application/vnd.github+json" \
-    -H "X-GitHub-Api-Version: 2022-11-28" \
-    /repos/hashicorp/"$REPO"/issues/"$PR_NUMBER"/comments \
-    -f body="$new_body"
+  new_body="Build Results:
+Build failed for these jobs: ${failed_jobs[*]}. Please refer to this workflow to learn more: https://github.com/hashicorp/vault/actions/runs/$RUN_ID"
 fi
+
+
+source ./.github/scripts/gh_comment.sh
+
+update_or_create_comment "$REPO" "$PR_NUMBER" "Build Results:" "$new_body"
diff --git a/.github/scripts/report_failed_tests.sh b/.github/scripts/report_failed_tests.sh
@@ -0,0 +1,42 @@
+#!/bin/bash
+
+set -e
+MAX_TESTS=10
+# this script expects the following env vars to be set
+# error if these are not set
+[ ${GITHUB_TOKEN:?} ]
+[ ${RUN_ID:?} ]
+[ ${REPO:?} ]
+[ ${PR_NUMBER:?} ]
+if [ -z "$TABLE_DATA" ]; then
+  BODY="CI Results:
+All Go tests succeeded! :white_check_mark:"
+else
+  # Remove any rows that don't have a test name
+  # Only keep the test type, test package, test name, and logs column
+  # Remove the scroll emoji
+  # Remove "github.com/hashicorp/vault" from the package name
+  TABLE_DATA=$(echo "$TABLE_DATA" | awk -F\| '{if ($4 != " - ") { print "|" $2 "|" $3 "|" $4 "|" $7 }}' | sed -r 's/ :scroll://' | sed -r 's/github.com\/hashicorp\/vault\///')
+  NUM_FAILURES=$(wc -l <<< "$TABLE_DATA")
+
+  # Check if the number of failures is greater than the maximum tests to display
+  # If so, limit the table to MAX_TESTS number of results
+  if [ "$NUM_FAILURES" -gt "$MAX_TESTS" ]; then
+      TABLE_DATA=$(echo "$TABLE_DATA" | head -n "$MAX_TESTS")
+      NUM_OTHER=( $NUM_FAILURES - "$MAX_TESTS" )
+      TABLE_DATA="$TABLE_DATA
+
+and $NUM_OTHER other tests"
+  fi
+
+  # Add the header for the table
+  BODY="CI Results:
+Failures:
+| Test Type | Package | Test | Logs |
+| --------- | ------- | ---- | ---- |
+${TABLE_DATA}"
+fi
+
+source ./.github/scripts/gh_comment.sh
+
+update_or_create_comment "$REPO" "$PR_NUMBER" "CI Results:" "$BODY"
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -268,10 +268,7 @@ jobs:
       - build-ubi
       - test
       - test-docker-k8s
-    if: |
-      always() && (cancelled() ||
-      contains(needs.*.result, 'cancelled') ||
-      contains(needs.*.result, 'failure'))
+    if: (success() || failure()) && github.head_ref != ''
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
@@ -293,10 +290,7 @@ jobs:
   completed-successfully:
     # We force a failure if any of the dependent jobs fail,
     # this is a workaround for the issue reported https://github.com/actions/runner/issues/2566
-    if: |
-      always() && (cancelled() ||
-      contains(needs.*.result, 'cancelled') ||
-      contains(needs.*.result, 'failure'))
+    if: always()
     runs-on: ubuntu-latest
     needs:
       - build-other
@@ -308,5 +302,4 @@ jobs:
       - test-docker-k8s
     steps:
       - run: |
-          echo "Some of the required build and test workflows have failed!"
-          exit 1
+          tr -d '\n' <<< '${{ toJSON(needs.*.result) }}' | grep -q -v -E '(failure|cancelled)'
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -101,15 +101,16 @@ jobs:
       - verify-changes
     # Don't run this job for docs/ui only PRs
     if: |
-      always() && (needs.verify-changes.result == 'skipped' ||
+      needs.verify-changes.result == 'skipped' ||
       (needs.verify-changes.outputs.is_docs_change == 'false' &&
-      needs.verify-changes.outputs.is_ui_change == 'false'))
+      needs.verify-changes.outputs.is_ui_change == 'false')
     uses: ./.github/workflows/test-go.yml
     with:
       # The regular Go tests use an extra runner to execute the
       # binary-dependent tests. We isolate them there so that the
       # other tests aren't slowed down waiting for a binary build.
-      total-runners: 17
+      binary-tests: true
+      total-runners: 16
       go-arch: amd64
       go-tags: '${{ needs.setup.outputs.go-tags }},deadlock'
       runs-on: ${{ needs.setup.outputs.compute-larger }}
@@ -353,14 +354,32 @@ jobs:
           # Sort all of the summary table rows and push them to a temp file.
           temp_file_name=temp-$(date +%s)
           cat failure-summary-*.md | sort >> "$temp_file_name"
-
+          
           # If there are test failures, present them in a format of a GitHub Markdown table.
           if [ -s "$temp_file_name" ]; then
             # shellcheck disable=SC2129
             # Here we create the headings for the summary table
             echo "| Test Type | Package | Test | Elapsed | Runner Index | Logs |" >> "$GITHUB_STEP_SUMMARY"
             echo "| --------- | ------- | ---- | ------- | ------------ | ---- |" >> "$GITHUB_STEP_SUMMARY"
-            cat "$temp_file_name" >>  "$GITHUB_STEP_SUMMARY"
+            # shellcheck disable=SC2002
+            cat "$temp_file_name" >> "$GITHUB_STEP_SUMMARY"
           else
             echo "### All Go tests passed! :white_check_mark:" >> "$GITHUB_STEP_SUMMARY"
           fi
+          
+          # the random EOF is needed for a multiline environment variable
+          EOF=$(dd if=/dev/urandom bs=15 count=1 status=none | base64)
+          # shellcheck disable=SC2129
+          echo "TABLE_TEST_RESULTS<<$EOF" >> "$GITHUB_ENV"
+          cat "$temp_file_name" >> "$GITHUB_ENV"
+          echo "$EOF" >> "$GITHUB_ENV" 
+      - uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
+      - name: Create comment
+        if: github.head_ref != ''
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          PR_NUMBER: ${{ github.event.pull_request.number }}
+          RUN_ID: ${{ github.run_id }}
+          REPO: ${{ github.event.repository.name }}
+          TABLE_DATA: ${{ env.TABLE_TEST_RESULTS }} 
+        run: ./.github/scripts/report_failed_tests.sh
diff --git a/.github/workflows/test-go.yml b/.github/workflows/test-go.yml
@@ -10,9 +10,14 @@ on:
         required: true
         type: string
       total-runners:
-        description: Number of runners to use for executing the tests on.
+        description: Number of runners to use for executing non-binary tests.
         required: true
         type: string
+      binary-tests:
+        description: Whether to run the binary tests.
+        required: false
+        default: false
+        type: boolean 
       env-vars:
         description: A map of environment variables as JSON.
         required: false
@@ -119,31 +124,46 @@ jobs:
         run: |
           (
             go list ./... | grep -v "_binary" | grep -v "vault/integ" | gotestsum tool ci-matrix --debug \
-              --partitions 16 \
+              --partitions "${{ inputs.total-runners }}" \
               --timing-files 'test-results/go-test/*.json' > matrix.json
           )
       - name: Capture list of binary tests
+        if: inputs.binary-tests
         id: list-binary-tests
         run: |
           LIST="$(go list ./... | grep "_binary" | xargs)"
           echo "list=$LIST" >> "$GITHUB_OUTPUT"
       - name: Build complete matrix
         id: build
         run: |
-          set -exo pipefail
-          export BINARY_TESTS="${{ steps.list-binary-tests.outputs.list }}"
-          (
-            echo -n "matrix="
-            jq -c --arg BINARY "${BINARY_TESTS}" \
-              '.include += [{
-                "id": 16,
-                "estimatedRuntime": "N/A",
-                "packages": $BINARY,
-                "description": "partition 16 - binary test packages"
-            }]' matrix.json
-          ) >> "$GITHUB_OUTPUT"
+            set -exo pipefail
+            matrix_file="matrix.json"
+            if [ "${{ inputs.binary-tests}}" == "true" ] && [ -n "${{ steps.list-binary-tests.outputs.list }}" ]; then
+              export BINARY_TESTS="${{ steps.list-binary-tests.outputs.list }}"
+              jq --arg BINARY "${BINARY_TESTS}" --arg BINARY_INDEX "${{ inputs.total-runners }}" \
+                '.include += [{
+                  "id": $BINARY_INDEX,
+                  "estimatedRuntime": "N/A",
+                  "packages": $BINARY,
+                  "description": "partition $BINARY_INDEX - binary test packages"
+              }]' matrix.json > new-matrix.json
+              matrix_file="new-matrix.json" 
+            fi
+            # convert the json to a map keyed by id
+            (
+              echo -n "matrix="
+                jq -c \
+                '.include | map( { (.id|tostring): . } ) | add' "$matrix_file" 
+            ) >> "$GITHUB_OUTPUT"
+            # extract an array of ids from the json
+            (
+              echo -n "matrix_ids="
+              jq -c \
+                '[ .include[].id | tostring ]' "$matrix_file"
+            ) >> "$GITHUB_OUTPUT"
     outputs:
       matrix: ${{ steps.build.outputs.matrix }}
+      matrix_ids: ${{ steps.build.outputs.matrix_ids }} 
 
   test-go:
     needs: test-matrix
@@ -154,7 +174,8 @@ jobs:
     runs-on: ${{ fromJSON(inputs.runs-on) }}
     strategy:
       fail-fast: false
-      matrix: ${{ fromJSON(needs.test-matrix.outputs.matrix) }}
+      matrix: 
+        id: ${{ fromJSON(needs.test-matrix.outputs.matrix_ids) }}
     env:
       GOPRIVATE: github.com/hashicorp/*
       TIMEOUT_IN_MINUTES: ${{ inputs.timeout-minutes }}
@@ -196,7 +217,7 @@ jobs:
         run: |
           git config --global url."https://${{ secrets.ELEVATED_GITHUB_TOKEN}}@github.com".insteadOf https://github.com
       - id: build
-        if: contains(matrix.packages, '_binary')
+        if: inputs.binary-tests && matrix.id == inputs.total-runners 
         env:
           GOPRIVATE: github.com/hashicorp/*
         run: time make ci-bootstrap dev
@@ -207,10 +228,16 @@ jobs:
           COMMIT_SHA: ${{ github.sha }}
         run: |
           set -exo pipefail
-
+ 
           # Build the dynamically generated source files.
           make prep
-
+
+          packages=$(echo "${{ toJSON(needs.test-matrix.outputs.matrix) }}" | jq -c -r --arg id "${{ matrix.id }}" '.[$id] | .packages') 
+
+          if [ -z "$packages" ]; then
+            echo "no test packages to run"
+            exit 1
+          fi
           # We don't want VAULT_LICENSE set when running Go tests, because that's
           # not what developers have in their environments and it could break some
           # tests; it would be like setting VAULT_TOKEN.  However some non-Go
@@ -245,12 +272,12 @@ jobs:
               --junitfile test-results/go-test/results-${{ matrix.id }}.xml \
               --jsonfile test-results/go-test/results-${{ matrix.id }}.json \
               --jsonfile-timing-events failure-summary-${{ matrix.id }}${{ inputs.name != '' && '-' || '' }}${{ inputs.name }}.json \
+              --packages "$packages" \
               -- \
               -tags "${{ inputs.go-tags }}" \
               -timeout=${{ env.TIMEOUT_IN_MINUTES }}m \
               -parallel=${{ inputs.go-test-parallelism }} \
               ${{ inputs.extra-flags }} \
-              ${{ matrix.packages }}
       - name: Prepare datadog-ci
         if: github.repository == 'hashicorp/vault' && (success() || failure())
         continue-on-error: true
@@ -299,13 +326,13 @@ jobs:
               let prefixToSearchFor;
               switch ("${{ inputs.name }}") {
                 case "race":
-                  prefixToSearchFor = 'Run Go tests with data race detection / test-go (${{ matrix.id }},'
+                  prefixToSearchFor = 'Run Go tests with data race detection / test-go (${{ matrix.id }})'
                   break
                 case "fips":
-                  prefixToSearchFor = 'Run Go tests with FIPS configuration / test-go (${{ matrix.id }},'
+                  prefixToSearchFor = 'Run Go tests with FIPS configuration / test-go (${{ matrix.id }})'
                   break
                 default:
-                  prefixToSearchFor = 'Run Go tests / test-go (${{ matrix.id }},'
+                  prefixToSearchFor = 'Run Go tests / test-go (${{ matrix.id }})'
               }
 
               const jobData = result.data.jobs.filter(

diff --git a/Makefile b/Makefile
@@ -115,12 +115,12 @@ vet:
 
 # deprecations runs staticcheck tool to look for deprecations. Checks entire code to see if it 
 # has deprecated function, variable, constant or field
-deprecations: bootstrap
+deprecations: bootstrap prep
 	@BUILD_TAGS='$(BUILD_TAGS)' ./scripts/deprecations-checker.sh ""
 
 # ci-deprecations runs staticcheck tool to look for deprecations. All output gets piped to revgrep
 # which will only return an error if changes that is not on main has deprecated function, variable, constant or field
-ci-deprecations: ci-bootstrap
+ci-deprecations: ci-bootstrap prep
 	@BUILD_TAGS='$(BUILD_TAGS)' ./scripts/deprecations-checker.sh main
 
 tools/codechecker/.bin/codechecker:
@@ -129,13 +129,13 @@ tools/codechecker/.bin/codechecker:
 # vet-codechecker runs our custom linters on the test functions. All output gets
 # piped to revgrep which will only return an error if new piece of code violates
 # the check 
-vet-codechecker: bootstrap tools/codechecker/.bin/codechecker
+vet-codechecker: bootstrap tools/codechecker/.bin/codechecker prep
 	@$(GO_CMD) vet -vettool=./tools/codechecker/.bin/codechecker -tags=$(BUILD_TAGS) ./... 2>&1 | revgrep
 
 # vet-codechecker runs our custom linters on the test functions. All output gets
 # piped to revgrep which will only return an error if new piece of code that is 
 # not on main violates the check 
-ci-vet-codechecker: ci-bootstrap tools/codechecker/.bin/codechecker
+ci-vet-codechecker: ci-bootstrap tools/codechecker/.bin/codechecker prep
 	@$(GO_CMD) vet -vettool=./tools/codechecker/.bin/codechecker -tags=$(BUILD_TAGS) ./... 2>&1 | revgrep origin/main
 
 # lint runs vet plus a number of other checkers, it is more comprehensive, but louder

diff --git a/scripts/deprecations-checker.sh b/scripts/deprecations-checker.sh
@@ -34,5 +34,5 @@ if [ -z $1 ]
     else
         # GitHub Actions will use this to find only changes wrt PR's base ref branch
         # revgrep CLI tool will return an exit status of 1 if any issues match, else it will return 0
-        staticcheck  -checks="SA1019" -tags="$BUILD_TAGS" 2>&1 | revgrep "$(git merge-base HEAD "origin/$1")"
+        staticcheck  -checks="SA1019" -tags="$BUILD_TAGS" 2>&1 | revgrep origin/"$1" 
 fi