Browse Source

ci: Periodically upload coverage to codecov from master (#58520)

These changes are related to Codecov Automated Test Selection. Read
more: https://docs.codecov.com/docs/automated-test-selection

These changes introduce 2 new workflows:
* codecov_ats_input_periodic_test_run - runs backend tests (not all of
them) in master hourly, with `--cov-context=test` option and uploads
them to codecov under the flag "smart-tests". This is used as input for
Codecov Automated Test Selection.

* codecov_carryforward_reports - carries forward the reports generated
byh the 1st workflow to subsequent commits in the master branch. Also
uploads static analysis info for them. This allows these commits to be
used as BASE for Automated Test Selection.

The periodic workflow is time expensive (hence running only every hour)
The carryforward workflow is time cheap (so we can run on every commit)

The changes here are necessary for getsentry/sentry#57587 to work
properly
Giovanni M Guidini 1 year ago
parent
commit
80e5f0c2c7

+ 44 - 26
.github/workflows/codecov_ats.yml

@@ -44,10 +44,6 @@ jobs:
         if: needs.files-changed.outputs.backend == 'true'
         needs: files-changed
         runs-on: ubuntu-latest
-        # Map a step output to a job output
-        outputs:
-          ATS_TESTS_TO_RUN: ${{ steps.codecov_automated_test_selection.outputs.ATS_TESTS_TO_RUN }}
-          ATS_TESTS_TO_SKIP: ${{ steps.codecov_automated_test_selection.outputs.ATS_TESTS_TO_SKIP }}
         steps:
         - uses: actions/checkout@v3
           with:
@@ -73,7 +69,7 @@ jobs:
             pg-version: 14
         - name: Download Codecov CLI
           run: |
-            pip install --extra-index-url https://pypi.org/simple --no-cache-dir codecov-cli>=0.4.0
+            pip install --extra-index-url https://pypi.org/simple --no-cache-dir codecov-cli>=0.4.1
         # Creates the commit and report objects in codecov
         - name: Codecov startup
           run: |
@@ -98,25 +94,38 @@ jobs:
         - name: Codecov Automated Test Selection
           id: codecov_automated_test_selection
           run: |
+            # Directory for the artifacts from this step
+            mkdir .artifacts/codecov_ats
+            # This is the base for the git diff BASE..HEAD
             BASE_COMMIT=$(git merge-base ${{ github.sha }}^ origin/master)
-            echo $BASE_COMMIT
+            # Get list of tests to run from Codecov
             output=$(codecovcli --codecov-yml-path=codecov.yml label-analysis --dry-run --token=${CODECOV_STATIC_TOKEN} --base-sha=${BASE_COMMIT}) || true
+            # Post processing and validation
             if [ -n "${output}" ];
             then
 
-              echo ATS_TESTS_TO_RUN=$(jq <<< $output '.runner_options + .ats_tests_to_run | @json | @sh' --raw-output) >> "$GITHUB_OUTPUT"
-              echo ATS_TESTS_TO_SKIP=$(jq <<< $output '.runner_options + .ats_tests_to_skip | @json | @sh' --raw-output) >> "$GITHUB_OUTPUT"
+              jq <<< $output '.runner_options + .ats_tests_to_run | @json' --raw-output > .artifacts/codecov_ats/tests_to_run.json
+              jq <<< $output '.runner_options + .ats_tests_to_skip | @json' --raw-output > .artifacts/codecov_ats/tests_to_skip.json
 
               testcount() { jq <<< $output ".$1 | length"; }
               run_count=$(testcount ats_tests_to_run)
               skip_count=$(testcount ats_tests_to_skip)
-              tee <<< "Selected $run_count / $(($run_count + $skip_count)) tests to run" "$GITHUB_STEP_SUMMARY"
+              # Parse any potential errors that made ATS fallback to running all tests
+              # And surface them
+              ats_fallback_reason=$(jq <<< "$output" '.ats_fallback_reason')
+              if [ "$ats_fallback_reason" == "null" ]; then
+                ats_success=true
+              else
+                ats_success=false
+              fi
+              tee <<< \
+                "{\"ats_success\": $ats_success, \"error\": $ats_fallback_reason, \"tests_to_run\": $run_count, \"tests_analyzed\": $((run_count+skip_count))}" \
+                "$GITHUB_STEP_SUMMARY" \
+                ".artifacts/codecov_ats/result.json"
             else
-              tee <<< "ATS failed. Can't get list of tests to run. Fallback to all tests" "$GITHUB_STEP_SUMMARY"
               # We need not forget to add the search options in the fallback command, otherwise pytest might run more tests than expected
               # These search options match what's defined in codecov.yml:105
-              echo 'ATS_TESTS_TO_RUN<<EOF' >> $GITHUB_OUTPUT
-              jq -c @json <<< '[
+              jq '@json' --raw-output <<< '[
                 "--cov-context=test",
                 "tests/sentry",
                 "tests/integration",
@@ -126,44 +135,53 @@ jobs:
                 "--ignore=tests/sentry/search/events",
                 "--ignore=tests/sentry/ingest/ingest_consumer/test_ingest_consumer_kafka.py",
                 "--ignore=tests/sentry/region_to_control/test_region_to_control_kafka.py"
-              ]' >> $GITHUB_OUTPUT
-              echo 'EOF' >> $GITHUB_OUTPUT
-              echo ATS_TESTS_TO_SKIP="'[]'" >> "$GITHUB_OUTPUT"
-              echo "::error ATS failed"
-              exit 1
+              ]' > .artifacts/codecov_ats/tests_to_skip.json
+              echo '[]' > .artifacts/codecov_ats/tests_to_run.json
+              # If we reached this point it means that ATS failed with some error
+              tee <<< '{"ats_success": false, "error": "exception_raised"}' "$GITHUB_STEP_SUMMARY" ".artifacts/codecov_ats/result.json"
             fi
           env:
             CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
             CODECOV_STATIC_TOKEN: ${{ secrets.CODECOV_STATIC_TOKEN }}
+        - uses: actions/upload-artifact@v3
+          with:
+            name: codecov_ats
+            path: .artifacts/codecov_ats
+            if-no-files-found: error
     # The actual running of tests would come here, after the labels are available
     # Something like pytest <options> $ATS_TESTS_TO_RUN
     debug:
       runs-on: ubuntu-latest
-      needs: coverage-ats
-      if: ${{ always() }}
+      needs:
+        - coverage-ats
+        - files-changed
+      # Avoids running this job if it's a frontend change
+      # It would fail if the coverage-ats step didn't run
+      if: needs.files-changed.outputs.backend == 'true'
       steps:
+        - uses: actions/download-artifact@v3
+          with:
+            name: codecov_ats
+            path: .artifacts
         - name: Debug ATS_TESTS_TO_RUN
           run: |
-            : ${{ needs.coverage-ats.outputs.ATS_TESTS_TO_RUN }}
-            length_of_tests=$(jq <<< ${{ needs.coverage-ats.outputs.ATS_TESTS_TO_RUN }} 'length')
+            length_of_tests=$(cat .artifacts/tests_to_run.json | jq 'length')
             # The 1st value doesn't count, it's '--cov-context=test' (hence -gt 1)
             if [ $length_of_tests -gt 1 ]; then
               echo "Running $length_of_tests tests"
               # --raw-output0 doesn't work.
-              jq <<< ${{ needs.coverage-ats.outputs.ATS_TESTS_TO_RUN }} 'join("\u0000")' --raw-output | tr -d '\n' | xargs -r0 echo
+              cat .artifacts/tests_to_run.json | jq 'join("\u0000")' --raw-output | tr -d '\n' | xargs -r0 echo 'pytest'
             else
               echo "No tests to run"
             fi
         - name: Debug ATS_TESTS_TO_SKIP
           run: |
-            : ${{ needs.coverage-ats.outputs.ATS_TESTS_TO_SKIP }}
-            ATS_TESTS_TO_SKIP='${{ needs.coverage-ats.outputs.ATS_TESTS_TO_SKIP }}'
-            length_of_tests=$(jq <<< ${{ needs.coverage-ats.outputs.ATS_TESTS_TO_SKIP }} 'length')
+            length_of_tests=$(cat .artifacts/tests_to_skip.json | jq 'length')
             # The 1st value doesn't count, it's '--cov-context=test'
             if [ $length_of_tests -gt 1 ]; then
               echo "Running $length_of_tests tests"
               # --raw-output0 doesn't work.
-              jq <<< ${{ needs.coverage-ats.outputs.ATS_TESTS_TO_SKIP }} 'join("\u0000")' --raw-output | tr -d '\n' | xargs -r0 echo
+              cat .artifacts/tests_to_skip.json | jq 'join("\u0000")' --raw-output | tr -d '\n' | xargs -r0 echo 'pytest'
             else
               echo "No tests to run"
             fi

+ 45 - 0
.github/workflows/codecov_carryforward_reports.yml

@@ -0,0 +1,45 @@
+name: Carry forward codecov reports
+# This workflow carries forward coverage reports for commits in master
+# The coverage reports are generated by .github/workflows/codecov_per_test_coverage.yml
+# By carrying forward the reports and uploading the static analysis information
+# We can use the commits in master as the BASE for Automated Test Selection
+# see .github/workflows/codecov_ats.yml
+
+on:
+  push:
+    branches: [master]
+
+jobs:
+  carryforward-reports-and-upload-static-analysis:
+      runs-on: ubuntu-latest
+      steps:
+        - uses: actions/checkout@v3
+        - name: Set up Python 3.10.10
+          uses: actions/setup-python@v4
+          with:
+            python-version: "3.10.10"
+        - name: Download Codecov CLI
+          run: |
+            pip install --extra-index-url https://pypi.org/simple --no-cache-dir pytest codecov-cli==0.4.0
+        # Creates the commit and report objects in codecov
+        # This carries forward previouly uploaded coverage reports to the new commit
+        - name: Codecov startup
+          run: |
+            codecovcli create-commit
+            codecovcli create-report
+          env:
+            CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
+        # Sends static analysis information to codecov
+        # This is used as an input in Codecov Automated Test Selection.
+        # It's necessary so we can use this commit as the BASE for comparison
+        - name: Static Analysis
+          run: |
+            codecovcli static-analysis --token=${CODECOV_STATIC_TOKEN} \
+            --folders-to-exclude .artifacts \
+            --folders-to-exclude .github \
+            --folders-to-exclude .venv \
+            --folders-to-exclude static \
+            --folders-to-exclude bin
+          env:
+            CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
+            CODECOV_STATIC_TOKEN: ${{ secrets.CODECOV_STATIC_TOKEN }}

+ 75 - 0
.github/workflows/codecov_per_test_coverage.yml

@@ -0,0 +1,75 @@
+name: Codecov - per test coverage
+# This workflow generates pytest coverage with the flag --cov-context=test
+# This coverage is used as input for Codecov Automated Test Selection (see .github/workflows/codecov_ats.yml)
+# However there's a performance toll in running tests with this flag.
+# So we will not be running the test suite on every commit
+
+on: [workflow_dispatch, workflow_call]
+
+jobs:
+  # Same as 'backend' in .github/workflows/backed.yml
+  # Except for run_backend_tests step (which includes the extra --cov-context=test flag)
+  # And the coverage generation and handling
+  backend-test-with-cov-context:
+      if: github.ref == 'refs/heads/master'
+      name: backend test
+      runs-on: ubuntu-20.04
+      timeout-minutes: 120
+      strategy:
+        # This helps not having to run multiple jobs because one fails, thus, reducing resource usage
+        # and reducing the risk that one of many runs would turn red again (read: intermittent tests)
+        fail-fast: false
+        matrix:
+          # XXX: When updating this, make sure you also update MATRIX_INSTANCE_TOTAL.
+          instance: [0, 1, 2, 3, 4, 5, 6]
+          pg-version: ['14']
+
+      env:
+        # XXX: `MATRIX_INSTANCE_TOTAL` must be hardcoded to the length of `strategy.matrix.instance`.
+        # If this increases, make sure to also increase `flags.backend.after_n_builds` in `codecov.yml`.
+        MATRIX_INSTANCE_TOTAL: 7
+
+      steps:
+        - uses: actions/checkout@93ea575cb5d8a053eaa0ac8fa3b40d7e05a33cc8 # v3.1.0
+          with:
+            # Avoid codecov error message related to SHA resolution:
+            # https://github.com/codecov/codecov-bash/blob/7100762afbc822b91806a6574658129fe0d23a7d/codecov#L891
+            fetch-depth: '2'
+
+        - name: Setup sentry env
+          uses: ./.github/actions/setup-sentry
+          id: setup
+          with:
+            kafka: true
+            snuba: true
+            symbolicator: true
+            # Right now, we run so few bigtable related tests that the
+            # overhead of running bigtable in all backend tests
+            # is way smaller than the time it would take to run in its own job.
+            bigtable: true
+            pg-version: ${{ matrix.pg-version }}
+
+        - name: Run backend test (${{ steps.setup.outputs.matrix-instance-number }} of ${{ steps.setup.outputs.matrix-instance-total }}) with --cov-context=test
+          id: run_backend_tests
+          run: |
+            make test-python-ci COV_ARGS=--cov-context=test
+
+        # Separate from the testing step above so that we always create the report
+        # Even if some tests fail
+        - name: Create coverage report in JSON format
+          if: ${{ always() }}
+          run: |
+            coverage json --show-contexts -o .artifacts/python.coverage.json
+
+        # Upload coverage data even if running the tests step fails since
+        # it reduces large coverage fluctuations
+        - name: Upload coverage - special case to test Codecov ATS
+          if: ${{ always() }}
+          uses: codecov/codecov-action@v4-beta
+          env:
+            CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
+          with:
+            files: .artifacts/python.coverage.codecov.json
+            flags: smart-tests
+            plugins: compress-pycoverage
+          continue-on-error: true

+ 9 - 0
.github/workflows/shuffle-tests.yml

@@ -3,6 +3,11 @@ name: shuffle-tests
 on:
   # Allow manually running
   workflow_dispatch:
+    inputs:
+      per-test-coverage:
+        description: Whether to get per-test coverage (uses ./github/workflows/codecov_per_test_coverage.yml)
+        required: true
+        default: 'true'
   # Run once a week on sunday
   schedule:
     - cron: '0 1 * * 0'
@@ -15,6 +20,10 @@ env:
   SENTRY_SHUFFLE_TESTS: true
 
 jobs:
+  per-test-coverage:
+    if: ${{ inputs.per-test-coverage == 'true' || github.event_name == 'schedule' }}
+    uses: ./.github/workflows/codecov_per_test_coverage.yml
+    secrets: inherit
   backend-test:
     name: run backend tests
     runs-on: ubuntu-20.04

+ 8 - 1
Makefile

@@ -125,6 +125,13 @@ test-js-ci: node-version-check
 	@yarn run test-ci
 	@echo ""
 
+# COV_ARGS controls extra args passed to pytest to generate covereage
+# It's used in test-python-ci. Typically generated an XML coverage file
+# Except in .github/workflows/codecov_per_test_coverage.yml
+# When it's dynamically changed to include --cov-context=test flag
+# See that workflow for more info
+COV_ARGS = --cov-report="xml:.artifacts/python.coverage.xml"
+
 test-python-ci: create-db
 	@echo "--> Running CI Python tests"
 	pytest \
@@ -133,7 +140,7 @@ test-python-ci: create-db
 		--ignore tests/apidocs \
 		--ignore tests/js \
 		--ignore tests/tools \
-		--cov . --cov-report="xml:.artifacts/python.coverage.xml"
+		--cov . $(COV_ARGS)
 	@echo ""
 
 # it's not possible to change settings.DATABASE after django startup, so

+ 25 - 13
codecov.yml

@@ -1,10 +1,8 @@
 # Setting coverage targets per flag
 coverage:
   status:
-    project:
-      default: false
+    project: false
     patch:
-      default: false
       frontend:
         # codecov will not fail status checks for master
         only_pulls: true
@@ -62,6 +60,18 @@ flags:
     # NOTE: If you change this, make sure to change `comment.after_n_builds` below as well.
     after_n_builds: 18
 
+# https://docs.codecov.com/docs/flags#two-approaches-to-flag-management
+flag_management:
+  individual_flags:
+    - name: smart-tests
+      carryforward: true
+      # https://docs.codecov.com/docs/getting-started-with-ats#step-2-configure-the-codecovyml-for-automated-test-selection
+      carryforward_mode: "labels"
+      statuses:
+        # https://github.com/codecov/shared/blob/main/shared/validation/user_schema.py#L310
+        - type: "patch"
+          only_pulls: true
+
 # Read more here: https://docs.codecov.com/docs/pull-request-comments
 comment:
   # This is the addition of carry forward builds and fresh builds, thus, it's the addition
@@ -83,16 +93,18 @@ cli:
   plugins:
     pycoverage:
       report_type: "json"
+    compress-pycoverage:
+      file_to_compress: ".artifacts/python.coverage.json"
+      # We don't want to upload the original coverage report by accident
+      # Because it's too big
+      delete_uncompressed: true
   runners:
-    python:
-      include_curr_dir: true
+    pytest:
       # Same args used for the backend tests
+      # See Makefile:135
       collect_tests_options:
-        - "tests/sentry"
-        - "tests/integration"
-        - "--ignore=tests/sentry/eventstream/kafka"
-        - "--ignore=tests/sentry/post_process_forwarder"
-        - "--ignore=tests/sentry/snuba"
-        - "--ignore=tests/sentry/search/events"
-        - "--ignore=tests/sentry/ingest/ingest_consumer/test_ingest_consumer_kafka.py"
-        - "--ignore=tests/sentry/region_to_control/test_region_to_control_kafka.py"
+        - "tests"
+        - "--ignore=tests/acceptance"
+        - "--ignore=tests/apidocs"
+        - "--ignore=tests/js"
+        - "--ignore=tests/tools"