diff --git a/.github/workflows/trigger-integration-tests.yml b/.github/workflows/trigger-integration-tests.yml index 9954d1539..b8bb90ba2 100644 --- a/.github/workflows/trigger-integration-tests.yml +++ b/.github/workflows/trigger-integration-tests.yml @@ -6,8 +6,8 @@ name: Trigger Integration Tests # Mirrors the canonical pattern in adbc-drivers/databricks. The model: # # - On a normal PR event (open / push / reopen / non-IT label) we -# post `success` Python Proxy Tests checks immediately so the -# required checks don't block the PR. The real tests are gated +# post a `success` Python Integration Tests check immediately so the +# required check doesn't block the PR. The real tests are gated # in the merge queue. # - When a maintainer adds the `integration-test` label we dispatch # the suite as a preview — useful for catching regressions before @@ -18,14 +18,14 @@ name: Trigger Integration Tests # gate. Only PRs whose tests dispatch (or auto-pass when no driver # files changed) can proceed to `main`. # -# Check-run names: databricks-driver-test's python-proxy-tests.yml is -# a `mode: [thrift, kernel]` matrix that posts two named checks per -# run — `Python Proxy Tests / thrift` and `Python Proxy Tests / kernel`. -# Every synthetic-success / auto-pass / dispatch-failure step below -# posts both names so the matrix legs always have a matching baseline -# check on the PR. The list of modes lives in the `MODES` constant -# at the top of each script block; keep it in sync with the matrix -# axis in databricks-driver-test/.github/workflows/python-proxy-tests.yml. +# Check-run name: databricks-driver-test's databricks-python-integration-tests.yml +# fans out the thrift + kernel backends INTERNALLY (matrix) and reports a +# SINGLE aggregated `Python Integration Tests` check — matching the go/nodejs +# receivers. This sender dispatches ONE `python-pr-test` (proxy_mode: replay) +# and every synthetic-success / auto-pass / dispatch-failure step posts that +# one check name so it always has a matching baseline on the PR. (The older +# per-mode `Python Proxy Tests / ` checks came from the shared reusable +# workflow, which is retained only for the weekly slow cron — not this gate.) # # Required external setup (outside this workflow): # @@ -34,17 +34,16 @@ name: Trigger Integration Tests # 2. `INTEGRATION_TEST_APP_ID` / `INTEGRATION_TEST_PRIVATE_KEY` repo # secrets installed for the dispatcher GitHub App (write access # to databricks/databricks-driver-test). -# 3. Merge queue enabled on `main` branch protection AND BOTH -# `Python Proxy Tests / thrift` and `Python Proxy Tests / kernel` -# listed as required status checks. Without this the merge-queue -# job is dead code and ITs run only on explicit label. The legacy -# `Python Proxy Tests` (no mode suffix) check is no longer posted -# by any workflow and must be removed from the required-checks -# list when this change lands. +# 3. Merge queue enabled on `main` branch protection AND +# `Python Integration Tests` listed as a required status check. +# Without this the merge-queue job is dead code and ITs run only on +# explicit label. When this change lands, swap the required-checks +# list: remove `Python Proxy Tests / thrift` and `Python Proxy Tests +# / kernel`, add `Python Integration Tests`. on: pull_request: - types: [opened, synchronize, reopened, labeled] + types: [opened, synchronize, reopened, labeled, closed] merge_group: # Trigger when added to merge queue jobs: @@ -118,21 +117,21 @@ jobs: }); # ============================================================================= - # For PRs: Always pass the per-mode Python Proxy Tests checks on + # For PRs: Always pass the Python Integration Tests check on # non-label events. The real run happens in the merge queue (or via # explicit label preview). Without this, the required - # `Python Proxy Tests / thrift` and `Python Proxy Tests / kernel` - # checks would block every PR that doesn't bother labelling. + # `Python Integration Tests` check would block every PR that doesn't + # bother labelling. # ============================================================================= skip-integration-tests-pr: - if: github.event_name == 'pull_request' && github.event.action != 'labeled' + if: github.event_name == 'pull_request' && github.event.action != 'labeled' && github.event.action != 'closed' runs-on: group: databricks-protected-runner-group labels: linux-ubuntu-latest permissions: checks: write steps: - - name: Skip Python Proxy Tests + - name: Skip Python Integration Tests uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 with: github-token: ${{ github.token }} @@ -141,32 +140,29 @@ jobs: // the declared `checks: write`, so checks.create 403s ("Resource // not accessible by integration"). Expected — a fork can't post // check-runs on the base repo. Swallow the 403 for forks so this - // poster doesn't show a spurious failure; the real Python Proxy - // Tests required checks are posted by the merge_group run (full - // perms) when a maintainer queues the PR. Other errors fail loudly. + // poster doesn't show a spurious failure; the real Python + // Integration Tests required check is posted by the merge_group run + // (full perms) when a maintainer queues the PR. Other errors fail loudly. const isFork = context.payload.pull_request.head.repo.fork; - const MODES = ['thrift', 'kernel']; - for (const mode of MODES) { - try { - await github.rest.checks.create({ - owner: context.repo.owner, - repo: context.repo.repo, - name: `Python Proxy Tests / ${mode}`, - head_sha: context.payload.pull_request.head.sha, - status: 'completed', - conclusion: 'success', - completed_at: new Date().toISOString(), - output: { - title: 'Skipped on PR — runs in merge queue', - summary: `Python Proxy Tests (${mode}) are skipped on PRs and run as a required gate in the merge queue. Add the \`integration-test\` label to preview them on this PR.` - } - }); - } catch (e) { - if (isFork && e.status === 403) { - core.notice(`Fork PR: cannot post the Python Proxy Tests / ${mode} check-run (read-only token). It will be posted by the merge queue at merge time.`); - } else { - throw e; + try { + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + name: 'Python Integration Tests', + head_sha: context.payload.pull_request.head.sha, + status: 'completed', + conclusion: 'success', + completed_at: new Date().toISOString(), + output: { + title: 'Skipped on PR — runs in merge queue', + summary: 'Python Integration Tests are skipped on PRs and run as a required gate in the merge queue. Add the `integration-test` label to preview them on this PR.' } + }); + } catch (e) { + if (isFork && e.status === 403) { + core.notice('Fork PR: cannot post the Python Integration Tests check-run (read-only token). It will be posted by the merge queue at merge time.'); + } else { + throw e; } } @@ -251,10 +247,11 @@ jobs: "pr_repo": "${{ github.repository }}", "pr_url": "${{ github.event.pull_request.html_url }}", "pr_title": "${{ steps.sanitize.outputs.result }}", - "pr_author": "${{ github.event.pull_request.user.login }}" + "pr_author": "${{ github.event.pull_request.user.login }}", + "proxy_mode": "replay" } - - name: Pass Python Proxy Tests check (no driver changes) + - name: Pass Python Integration Tests check (no driver changes) if: steps.changed.outputs.python != 'true' uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 with: @@ -264,22 +261,19 @@ jobs: # no-op runs. github-token: ${{ github.token }} script: | - const MODES = ['thrift', 'kernel']; - for (const mode of MODES) { - await github.rest.checks.create({ - owner: context.repo.owner, - repo: context.repo.repo, - name: `Python Proxy Tests / ${mode}`, - head_sha: context.payload.pull_request.head.sha, - status: 'completed', - conclusion: 'success', - completed_at: new Date().toISOString(), - output: { - title: 'Skipped — no driver changes', - summary: `No Python driver source files changed; skipping ${mode} integration tests.` - } - }); - } + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + name: 'Python Integration Tests', + head_sha: context.payload.pull_request.head.sha, + status: 'completed', + conclusion: 'success', + completed_at: new Date().toISOString(), + output: { + title: 'Skipped — no driver changes', + summary: 'No Python driver source files changed; skipping integration tests.' + } + }); - name: Fail check on dispatch error if: failure() && steps.changed.outputs.python == 'true' @@ -295,22 +289,19 @@ jobs: # which is all we need. github-token: ${{ github.token }} script: | - const MODES = ['thrift', 'kernel']; - for (const mode of MODES) { - await github.rest.checks.create({ - owner: context.repo.owner, - repo: context.repo.repo, - name: `Python Proxy Tests / ${mode}`, - head_sha: context.payload.pull_request.head.sha, - status: 'completed', - conclusion: 'failure', - completed_at: new Date().toISOString(), - output: { - title: 'Failed — error dispatching tests', - summary: `An error occurred while dispatching Python integration tests (${mode}). Check the workflow run logs.` - } - }); - } + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + name: 'Python Integration Tests', + head_sha: context.payload.pull_request.head.sha, + status: 'completed', + conclusion: 'failure', + completed_at: new Date().toISOString(), + output: { + title: 'Failed — error dispatching tests', + summary: 'An error occurred while dispatching Python integration tests. Check the workflow run logs.' + } + }); - name: Comment on PR if: steps.changed.outputs.python == 'true' @@ -321,7 +312,7 @@ jobs: owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, - body: 'Integration tests triggered. [View workflow run](https://github.com/databricks/databricks-driver-test/actions/workflows/python-proxy-tests.yml).' + body: 'Integration tests triggered. [View workflow runs](https://github.com/databricks/databricks-driver-test/actions/workflows/databricks-python-integration-tests.yml). Result posts back here as the "Python Integration Tests" check.' }); # ============================================================================= @@ -365,22 +356,19 @@ jobs: # equivalent step above for the rationale. github-token: ${{ github.token }} script: | - const MODES = ['thrift', 'kernel']; - for (const mode of MODES) { - await github.rest.checks.create({ - owner: context.repo.owner, - repo: context.repo.repo, - name: `Python Proxy Tests / ${mode}`, - head_sha: '${{ github.event.merge_group.head_sha }}', - status: 'completed', - conclusion: 'success', - completed_at: new Date().toISOString(), - output: { - title: 'Skipped — no driver changes', - summary: `No Python driver source files changed (${mode}).` - } - }); - } + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + name: 'Python Integration Tests', + head_sha: '${{ github.event.merge_group.head_sha }}', + status: 'completed', + conclusion: 'success', + completed_at: new Date().toISOString(), + output: { + title: 'Skipped — no driver changes', + summary: 'No Python driver source files changed.' + } + }); - name: Extract PR number from merge queue ref if: steps.changed.outputs.changed == 'true' @@ -422,7 +410,8 @@ jobs: "pr_repo": "${{ github.repository }}", "pr_url": "${{ github.server_url }}/${{ github.repository }}/pull/${{ steps.extract-pr.outputs.pr_number }}", "pr_title": "Merge queue validation", - "pr_author": "merge-queue" + "pr_author": "merge-queue", + "proxy_mode": "replay" } - name: Fail check on dispatch error @@ -433,19 +422,153 @@ jobs: # the rationale in the trigger-tests-pr job above. github-token: ${{ github.token }} script: | - const MODES = ['thrift', 'kernel']; - for (const mode of MODES) { - await github.rest.checks.create({ + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + name: 'Python Integration Tests', + head_sha: '${{ github.event.merge_group.head_sha }}', + status: 'completed', + conclusion: 'failure', + completed_at: new Date().toISOString(), + output: { + title: 'Failed — error dispatching tests', + summary: 'An error occurred while dispatching Python integration tests. Check the workflow run logs.' + } + }); + + # ============================================================================= + # After merge: trigger the multi-language coverage fan-out. + # Fires when a PR lands on main (merge queue or direct merge) and touched + # driver source. Dispatches `coverage-fanout` to databricks-driver-test, whose + # coverage-fanout-tracker.yml opens a tracking issue and runs the + # language-agnostic fan-out (a spec authored from THIS PR's diff, conformed as + # tests across every driver) as peco-engineer-bot. + # + # Fork-PR limitation: for a PR opened from an external fork, GitHub runs the + # `pull_request` (closed/merged) event with NO repository secrets and a + # read-only GITHUB_TOKEN. That means both the App-token generation step and + # the "Signal dispatch failure" fallback (which uses github.token to comment) + # cannot run for fork merges, so those merges are intentionally excluded from + # the fan-out — no dispatch and, by design, no failure comment. Coverage for a + # fork contribution is instead picked up by the next source-affecting merge + # from a maintainer branch, or the fan-out can be dispatched manually against + # databricks-driver-test. Wiring this off a `push`-to-`main` trigger (which + # does have secret access) would restore fork coverage but is a larger change + # and is deliberately out of scope here. + # ============================================================================= + trigger-coverage-fanout: + if: | + github.event_name == 'pull_request' && + github.event.action == 'closed' && + github.event.pull_request.merged == true && + github.event.pull_request.base.ref == 'main' && + github.event.pull_request.head.repo.full_name == github.repository + # Serialize by PR so a manual re-run (e.g. recovery after the failure + # comment fires, or an accidental Actions "Re-run") cannot overlap with an + # in-flight run and double-dispatch coverage-fanout. cancel-in-progress is + # false so a queued re-run waits rather than killing the original; the + # tracker in databricks-driver-test is the source of truth for dedup across + # sequential re-runs. + concurrency: + group: coverage-fanout-${{ github.event.pull_request.number }} + cancel-in-progress: false + runs-on: + group: databricks-protected-runner-group + labels: linux-ubuntu-latest + permissions: + contents: read + pull-requests: write + issues: write + steps: + - name: Check if driver source changed + id: changed + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + with: + script: | + const files = await github.paginate(github.rest.pulls.listFiles, { + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: context.payload.pull_request.number, + per_page: 100, + }); + // The whole repo IS the driver. Count a merge as source-affecting when it changes a file under src/. + // Docs/CI/test-only merges do not warrant a full multi-language fan-out. + // Note this is intentionally NARROWER than the PR-level IT gate (trigger-tests-pr / + // merge-queue-python), which also treats pyproject.toml / poetry.lock as driver-affecting + // ("dep bumps can break the integration suite"). Dependency-only merges are DELIBERATELY + // excluded here: the fan-out authors a conformance spec from THIS PR's source diff, and a + // dep-only bump produces no driver-behavior diff to conform into tests across drivers. + const isSource = (f) => f.startsWith('src/'); + // GitHub caps pulls.listFiles at 3000 files per PR (even via paginate). If a merge is that + // large the list is truncated, so a src/ file could sort beyond the cap and be missed. Since + // the whole gate hinges on this boolean, treat a truncated result set as source-affecting. + const truncated = files.length >= 3000; + const srcChanged = truncated || files.some((f) => isSource(f.filename)); + if (truncated) { + console.log('listFiles hit the 3000-file cap; treating merge as source-affecting.'); + } + console.log(`driver source changed: ${srcChanged}`); + core.setOutput('source', srcChanged.toString()); + + - name: Generate GitHub App token (databricks-driver-test) + if: steps.changed.outputs.source == 'true' + id: app-token + uses: actions/create-github-app-token@f8d387b68d61c58ab83c6c016672934102569859 # v3.0.0 + with: + app-id: ${{ secrets.INTEGRATION_TEST_APP_ID }} + private-key: ${{ secrets.INTEGRATION_TEST_PRIVATE_KEY }} + owner: databricks + repositories: databricks-driver-test + permission-contents: write + + - name: Dispatch coverage-fanout + if: steps.changed.outputs.source == 'true' + uses: peter-evans/repository-dispatch@ff45666b9427631e3450c54a1bcbee4d9ff4d7c0 # v3.0.0 + with: + token: ${{ steps.app-token.outputs.token }} + repository: databricks/databricks-driver-test + event-type: coverage-fanout + client-payload: '{"reference_repo": "${{ github.repository }}", "pr_number": "${{ github.event.pull_request.number }}", "pr_url": "${{ github.event.pull_request.html_url }}"}' + + - name: Signal dispatch failure + # Best-effort fan-out: the PR is already merged, so there is no + # required check to turn red. Without this handler a broken dispatch + # (rotated App secret, App uninstalled, driver-test API error) fails + # the step but surfaces nowhere and the coverage fan-out silently + # never runs. Emit a workflow warning and comment on the merged PR so + # the failure is noticeable. Uses the default token (checks/PR write + # via job permissions), not the App token, since App-token generation + # is itself a likely failure point. + # Gate on source != 'false' rather than == 'true': if the detection + # step itself fails, `source` is never set (empty, not 'true'), and a + # == 'true' gate would skip this handler too, so that failure would + # surface nowhere. Empty and 'true' both satisfy != 'false'; only a + # clean 'false' (no source change, nothing dispatched) stays silent. + if: failure() && steps.changed.outputs.source != 'false' + uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0 + with: + github-token: ${{ github.token }} + script: | + const runUrl = + `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}` + + `/actions/runs/${context.runId}`; + core.warning( + `Failed to run the coverage fan-out for databricks-driver-test ` + + `(source detection or dispatch step failed); the multi-language ` + + `coverage fan-out did not run. See ${runUrl}` + ); + try { + await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, - name: `Python Proxy Tests / ${mode}`, - head_sha: '${{ github.event.merge_group.head_sha }}', - status: 'completed', - conclusion: 'failure', - completed_at: new Date().toISOString(), - output: { - title: 'Failed — error dispatching tests', - summary: `An error occurred while dispatching Python integration tests (${mode}). Check the workflow run logs.` - } + issue_number: context.payload.pull_request.number, + body: + `⚠️ Failed to run the multi-language coverage fan-out ` + + `to \`databricks-driver-test\` after this PR merged ` + + `(source detection or dispatch step failed). Coverage ` + + `was not extended for this change. ` + + `[Workflow run](${runUrl})`, }); + } catch (e) { + core.warning(`Could not comment on the merged PR: ${e.message}`); }