-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
feat(infra): Add backend selective testing workflow #105500
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
58f5c78
e45a566
7f8652b
740ad75
2d4b6af
e0aca03
38ba69b
3f8b896
9aabeb7
4258b12
e1b720e
c0de6e6
6d6d3c8
9b9ddd2
b5265d2
9ad854d
bf78444
862cff1
78f129e
fc23c8f
0b26c08
25c4693
7b566a7
917abe3
6a39767
84a1c19
11c7547
0bd2999
5d7fded
8e7347d
6b5826f
ac0d587
25b0fff
f0093e3
54aa22c
32aac6e
b257dae
52d3c99
043cfd8
e3e0501
19db7ab
c530b88
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,261 @@ | ||
| name: '[NOT REQUIRED] backend (selective)' | ||
|
|
||
| on: | ||
| pull_request: | ||
| paths: | ||
| - 'src/sentry/preprod/**' | ||
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} | ||
| cancel-in-progress: true | ||
|
|
||
| # hack for https://github.com/actions/cache/issues/810#issuecomment-1222550359 | ||
| env: | ||
| SEGMENT_DOWNLOAD_TIMEOUT_MINS: 3 | ||
| SNUBA_NO_WORKERS: 1 | ||
|
|
||
| jobs: | ||
| files-changed: | ||
| name: detect what files changed | ||
| runs-on: ubuntu-24.04 | ||
| timeout-minutes: 3 | ||
| continue-on-error: true | ||
| outputs: | ||
| api_docs: ${{ steps.changes.outputs.api_docs }} | ||
| backend: ${{ steps.changes.outputs.backend_all }} | ||
| backend_dependencies: ${{ steps.changes.outputs.backend_dependencies }} | ||
| backend_api_urls: ${{ steps.changes.outputs.backend_api_urls }} | ||
| backend_any_type: ${{ steps.changes.outputs.backend_any_type }} | ||
| migration_lockfile: ${{ steps.changes.outputs.migration_lockfile }} | ||
| steps: | ||
| - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 | ||
|
|
||
| - name: Check for backend file changes | ||
| uses: dorny/paths-filter@0bc4621a3135347011ad047f9ecf449bf72ce2bd # v3.0.0 | ||
| id: changes | ||
| with: | ||
| token: ${{ github.token }} | ||
| filters: .github/file-filters.yml | ||
|
|
||
| prepare-selective-tests: | ||
| if: needs.files-changed.outputs.backend == 'true' | ||
| needs: files-changed | ||
| name: prepare selective tests | ||
| runs-on: ubuntu-24.04 | ||
| timeout-minutes: 10 | ||
| continue-on-error: true | ||
| permissions: | ||
| contents: read | ||
| id-token: write | ||
| outputs: | ||
| has-coverage: ${{ steps.find-coverage.outputs.found }} | ||
| coverage-sha: ${{ steps.find-coverage.outputs.coverage-sha }} | ||
| changed-files: ${{ steps.changed-files.outputs.files }} | ||
| test-count: ${{ steps.compute-tests.outputs.test-count }} | ||
| has-selected-tests: ${{ steps.compute-tests.outputs.has-selected-tests }} | ||
| steps: | ||
| - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 | ||
| with: | ||
| fetch-depth: 0 # Need full history for git diff | ||
|
|
||
| - name: Setup Python | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: '3.13.1' | ||
|
|
||
| - name: Authenticate to Google Cloud | ||
| id: gcloud-auth | ||
| uses: google-github-actions/auth@v2 | ||
| with: | ||
| project_id: sentry-dev-tooling | ||
| workload_identity_provider: ${{ secrets.SENTRY_GCP_DEV_WORKLOAD_IDENTITY_POOL }} | ||
| service_account: ${{ secrets.COLLECT_TEST_DATA_SERVICE_ACCOUNT_EMAIL }} | ||
|
|
||
| - name: Find coverage data for selective testing | ||
| id: find-coverage | ||
| env: | ||
| GCS_BUCKET: sentry-coverage-data | ||
| run: | | ||
| set -euo pipefail | ||
|
|
||
| # Get the base commit (what the PR branches from) | ||
| BASE_SHA="${{ github.event.pull_request.base.sha }}" | ||
|
|
||
| echo "Looking for coverage data starting from base commit: $BASE_SHA" | ||
|
|
||
| COVERAGE_SHA="" | ||
| for sha in $(git rev-list "$BASE_SHA" --max-count=30); do | ||
| # Check if coverage exists in GCS for this commit | ||
| if gcloud storage ls "gs://${GCS_BUCKET}/${sha}/" &>/dev/null; then | ||
| COVERAGE_SHA="$sha" | ||
| echo "Found coverage data at commit: $sha" | ||
| break | ||
| fi | ||
| echo "No coverage at $sha, checking parent..." | ||
| done | ||
|
|
||
| if [[ -z "$COVERAGE_SHA" ]]; then | ||
| echo "No coverage found in last 30 commits, will run full test suite" | ||
| echo "found=false" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "found=true" >> "$GITHUB_OUTPUT" | ||
| echo "coverage-sha=$COVERAGE_SHA" >> "$GITHUB_OUTPUT" | ||
| fi | ||
|
|
||
| - name: Download coverage database | ||
| id: download-coverage | ||
| if: steps.find-coverage.outputs.found == 'true' | ||
| env: | ||
| COVERAGE_SHA: ${{ steps.find-coverage.outputs.coverage-sha }} | ||
| run: | | ||
| set -euxo pipefail | ||
| mkdir -p .coverage | ||
|
|
||
| if ! gcloud storage cp "gs://sentry-coverage-data/${COVERAGE_SHA}/.coverage.combined" .coverage/; then | ||
| echo "Warning: Failed to download coverage file" | ||
| echo "coverage-file=" >> "$GITHUB_OUTPUT" | ||
| exit 0 | ||
| fi | ||
|
|
||
| if [[ ! -f .coverage/.coverage.combined ]]; then | ||
| echo "Warning: Coverage file not found after download" | ||
| ls -la .coverage/ || true | ||
| echo "coverage-file=" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "Downloaded coverage file: .coverage/.coverage.combined" | ||
| echo "coverage-file=.coverage/.coverage.combined" >> "$GITHUB_OUTPUT" | ||
| fi | ||
|
|
||
| - name: Get changed files | ||
| id: changed-files | ||
| run: | | ||
| # Get files changed between base and head of PR | ||
| BASE_SHA="${{ github.event.pull_request.base.sha }}" | ||
| HEAD_SHA="${{ github.event.pull_request.head.sha }}" | ||
|
|
||
| CHANGED_FILES=$(git diff --name-only "$BASE_SHA" "$HEAD_SHA" | tr '\n' ' ') | ||
| echo "Changed files: $CHANGED_FILES" | ||
| echo "files=$CHANGED_FILES" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: Compute selected tests | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we also considering new files that are added here? And ensuring we are running all tests in new test files?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good callout, added |
||
| id: compute-tests | ||
| if: steps.download-coverage.outputs.coverage-file != '' | ||
| env: | ||
| COVERAGE_DB: ${{ steps.download-coverage.outputs.coverage-file }} | ||
| CHANGED_FILES: ${{ steps.changed-files.outputs.files }} | ||
| run: make compute-selected-tests | ||
|
|
||
| - name: Upload coverage database artifact | ||
| if: steps.download-coverage.outputs.coverage-file != '' | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: coverage-db-${{ github.run_id }} | ||
| path: .coverage/ | ||
| retention-days: 1 | ||
| include-hidden-files: true | ||
|
|
||
| - name: Upload selected tests artifact | ||
| if: steps.compute-tests.outputs.has-selected-tests == 'true' | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: selected-tests-${{ github.run_id }} | ||
| path: .artifacts/selected-tests.txt | ||
| retention-days: 1 | ||
|
|
||
| calculate-shards: | ||
| if: needs.files-changed.outputs.backend == 'true' | ||
| needs: [files-changed, prepare-selective-tests] | ||
| name: calculate test shards | ||
| runs-on: ubuntu-24.04 | ||
| timeout-minutes: 5 | ||
| continue-on-error: true | ||
| outputs: | ||
| shard-count: ${{ steps.calculate-shards.outputs.shard-count }} | ||
| shard-indices: ${{ steps.calculate-shards.outputs.shard-indices }} | ||
| steps: | ||
| - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 | ||
|
|
||
| - name: Setup sentry env | ||
| uses: ./.github/actions/setup-sentry | ||
| id: setup | ||
| with: | ||
| mode: backend-ci | ||
| skip-devservices: true | ||
|
|
||
| - name: Download selected tests artifact | ||
| if: needs.prepare-selective-tests.outputs.has-selected-tests == 'true' | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| name: selected-tests-${{ github.run_id }} | ||
| path: .artifacts/ | ||
|
|
||
| - name: Calculate test shards | ||
| id: calculate-shards | ||
| env: | ||
| SELECTED_TESTS_FILE: ${{ needs.prepare-selective-tests.outputs.has-selected-tests == 'true' && '.artifacts/selected-tests.txt' || '' }} | ||
| SELECTED_TEST_COUNT: ${{ needs.prepare-selective-tests.outputs.test-count }} | ||
| run: | | ||
| python3 .github/workflows/scripts/calculate-backend-test-shards.py | ||
|
|
||
| backend-test-selective: | ||
| if: needs.files-changed.outputs.backend == 'true' | ||
| needs: [files-changed, prepare-selective-tests, calculate-shards] | ||
| name: backend tests | ||
| runs-on: ubuntu-24.04 | ||
| timeout-minutes: 60 | ||
| continue-on-error: true | ||
| permissions: | ||
| contents: read | ||
| id-token: write | ||
| actions: read # used for DIM metadata | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| instance: ${{ fromJSON(needs.calculate-shards.outputs.shard-indices) }} | ||
|
|
||
| env: | ||
| MATRIX_INSTANCE_TOTAL: ${{ needs.calculate-shards.outputs.shard-count }} | ||
| TEST_GROUP_STRATEGY: roundrobin | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 | ||
|
|
||
| - name: Setup sentry env | ||
| uses: ./.github/actions/setup-sentry | ||
| id: setup | ||
| with: | ||
| mode: backend-ci | ||
|
|
||
| - name: Download selected tests artifact | ||
| if: needs.prepare-selective-tests.outputs.has-selected-tests == 'true' | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| name: selected-tests-${{ github.run_id }} | ||
| path: .artifacts/ | ||
|
|
||
| - name: Run backend tests (${{ steps.setup.outputs.matrix-instance-number }} of ${{ steps.setup.outputs.matrix-instance-total }}) | ||
| id: run_backend_tests | ||
| run: | | ||
| if [[ "${{ needs.prepare-selective-tests.outputs.has-selected-tests }}" == "true" ]]; then | ||
| make test-backend-ci-selective SELECTED_TESTS_FILE=.artifacts/selected-tests.txt | ||
| else | ||
| make test-python-ci | ||
| fi | ||
|
|
||
| - name: Inspect failure | ||
| if: failure() | ||
| run: | | ||
| if command -v devservices; then | ||
| devservices logs | ||
| fi | ||
|
|
||
| - name: Collect test data | ||
| uses: ./.github/actions/collect-test-data | ||
| if: ${{ !cancelled() }} | ||
| with: | ||
| artifact_path: .artifacts/pytest.json | ||
| gcs_bucket: ${{ secrets.COLLECT_TEST_DATA_GCS_BUCKET }} | ||
| gcp_project_id: ${{ secrets.COLLECT_TEST_DATA_GCP_PROJECT_ID }} | ||
| workload_identity_provider: ${{ secrets.SENTRY_GCP_DEV_WORKLOAD_IDENTITY_POOL }} | ||
| service_account_email: ${{ secrets.COLLECT_TEST_DATA_SERVICE_ACCOUNT_EMAIL }} | ||
| matrix_instance_number: ${{ steps.setup.outputs.matrix-instance-number }} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,28 +5,49 @@ | |
| import re | ||
| import subprocess | ||
| import sys | ||
| from pathlib import Path | ||
|
|
||
| TESTS_PER_SHARD = 1200 | ||
| MIN_SHARDS = 1 | ||
| MAX_SHARDS = 22 | ||
| DEFAULT_SHARDS = 22 | ||
|
|
||
| PYTEST_ARGS = [ | ||
| PYTEST_BASE_ARGS = [ | ||
| "pytest", | ||
| "--collect-only", | ||
| "--quiet", | ||
| "tests", | ||
| "--ignore=tests/acceptance", | ||
| "--ignore=tests/apidocs", | ||
| "--ignore=tests/js", | ||
| "--ignore=tests/tools", | ||
| ] | ||
|
|
||
|
|
||
| def collect_test_count(): | ||
| def collect_test_count() -> int | None: | ||
| """Collect the number of tests to run, either from selected files or full suite.""" | ||
| selected_tests_file = os.environ.get("SELECTED_TESTS_FILE") | ||
|
|
||
| if selected_tests_file: | ||
| path = Path(selected_tests_file) | ||
| if not path.exists(): | ||
| print(f"Selected tests file not found: {selected_tests_file}", file=sys.stderr) | ||
| return None | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we be returning None here? Or should it be an int
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated typing of this file, should all be handled now, but yes None is intentional as that defaults to 22 (existing count) shards. |
||
|
|
||
| with path.open() as f: | ||
| selected_files = [line.strip() for line in f if line.strip()] | ||
|
|
||
| if not selected_files: | ||
| print("No selected test files, running 0 tests", file=sys.stderr) | ||
| return 0 | ||
|
|
||
| print(f"Counting tests in {len(selected_files)} selected files", file=sys.stderr) | ||
| pytest_args = PYTEST_BASE_ARGS + selected_files | ||
| else: | ||
| pytest_args = PYTEST_BASE_ARGS + ["tests"] | ||
|
|
||
| try: | ||
| result = subprocess.run( | ||
| PYTEST_ARGS, | ||
| pytest_args, | ||
| capture_output=True, | ||
| text=True, | ||
| check=False, | ||
|
|
@@ -40,7 +61,6 @@ def collect_test_count(): | |
| print(f"Collected {count} tests", file=sys.stderr) | ||
| return count | ||
|
|
||
| # If no match, check if pytest failed | ||
| if result.returncode != 0: | ||
| print( | ||
| f"Pytest collection failed (exit {result.returncode})", | ||
|
|
@@ -56,7 +76,7 @@ def collect_test_count(): | |
| return None | ||
|
|
||
|
|
||
| def calculate_shards(test_count): | ||
| def calculate_shards(test_count: int | None) -> int: | ||
| if test_count is None: | ||
| print(f"Using default shard count: {DEFAULT_SHARDS}", file=sys.stderr) | ||
| return DEFAULT_SHARDS | ||
|
|
@@ -82,10 +102,9 @@ def calculate_shards(test_count): | |
| return bounded | ||
|
|
||
|
|
||
| def main(): | ||
| def main() -> int: | ||
| test_count = collect_test_count() | ||
| shard_count = calculate_shards(test_count) | ||
| # Generate a JSON array of shard indices [0, 1, 2, ..., shard_count-1] | ||
| shard_indices = json.dumps(list(range(shard_count))) | ||
|
|
||
| github_output = os.getenv("GITHUB_OUTPUT") | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried to make this super clear that it's not required so devs won't get confused if they see a red X they can merge past
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also might want/need to communicate this out to #team-engineering or some other larger slack channel to let devs know to ignore this check for the time being as we collect data