From 9af5a3b8d3cf53cab653405a2be252e08d582032 Mon Sep 17 00:00:00 2001 From: st0012 Date: Tue, 1 Sep 2026 15:11:08 +0100 Subject: [PATCH 1/3] Support previews for stacked pull requests --- .github/workflows/cloudflare-preview.yml | 30 +++++++++++++++--------- .github/workflows/pr-preview-check.yml | 2 +- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/.github/workflows/cloudflare-preview.yml b/.github/workflows/cloudflare-preview.yml index d9eae21eae..ef2f9414b2 100644 --- a/.github/workflows/cloudflare-preview.yml +++ b/.github/workflows/cloudflare-preview.yml @@ -22,6 +22,7 @@ jobs: current: ${{ steps.resolve.outputs.current }} number: ${{ steps.resolve.outputs.number }} head_sha: ${{ steps.resolve.outputs.head_sha }} + base_ref: ${{ steps.resolve.outputs.base_ref }} steps: # A forked workflow_run can omit pull request details, so the artifact # must match the exact open pull request and commit. @@ -32,7 +33,6 @@ jobs: script: | const run = context.payload.workflow_run; const expectedRepository = 'ruby/rdoc'; - const expectedBase = 'master'; const artifactName = 'pr-preview-site'; const maximumArchiveBytes = 500 * 1024 * 1024; @@ -66,7 +66,6 @@ jobs: repo: context.repo.repo, state: 'open', head: `${headOwner}:${headBranch}`, - base: expectedBase, per_page: 100, }); candidateNumbers = new Set( @@ -94,7 +93,7 @@ jobs: const matches = candidates.filter(pull => pull.base.repo.full_name === expectedRepository && - pull.base.ref === expectedBase && + pull.base.ref && pull.head.repo?.full_name === headRepository && pull.head.ref === headBranch ); @@ -109,6 +108,7 @@ jobs: core.setOutput('current', current.toString()); core.setOutput('number', pull.number.toString()); core.setOutput('head_sha', run.head_sha); + core.setOutput('base_ref', pull.base.ref); if (!current) { core.notice('The pull request changed or closed after this preview build.'); @@ -254,18 +254,22 @@ jobs: core.info(`Accepted ${fileCount} static files (${totalBytes} bytes).`); # The pull request can change after artifact selection. A second head - # comparison blocks deployment of a stale commit. - - name: Confirm pull request head + # and base comparison blocks deployment with stale pull request data. + - name: Confirm pull request head and base id: current uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 env: PR_NUMBER: ${{ needs.resolve.outputs.number }} EXPECTED_SHA: ${{ needs.resolve.outputs.head_sha }} + EXPECTED_BASE_REF: ${{ needs.resolve.outputs.base_ref }} with: script: | const number = process.env.PR_NUMBER; const expectedSha = process.env.EXPECTED_SHA; - if (!/^[1-9][0-9]*$/.test(number) || !/^[0-9a-f]{40}$/.test(expectedSha)) { + const expectedBaseRef = process.env.EXPECTED_BASE_REF; + if (!/^[1-9][0-9]*$/.test(number) || + !/^[0-9a-f]{40}$/.test(expectedSha) || + !expectedBaseRef) { core.setFailed('The resolved pull request metadata is invalid.'); return; } @@ -278,7 +282,7 @@ jobs: const current = pull.state === 'open' && pull.base.repo.full_name === 'ruby/rdoc' && - pull.base.ref === 'master' && + pull.base.ref === expectedBaseRef && pull.head.sha === expectedSha; core.setOutput('current', current.toString()); @@ -308,14 +312,15 @@ jobs: --branch="${{ needs.resolve.outputs.number }}-preview" --commit-hash="${{ needs.resolve.outputs.head_sha }}" - # The workflow reuses one marked comment to avoid notification spam and - # show the exact commit for the preview. + # The workflow rechecks the pull request after deployment, then reuses + # one marked comment to avoid stale links and notification spam. - name: Update preview comment if: steps.current.outputs.current == 'true' uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 env: PR_NUMBER: ${{ needs.resolve.outputs.number }} EXPECTED_SHA: ${{ needs.resolve.outputs.head_sha }} + EXPECTED_BASE_REF: ${{ needs.resolve.outputs.base_ref }} PREVIEW_ALIAS_URL: ${{ steps.deploy.outputs.pages-deployment-alias-url }} PREVIEW_DEPLOYMENT_URL: ${{ steps.deploy.outputs.deployment-url }} with: @@ -323,11 +328,14 @@ jobs: const marker = ''; const number = process.env.PR_NUMBER; const expectedSha = process.env.EXPECTED_SHA; + const expectedBaseRef = process.env.EXPECTED_BASE_REF; const previewUrl = ( process.env.PREVIEW_ALIAS_URL || process.env.PREVIEW_DEPLOYMENT_URL || '' ).trim(); - if (!/^[1-9][0-9]*$/.test(number) || !/^[0-9a-f]{40}$/.test(expectedSha)) { + if (!/^[1-9][0-9]*$/.test(number) || + !/^[0-9a-f]{40}$/.test(expectedSha) || + !expectedBaseRef) { core.setFailed('The preview comment metadata is invalid.'); return; } @@ -353,7 +361,7 @@ jobs: const current = pull.state === 'open' && pull.base.repo.full_name === 'ruby/rdoc' && - pull.base.ref === 'master' && + pull.base.ref === expectedBaseRef && pull.head.sha === expectedSha; if (!current) { core.notice('Skipped the preview comment because the pull request changed.'); diff --git a/.github/workflows/pr-preview-check.yml b/.github/workflows/pr-preview-check.yml index 8ce96d0887..2a5b620f8b 100644 --- a/.github/workflows/pr-preview-check.yml +++ b/.github/workflows/pr-preview-check.yml @@ -2,7 +2,7 @@ name: Build PR Preview on: pull_request: - types: [opened, synchronize, reopened] + types: [opened, synchronize, reopened, edited] permissions: contents: read From de8eaca878b1457b1bb072667a37e0547e0d59e6 Mon Sep 17 00:00:00 2001 From: st0012 Date: Sat, 5 Sep 2026 11:20:23 +0100 Subject: [PATCH 2/3] Preserve previews for shared-head pull requests --- .github/tests/pr-preview.test.cjs | 190 +++++++++++++++++++++++ .github/workflows/cloudflare-preview.yml | 86 +++++----- .github/workflows/lint.yml | 2 + .github/workflows/pr-preview-check.yml | 13 +- 4 files changed, 250 insertions(+), 41 deletions(-) create mode 100644 .github/tests/pr-preview.test.cjs diff --git a/.github/tests/pr-preview.test.cjs b/.github/tests/pr-preview.test.cjs new file mode 100644 index 0000000000..0e59ef929c --- /dev/null +++ b/.github/tests/pr-preview.test.cjs @@ -0,0 +1,190 @@ +const assert = require('node:assert/strict'); +const { execFileSync } = require('node:child_process'); +const path = require('node:path'); +const test = require('node:test'); + +// Use the workflow scripts themselves, not a second implementation of the resolver. +const workflows = JSON.parse(execFileSync('ruby', ['-ryaml', '-rjson', '-e', + 'puts JSON.generate(ARGV.map { |file| YAML.load_file(file) })', + '.github/workflows/cloudflare-preview.yml', '.github/workflows/pr-preview-check.yml', +], { cwd: path.resolve(__dirname, '../..'), encoding: 'utf8' })); +const [deployment, build] = workflows; +const resolveScript = deployment.jobs.resolve.steps[0].with.script; +const currentStep = deployment.jobs.deploy.steps.find(step => step.id === 'current'); +const commentStep = deployment.jobs.deploy.steps.find(step => step.name === 'Update preview comment'); +const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor; +const sha = 'a'.repeat(40); +const artifact = { name: 'pr-preview-site', expired: false, size_in_bytes: 1000 }; + +function pull(number = 1, base = 'master', branch = 'child') { + return { + number, state: 'open', + base: { ref: base, repo: { full_name: 'ruby/rdoc' } }, + head: { sha, ref: branch, repo: { full_name: 'contributor/rdoc' } }, + }; +} + +async function execute(script, { + pulls = [pull()], associated = [], conclusion = 'success', artifacts = [artifact], + jobs = [{ conclusion: 'success' }], env = {}, comments = [], +} = {}) { + const outputs = {}, failures = [], writes = [], calls = []; + const rest = { + pulls: { + list: () => {}, + get: async ({ pull_number }) => ({ data: pulls.find(pr => pr.number === pull_number) }), + }, + actions: { listWorkflowRunArtifacts: () => {}, listJobsForWorkflowRun: () => {} }, + issues: { + listComments: () => {}, + createComment: async args => writes.push({ action: 'create', ...args }), + updateComment: async args => writes.push({ action: 'update', ...args }), + }, + }; + const github = { rest, paginate: async (method, args) => { + calls.push(args); + if (method === rest.pulls.list) { + return pulls.filter(pr => pr.state === args.state && + `contributor:${pr.head.ref}` === args.head && (!args.base || pr.base.ref === args.base)); + } + if (method === rest.actions.listWorkflowRunArtifacts) return artifacts; + if (method === rest.actions.listJobsForWorkflowRun) return jobs; + if (method === rest.issues.listComments) return comments; + throw new Error('Unexpected API call'); + } }; + const context = { repo: { owner: 'ruby', repo: 'rdoc' }, payload: { workflow_run: { + id: 123, conclusion, head_sha: sha, head_branch: 'child', + head_repository: { full_name: 'contributor/rdoc', owner: { login: 'contributor' } }, + pull_requests: associated.map(number => ({ number })), + } } }; + await new AsyncFunction('context', 'github', 'core', 'process', script)( + context, github, { + setOutput: (key, value) => { outputs[key] = value; }, + setFailed: message => failures.push(message), notice: () => {}, + }, { env }, + ); + return { outputs, failures, writes, calls }; +} + +for (const associated of [[], [1, 2], [2]]) { + test(`same head supports each associated PR: ${JSON.stringify(associated)}`, async () => { + const result = await execute(resolveScript, { pulls: [pull(), pull(2, 'parent')], associated }); + assert.deepEqual(result.failures, []); + const numbers = associated.length ? associated : [1, 2]; + assert.deepEqual(JSON.parse(result.outputs.pull_requests), numbers.map(number => ({ + number, head_sha: sha, base_ref: number === 1 ? 'master' : 'parent', + }))); + assert.ok(result.calls.some(call => call.run_id === 123 && call.name === artifact.name)); + }); +} + +test('ordinary stacked PR resolves independently of its parent', async () => { + const result = await execute(resolveScript, { pulls: [pull(1, 'master', 'parent'), pull(2, 'parent')] }); + assert.deepEqual(JSON.parse(result.outputs.pull_requests), [{ number: 2, head_sha: sha, base_ref: 'parent' }]); +}); + +for (const change of ['closed', 'stale', 'head repository', 'head branch', 'base repository']) { + test(`a ${change} candidate does not block another current PR`, async () => { + const invalid = pull(2, 'parent'); + if (change === 'closed') invalid.state = 'closed'; + if (change === 'stale') invalid.head.sha = 'b'.repeat(40); + if (change === 'head repository') invalid.head.repo.full_name = 'other/rdoc'; + if (change === 'head branch') invalid.head.ref = 'other'; + if (change === 'base repository') invalid.base.repo.full_name = 'other/rdoc'; + const result = await execute(resolveScript, { pulls: [pull(), invalid], associated: [1, 2] }); + assert.deepEqual(result.failures, []); + assert.deepEqual(JSON.parse(result.outputs.pull_requests), [{ number: 1, head_sha: sha, base_ref: 'master' }]); + }); +} + +test('no current PR produces no deployment targets', async () => { + const result = await execute(resolveScript, { pulls: [] }); + assert.deepEqual(result.failures, []); + assert.equal(result.outputs.pull_requests, '[]'); +}); + +for (const conclusion of ['success', 'failure']) { + test(`${conclusion} with a valid artifact produces a preview`, async () => { + const result = await execute(resolveScript, { conclusion }); + assert.deepEqual(result.failures, []); + assert.equal(JSON.parse(result.outputs.pull_requests).length, 1); + }); +} + +for (const [name, options, errors] of [ + ['intentional skip', { artifacts: [], jobs: [{ conclusion: 'skipped' }] }, 0], + ['failed build', { artifacts: [], conclusion: 'failure' }, 0], + ['cancelled build', { conclusion: 'cancelled' }, 0], + ['missing upload', { artifacts: [] }, 1], + ['missing jobs and upload', { artifacts: [], jobs: [] }, 1], + ['expired upload', { artifacts: [{ ...artifact, expired: true }] }, 1], + ['duplicate upload', { artifacts: [artifact, artifact] }, 1], + ['oversized upload', { artifacts: [{ ...artifact, size_in_bytes: 501 * 1024 * 1024 }] }, 1], +]) { + test(`${name} cannot deploy`, async () => { + const result = await execute(resolveScript, options); + assert.equal(result.failures.length, errors); + assert.equal(result.outputs.pull_requests, '[]'); + }); +} + +for (const change of ['base', 'head', 'state']) { + test(`a changed ${change} only skips that PR's deployment and comment`, async () => { + const pulls = [pull(), pull(2, 'parent')]; + const resolved = await execute(resolveScript, { pulls }); + if (change === 'base') pulls[1].base.ref = 'retargeted'; + if (change === 'head') pulls[1].head.sha = 'b'.repeat(40); + if (change === 'state') pulls[1].state = 'closed'; + for (const target of JSON.parse(resolved.outputs.pull_requests)) { + const env = { + PR_NUMBER: String(target.number), EXPECTED_SHA: target.head_sha, EXPECTED_BASE_REF: target.base_ref, + PREVIEW_ALIAS_URL: `https://${target.number}-preview.rdoc-6cd.pages.dev/`, + }; + const current = await execute(currentStep.with.script, { pulls, env }); + assert.equal(current.outputs.current, String(target.number === 1)); + const result = await execute(commentStep.with.script, { pulls, env }); + assert.equal(result.writes.length, target.number === 1 ? 1 : 0); + if (target.number === 1) assert.equal(result.writes[0].issue_number, 1); + } + }); +} + +test('preview updates reuse the marked bot comment', async () => { + const result = await execute(commentStep.with.script, { + env: { PR_NUMBER: '1', EXPECTED_SHA: sha, EXPECTED_BASE_REF: 'master', PREVIEW_ALIAS_URL: 'https://1-preview.rdoc-6cd.pages.dev/' }, + comments: [{ id: 55, user: { login: 'github-actions[bot]' }, body: '\nold' }], + }); + assert.equal(result.writes[0].action, 'update'); + assert.equal(result.writes[0].comment_id, 55); +}); + +test('only source changes and base retargets run the build', () => { + const allowed = new Function('github', `return Boolean(${build.jobs.build.if});`); + for (const action of ['opened', 'synchronize', 'reopened', 'edited']) { + for (const field of ['title', 'body', 'base']) { + const github = { repository: 'ruby/rdoc', event: { action, changes: { [field]: {} } } }; + assert.equal(allowed(github), action !== 'edited' || field === 'base', `${action}: ${field}`); + } + } + // Job-scoped concurrency prevents skipped metadata edits from cancelling an active build. + assert.equal(build.concurrency, undefined); + assert.equal(build.jobs.build.concurrency.group, 'pr-preview-build-${{ github.event.pull_request.number }}'); + assert.equal(build.jobs.build.concurrency['cancel-in-progress'], true); +}); + +test('matrix deployments use separate PR aliases and cancellation groups', () => { + const job = deployment.jobs.deploy; + const allowed = new Function('needs', `return Boolean(${job.if});`); + assert.equal(allowed({ resolve: { outputs: { pull_requests: '[]' } } }), false); + assert.equal(allowed({ resolve: { outputs: { pull_requests: '[{"number":1}]' } } }), true); + assert.equal(job.strategy['fail-fast'], false); + assert.equal(job.strategy.matrix.pull_request, '${{ fromJSON(needs.resolve.outputs.pull_requests) }}'); + assert.equal(job.concurrency.group, 'pr-preview-deploy-${{ matrix.pull_request.number }}'); + for (const step of [currentStep, commentStep]) { + assert.equal(step.env.PR_NUMBER, '${{ matrix.pull_request.number }}'); + assert.equal(step.env.EXPECTED_SHA, '${{ matrix.pull_request.head_sha }}'); + assert.equal(step.env.EXPECTED_BASE_REF, '${{ matrix.pull_request.base_ref }}'); + } + assert.match(job.steps.find(step => step.id === 'deploy').with.command, + /--branch="\$\{\{ matrix\.pull_request\.number \}\}-preview"/); +}); diff --git a/.github/workflows/cloudflare-preview.yml b/.github/workflows/cloudflare-preview.yml index ef2f9414b2..3085d542d8 100644 --- a/.github/workflows/cloudflare-preview.yml +++ b/.github/workflows/cloudflare-preview.yml @@ -19,14 +19,11 @@ jobs: actions: read pull-requests: read outputs: - current: ${{ steps.resolve.outputs.current }} - number: ${{ steps.resolve.outputs.number }} - head_sha: ${{ steps.resolve.outputs.head_sha }} - base_ref: ${{ steps.resolve.outputs.base_ref }} + pull_requests: ${{ steps.resolve.outputs.pull_requests }} steps: # A forked workflow_run can omit pull request details, so the artifact - # must match the exact open pull request and commit. - - name: Resolve current pull request and artifact + # must match the source repository, branch, and commit of each open PR. + - name: Resolve current pull requests and artifact id: resolve uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: @@ -35,9 +32,9 @@ jobs: const expectedRepository = 'ruby/rdoc'; const artifactName = 'pr-preview-site'; const maximumArchiveBytes = 500 * 1024 * 1024; + core.setOutput('pull_requests', '[]'); if (!['success', 'failure'].includes(run.conclusion)) { - core.setOutput('current', 'false'); core.notice(`Skipped a preview build with conclusion: ${run.conclusion}.`); return; } @@ -76,7 +73,6 @@ jobs: } if (candidateNumbers.size === 0) { - core.setOutput('current', 'false'); core.notice('No open pull request uses this preview build.'); return; } @@ -92,26 +88,15 @@ jobs: } const matches = candidates.filter(pull => + pull.state === 'open' && pull.base.repo.full_name === expectedRepository && - pull.base.ref && pull.head.repo?.full_name === headRepository && - pull.head.ref === headBranch + pull.head.ref === headBranch && + pull.head.sha === run.head_sha ); - if (matches.length !== 1) { - core.setFailed(`Expected one pull request for this preview build, found ${matches.length}.`); - return; - } - - const pull = matches[0]; - const current = pull.state === 'open' && pull.head.sha === run.head_sha; - core.setOutput('current', current.toString()); - core.setOutput('number', pull.number.toString()); - core.setOutput('head_sha', run.head_sha); - core.setOutput('base_ref', pull.base.ref); - - if (!current) { - core.notice('The pull request changed or closed after this preview build.'); + if (matches.length === 0) { + core.notice('No current open pull request uses this preview build.'); return; } @@ -130,11 +115,26 @@ jobs: ); if (matchingArtifacts.length === 0 && run.conclusion === 'failure') { - core.setOutput('current', 'false'); core.notice('The failed preview build did not publish an artifact.'); return; } + // Title and body edits skip the build job but can still complete successfully. + // Keep missing uploads an error when a build actually ran. + if (matchingArtifacts.length === 0) { + const jobs = await github.paginate(github.rest.actions.listJobsForWorkflowRun, { + owner: context.repo.owner, + repo: context.repo.repo, + run_id: run.id, + filter: 'latest', + per_page: 100, + }); + if (jobs.length > 0 && jobs.every(job => job.conclusion === 'skipped')) { + core.notice('The preview build was skipped.'); + return; + } + } + if (matchingArtifacts.length !== 1) { core.setFailed(`Expected one ${artifactName} artifact, found ${matchingArtifacts.length}.`); return; @@ -143,16 +143,30 @@ jobs: const artifact = matchingArtifacts[0]; if (artifact.size_in_bytes <= 0 || artifact.size_in_bytes > maximumArchiveBytes) { core.setFailed(`The preview artifact archive has an invalid size: ${artifact.size_in_bytes} bytes.`); + return; } + // The site comes from the head commit, not a merge with the base branch. + // PRs with the same source repository, branch, and commit can share it. + core.setOutput('pull_requests', JSON.stringify(matches.map(pull => ({ + number: pull.number, + head_sha: run.head_sha, + base_ref: pull.base.ref, + })))); + deploy: - name: Deploy Preview + name: Deploy Preview for PR ${{ matrix.pull_request.number }} needs: resolve - if: needs.resolve.outputs.current == 'true' + if: needs.resolve.outputs.pull_requests != '[]' runs-on: ubuntu-latest timeout-minutes: 15 + # Each PR keeps its own preview and stale-data checks, even when the source is shared. + strategy: + fail-fast: false + matrix: + pull_request: ${{ fromJSON(needs.resolve.outputs.pull_requests) }} concurrency: - group: pr-preview-deploy-${{ needs.resolve.outputs.number }} + group: pr-preview-deploy-${{ matrix.pull_request.number }} cancel-in-progress: true permissions: actions: read @@ -259,9 +273,9 @@ jobs: id: current uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 env: - PR_NUMBER: ${{ needs.resolve.outputs.number }} - EXPECTED_SHA: ${{ needs.resolve.outputs.head_sha }} - EXPECTED_BASE_REF: ${{ needs.resolve.outputs.base_ref }} + PR_NUMBER: ${{ matrix.pull_request.number }} + EXPECTED_SHA: ${{ matrix.pull_request.head_sha }} + EXPECTED_BASE_REF: ${{ matrix.pull_request.base_ref }} with: script: | const number = process.env.PR_NUMBER; @@ -309,8 +323,8 @@ jobs: command: >- pages deploy "${{ runner.temp }}/pr-preview-site" --project-name=rdoc - --branch="${{ needs.resolve.outputs.number }}-preview" - --commit-hash="${{ needs.resolve.outputs.head_sha }}" + --branch="${{ matrix.pull_request.number }}-preview" + --commit-hash="${{ matrix.pull_request.head_sha }}" # The workflow rechecks the pull request after deployment, then reuses # one marked comment to avoid stale links and notification spam. @@ -318,9 +332,9 @@ jobs: if: steps.current.outputs.current == 'true' uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 env: - PR_NUMBER: ${{ needs.resolve.outputs.number }} - EXPECTED_SHA: ${{ needs.resolve.outputs.head_sha }} - EXPECTED_BASE_REF: ${{ needs.resolve.outputs.base_ref }} + PR_NUMBER: ${{ matrix.pull_request.number }} + EXPECTED_SHA: ${{ matrix.pull_request.head_sha }} + EXPECTED_BASE_REF: ${{ matrix.pull_request.base_ref }} PREVIEW_ALIAS_URL: ${{ steps.deploy.outputs.pages-deployment-alias-url }} PREVIEW_DEPLOYMENT_URL: ${{ steps.deploy.outputs.deployment-url }} with: diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 1855731491..565d76e876 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -27,6 +27,8 @@ jobs: bundler-cache: true - name: Run rubocop run: bundle exec rubocop + - name: Test preview workflows + run: node --test .github/tests/pr-preview.test.cjs - name: Run herb linter # npx should be available by default so we don't need `setup-node` action # Don't lint erb files outside of lib as the current directory also includes vendor/bundle diff --git a/.github/workflows/pr-preview-check.yml b/.github/workflows/pr-preview-check.yml index 2a5b620f8b..1efa6f6c95 100644 --- a/.github/workflows/pr-preview-check.yml +++ b/.github/workflows/pr-preview-check.yml @@ -7,16 +7,19 @@ on: permissions: contents: read -concurrency: - group: pr-preview-build-${{ github.event.pull_request.number }} - cancel-in-progress: true - jobs: build: name: Build Preview - if: github.repository == 'ruby/rdoc' + # Base retargets need a preview, but title and body edits do not change the site. + if: >- + github.repository == 'ruby/rdoc' && + (github.event.action != 'edited' || github.event.changes.base) runs-on: ubuntu-latest timeout-minutes: 20 + # A skipped edit must not cancel an active build for the same PR. + concurrency: + group: pr-preview-build-${{ github.event.pull_request.number }} + cancel-in-progress: true steps: # This job executes untrusted pull request code. It must not receive secrets # or a token with write access. From 90bd79b62e9396ece40c6b1011df67959c7c25a2 Mon Sep 17 00:00:00 2001 From: st0012 Date: Sat, 5 Sep 2026 12:07:41 +0100 Subject: [PATCH 3/3] Remove preview workflow tests --- .github/tests/pr-preview.test.cjs | 190 ------------------------------ .github/workflows/lint.yml | 2 - 2 files changed, 192 deletions(-) delete mode 100644 .github/tests/pr-preview.test.cjs diff --git a/.github/tests/pr-preview.test.cjs b/.github/tests/pr-preview.test.cjs deleted file mode 100644 index 0e59ef929c..0000000000 --- a/.github/tests/pr-preview.test.cjs +++ /dev/null @@ -1,190 +0,0 @@ -const assert = require('node:assert/strict'); -const { execFileSync } = require('node:child_process'); -const path = require('node:path'); -const test = require('node:test'); - -// Use the workflow scripts themselves, not a second implementation of the resolver. -const workflows = JSON.parse(execFileSync('ruby', ['-ryaml', '-rjson', '-e', - 'puts JSON.generate(ARGV.map { |file| YAML.load_file(file) })', - '.github/workflows/cloudflare-preview.yml', '.github/workflows/pr-preview-check.yml', -], { cwd: path.resolve(__dirname, '../..'), encoding: 'utf8' })); -const [deployment, build] = workflows; -const resolveScript = deployment.jobs.resolve.steps[0].with.script; -const currentStep = deployment.jobs.deploy.steps.find(step => step.id === 'current'); -const commentStep = deployment.jobs.deploy.steps.find(step => step.name === 'Update preview comment'); -const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor; -const sha = 'a'.repeat(40); -const artifact = { name: 'pr-preview-site', expired: false, size_in_bytes: 1000 }; - -function pull(number = 1, base = 'master', branch = 'child') { - return { - number, state: 'open', - base: { ref: base, repo: { full_name: 'ruby/rdoc' } }, - head: { sha, ref: branch, repo: { full_name: 'contributor/rdoc' } }, - }; -} - -async function execute(script, { - pulls = [pull()], associated = [], conclusion = 'success', artifacts = [artifact], - jobs = [{ conclusion: 'success' }], env = {}, comments = [], -} = {}) { - const outputs = {}, failures = [], writes = [], calls = []; - const rest = { - pulls: { - list: () => {}, - get: async ({ pull_number }) => ({ data: pulls.find(pr => pr.number === pull_number) }), - }, - actions: { listWorkflowRunArtifacts: () => {}, listJobsForWorkflowRun: () => {} }, - issues: { - listComments: () => {}, - createComment: async args => writes.push({ action: 'create', ...args }), - updateComment: async args => writes.push({ action: 'update', ...args }), - }, - }; - const github = { rest, paginate: async (method, args) => { - calls.push(args); - if (method === rest.pulls.list) { - return pulls.filter(pr => pr.state === args.state && - `contributor:${pr.head.ref}` === args.head && (!args.base || pr.base.ref === args.base)); - } - if (method === rest.actions.listWorkflowRunArtifacts) return artifacts; - if (method === rest.actions.listJobsForWorkflowRun) return jobs; - if (method === rest.issues.listComments) return comments; - throw new Error('Unexpected API call'); - } }; - const context = { repo: { owner: 'ruby', repo: 'rdoc' }, payload: { workflow_run: { - id: 123, conclusion, head_sha: sha, head_branch: 'child', - head_repository: { full_name: 'contributor/rdoc', owner: { login: 'contributor' } }, - pull_requests: associated.map(number => ({ number })), - } } }; - await new AsyncFunction('context', 'github', 'core', 'process', script)( - context, github, { - setOutput: (key, value) => { outputs[key] = value; }, - setFailed: message => failures.push(message), notice: () => {}, - }, { env }, - ); - return { outputs, failures, writes, calls }; -} - -for (const associated of [[], [1, 2], [2]]) { - test(`same head supports each associated PR: ${JSON.stringify(associated)}`, async () => { - const result = await execute(resolveScript, { pulls: [pull(), pull(2, 'parent')], associated }); - assert.deepEqual(result.failures, []); - const numbers = associated.length ? associated : [1, 2]; - assert.deepEqual(JSON.parse(result.outputs.pull_requests), numbers.map(number => ({ - number, head_sha: sha, base_ref: number === 1 ? 'master' : 'parent', - }))); - assert.ok(result.calls.some(call => call.run_id === 123 && call.name === artifact.name)); - }); -} - -test('ordinary stacked PR resolves independently of its parent', async () => { - const result = await execute(resolveScript, { pulls: [pull(1, 'master', 'parent'), pull(2, 'parent')] }); - assert.deepEqual(JSON.parse(result.outputs.pull_requests), [{ number: 2, head_sha: sha, base_ref: 'parent' }]); -}); - -for (const change of ['closed', 'stale', 'head repository', 'head branch', 'base repository']) { - test(`a ${change} candidate does not block another current PR`, async () => { - const invalid = pull(2, 'parent'); - if (change === 'closed') invalid.state = 'closed'; - if (change === 'stale') invalid.head.sha = 'b'.repeat(40); - if (change === 'head repository') invalid.head.repo.full_name = 'other/rdoc'; - if (change === 'head branch') invalid.head.ref = 'other'; - if (change === 'base repository') invalid.base.repo.full_name = 'other/rdoc'; - const result = await execute(resolveScript, { pulls: [pull(), invalid], associated: [1, 2] }); - assert.deepEqual(result.failures, []); - assert.deepEqual(JSON.parse(result.outputs.pull_requests), [{ number: 1, head_sha: sha, base_ref: 'master' }]); - }); -} - -test('no current PR produces no deployment targets', async () => { - const result = await execute(resolveScript, { pulls: [] }); - assert.deepEqual(result.failures, []); - assert.equal(result.outputs.pull_requests, '[]'); -}); - -for (const conclusion of ['success', 'failure']) { - test(`${conclusion} with a valid artifact produces a preview`, async () => { - const result = await execute(resolveScript, { conclusion }); - assert.deepEqual(result.failures, []); - assert.equal(JSON.parse(result.outputs.pull_requests).length, 1); - }); -} - -for (const [name, options, errors] of [ - ['intentional skip', { artifacts: [], jobs: [{ conclusion: 'skipped' }] }, 0], - ['failed build', { artifacts: [], conclusion: 'failure' }, 0], - ['cancelled build', { conclusion: 'cancelled' }, 0], - ['missing upload', { artifacts: [] }, 1], - ['missing jobs and upload', { artifacts: [], jobs: [] }, 1], - ['expired upload', { artifacts: [{ ...artifact, expired: true }] }, 1], - ['duplicate upload', { artifacts: [artifact, artifact] }, 1], - ['oversized upload', { artifacts: [{ ...artifact, size_in_bytes: 501 * 1024 * 1024 }] }, 1], -]) { - test(`${name} cannot deploy`, async () => { - const result = await execute(resolveScript, options); - assert.equal(result.failures.length, errors); - assert.equal(result.outputs.pull_requests, '[]'); - }); -} - -for (const change of ['base', 'head', 'state']) { - test(`a changed ${change} only skips that PR's deployment and comment`, async () => { - const pulls = [pull(), pull(2, 'parent')]; - const resolved = await execute(resolveScript, { pulls }); - if (change === 'base') pulls[1].base.ref = 'retargeted'; - if (change === 'head') pulls[1].head.sha = 'b'.repeat(40); - if (change === 'state') pulls[1].state = 'closed'; - for (const target of JSON.parse(resolved.outputs.pull_requests)) { - const env = { - PR_NUMBER: String(target.number), EXPECTED_SHA: target.head_sha, EXPECTED_BASE_REF: target.base_ref, - PREVIEW_ALIAS_URL: `https://${target.number}-preview.rdoc-6cd.pages.dev/`, - }; - const current = await execute(currentStep.with.script, { pulls, env }); - assert.equal(current.outputs.current, String(target.number === 1)); - const result = await execute(commentStep.with.script, { pulls, env }); - assert.equal(result.writes.length, target.number === 1 ? 1 : 0); - if (target.number === 1) assert.equal(result.writes[0].issue_number, 1); - } - }); -} - -test('preview updates reuse the marked bot comment', async () => { - const result = await execute(commentStep.with.script, { - env: { PR_NUMBER: '1', EXPECTED_SHA: sha, EXPECTED_BASE_REF: 'master', PREVIEW_ALIAS_URL: 'https://1-preview.rdoc-6cd.pages.dev/' }, - comments: [{ id: 55, user: { login: 'github-actions[bot]' }, body: '\nold' }], - }); - assert.equal(result.writes[0].action, 'update'); - assert.equal(result.writes[0].comment_id, 55); -}); - -test('only source changes and base retargets run the build', () => { - const allowed = new Function('github', `return Boolean(${build.jobs.build.if});`); - for (const action of ['opened', 'synchronize', 'reopened', 'edited']) { - for (const field of ['title', 'body', 'base']) { - const github = { repository: 'ruby/rdoc', event: { action, changes: { [field]: {} } } }; - assert.equal(allowed(github), action !== 'edited' || field === 'base', `${action}: ${field}`); - } - } - // Job-scoped concurrency prevents skipped metadata edits from cancelling an active build. - assert.equal(build.concurrency, undefined); - assert.equal(build.jobs.build.concurrency.group, 'pr-preview-build-${{ github.event.pull_request.number }}'); - assert.equal(build.jobs.build.concurrency['cancel-in-progress'], true); -}); - -test('matrix deployments use separate PR aliases and cancellation groups', () => { - const job = deployment.jobs.deploy; - const allowed = new Function('needs', `return Boolean(${job.if});`); - assert.equal(allowed({ resolve: { outputs: { pull_requests: '[]' } } }), false); - assert.equal(allowed({ resolve: { outputs: { pull_requests: '[{"number":1}]' } } }), true); - assert.equal(job.strategy['fail-fast'], false); - assert.equal(job.strategy.matrix.pull_request, '${{ fromJSON(needs.resolve.outputs.pull_requests) }}'); - assert.equal(job.concurrency.group, 'pr-preview-deploy-${{ matrix.pull_request.number }}'); - for (const step of [currentStep, commentStep]) { - assert.equal(step.env.PR_NUMBER, '${{ matrix.pull_request.number }}'); - assert.equal(step.env.EXPECTED_SHA, '${{ matrix.pull_request.head_sha }}'); - assert.equal(step.env.EXPECTED_BASE_REF, '${{ matrix.pull_request.base_ref }}'); - } - assert.match(job.steps.find(step => step.id === 'deploy').with.command, - /--branch="\$\{\{ matrix\.pull_request\.number \}\}-preview"/); -}); diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 565d76e876..1855731491 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -27,8 +27,6 @@ jobs: bundler-cache: true - name: Run rubocop run: bundle exec rubocop - - name: Test preview workflows - run: node --test .github/tests/pr-preview.test.cjs - name: Run herb linter # npx should be available by default so we don't need `setup-node` action # Don't lint erb files outside of lib as the current directory also includes vendor/bundle