Backport of Limit number of tests in CI comment into release/1.14.x (#21971)

* backport of commit dc104898f700447f7764919445c7559baeb7e987 (#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 <mike.palmiotto@hashicorp.com> * Update .github/workflows/test-go.yml Co-authored-by: Mike Palmiotto <mike.palmiotto@hashicorp.com> * fixes --------- Co-authored-by: Mike Palmiotto <mike.palmiotto@hashicorp.com> * backport of commit 3b00dde1ba4d479fbd67b1d0767e421e495d8cce (#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 245430215c00d80a38283020fca114bade022e0f (#21973) * only add binary tests if they exist * shellcheck --------- Co-authored-by: miagilepner <mia.epner@hashicorp.com> Co-authored-by: Mike Palmiotto <mike.palmiotto@hashicorp.com>
2023-07-20 09:07:20 -04:00 · 2023-07-20 09:07:20 -04:00 · 45af65da81
parent 9bbd9b0534
commit 45af65da81
8 changed files with 169 additions and 65 deletions
--- 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
+}
--- 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"
--- 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"
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@ -292,10 +292,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
@ -317,10 +314,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
@ -332,5 +326,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)'
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@ -109,7 +109,8 @@ jobs:
      # 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 }}
@ -363,14 +364,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
--- 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,10 +124,11 @@ 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)"
@ -130,20 +136,34 @@ jobs:
      - 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
@ -253,7 +280,7 @@ jobs:
              --jsonfile test-results/go-test/results-${{ matrix.id }}.json \
              --jsonfile-timing-events failure-summary-${{ matrix.id }}${{ inputs.name != '' && '-' || '' }}${{ inputs.name }}.json \
              $RERUN_FAILS \
-              --packages "${{ matrix.packages }}" \
+              --packages "$packages" \
              -- \
              -tags "${{ inputs.go-tags }}" \
              -timeout=${{ env.TIMEOUT_IN_MINUTES }}m \
@ -307,13 +334,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(
--- a/8
+++ b/8
@ -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
--- 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