-
Notifications
You must be signed in to change notification settings - Fork 597
fix(ci): Add ability to run full test suite on PRs #6718
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
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,112 @@ | ||
| name: Full test suite trigger | ||
|
|
||
| # Maintainers trigger this workflow with `/ci-run-all-tests`. | ||
| # `issue_comment` uses the trusted default-branch workflow and supports statuses | ||
| # for fork PRs. | ||
| on: | ||
| issue_comment: | ||
| types: [created] | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| authorize: | ||
| name: Authorize and resolve target | ||
| if: >- | ||
| ${{ github.event.issue.pull_request != null && | ||
| github.event.comment.body == '/ci-run-all-tests' }} | ||
| runs-on: ubuntu-24.04 | ||
| timeout-minutes: 5 | ||
| permissions: | ||
| contents: read | ||
| pull-requests: read | ||
| outputs: | ||
| head_sha: ${{ steps.resolve.outputs.head_sha }} | ||
| merge_sha: ${{ steps.resolve.outputs.merge_sha }} | ||
| steps: | ||
| # Authorize the original commenter; `github.actor` changes on retries. | ||
| - name: Check commenter has write access or above | ||
| uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 | ||
| env: | ||
| COMMENTER: ${{ github.event.comment.user.login }} | ||
| with: | ||
| script: | | ||
| const commenter = process.env.COMMENTER; | ||
| const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| username: commenter, | ||
| }); | ||
| core.info(`${commenter}: permission=${data.permission} role=${data.role_name}`); | ||
|
|
||
| // The coarse permission field reports the maintain role as write. | ||
| if (!['admin', 'write'].includes(data.permission)) { | ||
| core.setFailed( | ||
| `@${commenter} has '${data.role_name}' access to this repo, but ` + | ||
| 'triggering the full test suite requires write access or above.' | ||
| ); | ||
| } | ||
|
|
||
| # Resolve immutable head and merge SHAs when the command is received. | ||
| # Mergeability is computed asynchronously, so poll until it is available. | ||
| - name: Resolve PR head and merge commit | ||
| id: resolve | ||
| uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 | ||
| with: | ||
| script: | | ||
| const pull_number = context.issue.number; | ||
| let pr; | ||
| for (let attempt = 1; attempt <= 10; attempt += 1) { | ||
| ({ data: pr } = await github.rest.pulls.get({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| pull_number, | ||
| })); | ||
| if (pr.state !== 'open') { | ||
| core.setFailed(`PR #${pull_number} is ${pr.state}, not open.`); | ||
| return; | ||
| } | ||
| if (pr.mergeable !== null) { | ||
| break; | ||
| } | ||
| core.info(`Mergeability not computed yet (attempt ${attempt}/10); retrying in 3s.`); | ||
| await new Promise((resolve) => { setTimeout(resolve, 3000); }); | ||
| } | ||
|
|
||
| if (pr.mergeable === null) { | ||
| core.setFailed( | ||
| `GitHub did not finish computing mergeability for PR #${pull_number}. ` + | ||
| 'Comment /ci-run-all-tests again in a moment.' | ||
| ); | ||
| return; | ||
| } | ||
| if (pr.mergeable === false) { | ||
| core.setFailed( | ||
| `PR #${pull_number} conflicts with ${pr.base.ref}. Merge or rebase ` + | ||
| `${pr.base.ref} before running the full suite.` | ||
| ); | ||
| return; | ||
| } | ||
| if (!pr.merge_commit_sha) { | ||
| core.setFailed(`PR #${pull_number} has no test merge commit to test.`); | ||
| return; | ||
| } | ||
|
|
||
| core.info(`Testing merge commit ${pr.merge_commit_sha} (head ${pr.head.sha}).`); | ||
| core.setOutput('head_sha', pr.head.sha); | ||
| core.setOutput('merge_sha', pr.merge_commit_sha); | ||
|
|
||
| full-tests: | ||
| name: Run full test suite | ||
| needs: authorize | ||
| uses: ./.github/workflows/full-tests.yml | ||
| permissions: | ||
| contents: read | ||
| statuses: write | ||
| with: | ||
| head_sha: ${{ needs.authorize.outputs.head_sha }} | ||
| merge_sha: ${{ needs.authorize.outputs.merge_sha }} | ||
| pr_number: ${{ github.event.issue.number }} | ||
| secrets: | ||
| DISCORD_WEBHOOK: ${{ secrets.DISCORD_WEBHOOK }} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,183 @@ | ||
| name: Full test suite | ||
|
|
||
| # Runs `make test-all` against a PR merge commit and reports status on its head. | ||
| on: | ||
| workflow_call: | ||
| inputs: | ||
| head_sha: | ||
| description: "PR head SHA -- the commit the status is reported on" | ||
| required: true | ||
| type: string | ||
| merge_sha: | ||
| description: "PR test merge commit SHA -- this is what gets tested" | ||
| required: true | ||
| type: string | ||
| pr_number: | ||
| description: "PR number being tested" | ||
| required: true | ||
| type: string | ||
| secrets: | ||
| DISCORD_WEBHOOK: | ||
| required: false | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| env: | ||
| # Must match the required status context in branch protection. | ||
| STATUS_CONTEXT: "full-test-suite" | ||
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ inputs.pr_number }} | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| prepare: | ||
| name: Set pending status | ||
| runs-on: ubuntu-24.04 | ||
| timeout-minutes: 3 | ||
| permissions: | ||
| statuses: write | ||
| steps: | ||
| - uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 | ||
| env: | ||
| STATUS_SHA: ${{ inputs.head_sha }} | ||
| with: | ||
| script: | | ||
| await github.rest.repos.createCommitStatus({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| sha: process.env.STATUS_SHA, | ||
| state: 'pending', | ||
| context: process.env.STATUS_CONTEXT, | ||
| description: 'Running full test suite...', | ||
| target_url: `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`, | ||
| }); | ||
|
|
||
| full-tests: | ||
| name: All-features tests + failpoints (make test-all) | ||
| needs: prepare | ||
| runs-on: ubuntu-24.04 | ||
| timeout-minutes: 60 | ||
| # PR-authored code runs only in this job. It has no secrets or write access, | ||
| # checkout does not persist credentials, and cache writes are disabled. Jobs | ||
| # with write access never execute PR code. This isolation mitigates CodeQL's | ||
| # `actions/untrusted-checkout` finding. | ||
| permissions: | ||
| contents: read | ||
| steps: | ||
| - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| with: | ||
| ref: ${{ inputs.merge_sha }} | ||
| persist-credentials: false | ||
|
|
||
| - name: Install Ubuntu packages | ||
|
Comment on lines
+69
to
+74
|
||
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get -y install libsasl2-dev libcurl4-openssl-dev | ||
|
|
||
| # apt's protobuf-compiler is too old for proto3 optional fields required | ||
| # by the substrait crate enabled through the datafusion feature. | ||
| - name: Install protoc | ||
| uses: taiki-e/install-action@7769b73c2ec98c38dfcf2e18c83cfd4880c038c1 | ||
| with: | ||
| tool: protoc | ||
|
|
||
| - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v.6.2.0 | ||
| with: | ||
| python-version: '3.11' | ||
|
|
||
| - name: Setup stable Rust Toolchain | ||
| uses: dtolnay/rust-toolchain@3c5f7ea28cd621ae0bf5283f0e981fb97b8a7af9 # master | ||
| with: | ||
| toolchain: stable | ||
|
|
||
| # Do not save caches from PR code: `issue_comment` runs use the default- | ||
| # branch cache scope, so writes could poison trusted workflows. Reuse the | ||
| # trusted `quickwit-cargo` key read-only. | ||
| - name: Setup cache (read-only) | ||
| uses: Swatinem/rust-cache@c19371144df3bb44fab255c43d04cbc2ab54d1c4 # v2.9.1 | ||
| with: | ||
| workspaces: "./quickwit -> target" | ||
| shared-key: "quickwit-cargo" | ||
| save-if: false | ||
|
|
||
| - name: Install cargo-nextest | ||
| uses: taiki-e/install-action@7769b73c2ec98c38dfcf2e18c83cfd4880c038c1 | ||
| with: | ||
| tool: cargo-nextest | ||
|
|
||
| - name: Install python packages | ||
| run: | | ||
| pip install --user --require-hashes -r ${{ github.workspace }}/.github/workflows/requirements.txt | ||
| pipenv install --deploy --ignore-pipfile | ||
| working-directory: ./quickwit/quickwit-cli/tests | ||
|
|
||
| - name: Prepare LocalStack S3 | ||
| run: pipenv run ./prepare_tests.sh | ||
| working-directory: ./quickwit/quickwit-cli/tests | ||
|
Comment on lines
+116
to
+118
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.
On a fresh AGENTS.md reference: AGENTS.md:L142-L145 Useful? React with 👍 / 👎. |
||
|
|
||
| - name: Run full test suite | ||
| run: make test-all | ||
|
|
||
| report-status: | ||
| name: Report final status | ||
| needs: [prepare, full-tests] | ||
| if: ${{ always() && needs.prepare.result == 'success' }} | ||
| runs-on: ubuntu-24.04 | ||
| timeout-minutes: 3 | ||
| permissions: | ||
| statuses: write | ||
| steps: | ||
| - uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 | ||
| env: | ||
| TEST_SHA: ${{ inputs.merge_sha }} | ||
| STATUS_SHA: ${{ inputs.head_sha }} | ||
| RESULT: ${{ needs.full-tests.result }} | ||
| with: | ||
| script: | | ||
| const state = process.env.RESULT === 'success' ? 'success' : 'failure'; | ||
| const testSha = process.env.TEST_SHA; | ||
| const statusSha = process.env.STATUS_SHA; | ||
|
|
||
| const suffix = testSha === statusSha ? '' : ` (merge commit ${testSha.slice(0, 7)})`; | ||
|
|
||
| await github.rest.repos.createCommitStatus({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| sha: statusSha, | ||
| state, | ||
| context: process.env.STATUS_CONTEXT, | ||
| description: (state === 'success' ? 'Full test suite passed' : 'Full test suite failed') + suffix, | ||
| target_url: `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`, | ||
| }); | ||
|
|
||
| on-failure: | ||
| name: Send failure notification | ||
| needs: [prepare, full-tests] | ||
| if: >- | ||
| ${{ always() && github.repository_owner == 'quickwit-oss' && | ||
| needs.prepare.result == 'success' && needs.full-tests.result == 'failure' }} | ||
| runs-on: ubuntu-24.04 | ||
| timeout-minutes: 2 | ||
| permissions: {} | ||
| # Job-level env is required by the step condition. This job does not check | ||
| # out PR code. | ||
| env: | ||
| DISCORD_WEBHOOK: ${{ secrets.DISCORD_WEBHOOK }} | ||
| TARGET: "PR #${{ inputs.pr_number }}" | ||
| steps: | ||
| - name: Send message | ||
| if: env.DISCORD_WEBHOOK != '' | ||
| uses: sarisia/actions-status-discord@eb045afee445dc055c18d3d90bd0f244fd062708 # v1.16.0 | ||
| with: | ||
| webhook: ${{ env.DISCORD_WEBHOOK }} | ||
| nodetail: true | ||
| color: "#FF0000" | ||
| title: "" | ||
| description: | | ||
| ### ❌ ${{ env.TARGET }} | ||
|
|
||
| The full test suite (`make test-all`) failed. | ||
|
|
||
| **[View logs](https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }})** | ||
Uh oh!
There was an error while loading. Please reload this page.