diff --git a/service/src/app.ts b/service/src/app.ts index 6b338e9..47f0ebc 100644 --- a/service/src/app.ts +++ b/service/src/app.ts @@ -40,6 +40,7 @@ export function createApp(deps: AppDeps): Express { logger: deps.logger, pullRequestOptions: { coverageServiceEnabled: deps.cfg.coverageServiceEnabled, + coverageServiceBranchPrefix: deps.cfg.coverageServiceBranchPrefix, }, }, deps.logger, diff --git a/service/src/dispatch/coverage/summary.test.ts b/service/src/dispatch/coverage/summary.test.ts index 82fddd0..aac82b1 100644 --- a/service/src/dispatch/coverage/summary.test.ts +++ b/service/src/dispatch/coverage/summary.test.ts @@ -124,4 +124,9 @@ describe('buildTestPRBody', () => { expect(out).toContain('✅'); expect(out).toContain('—'); }); + + it('includes the openreview: skip marker so webhooks ignore this PR', () => { + const out = buildTestPRBody({ run: baseRun, headRef: 'feat/foo', testPrFileCount: 0 }); + expect(out).toContain(''); + }); }); diff --git a/service/src/dispatch/coverage/summary.ts b/service/src/dispatch/coverage/summary.ts index 2edd091..beda4e5 100644 --- a/service/src/dispatch/coverage/summary.ts +++ b/service/src/dispatch/coverage/summary.ts @@ -1,3 +1,5 @@ +import { OPENREVIEW_SKIP_MARKER } from '../../github/stacked-test-pr.js'; + import type { PrRun } from './types.js'; /** @@ -101,6 +103,7 @@ export function buildTestPRBody(input: SummaryInput & { headRef: string }): stri '', '> Merge this PR *into the feature branch* (not into the default branch).', '', + ``, '', ); return lines.join('\n'); diff --git a/service/src/github/stacked-test-pr.test.ts b/service/src/github/stacked-test-pr.test.ts new file mode 100644 index 0000000..7436bcd --- /dev/null +++ b/service/src/github/stacked-test-pr.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from 'vitest'; + +import { + isStackedTestBranch, + OPENREVIEW_SKIP_MARKER, +} from './stacked-test-pr.js'; + +describe('isStackedTestBranch', () => { + it('matches the default openreview/tests/pr-N pattern', () => { + expect(isStackedTestBranch('openreview/tests/pr-6')).toBe(true); + expect(isStackedTestBranch('openreview/tests/pr-495')).toBe(true); + }); + + it('rejects feature branches and partial matches', () => { + expect(isStackedTestBranch('feature/foo')).toBe(false); + expect(isStackedTestBranch('openreview/tests/pr-6-extra')).toBe(false); + expect(isStackedTestBranch('openreview/tests')).toBe(false); + }); + + it('respects a custom branch prefix', () => { + expect(isStackedTestBranch('custom/tests/pr-1', 'custom/tests')).toBe(true); + expect(isStackedTestBranch('openreview/tests/pr-1', 'custom/tests')).toBe( + false, + ); + }); +}); + +describe('OPENREVIEW_SKIP_MARKER', () => { + it('is the documented skip string', () => { + expect(OPENREVIEW_SKIP_MARKER).toBe('openreview: skip'); + }); +}); diff --git a/service/src/github/stacked-test-pr.ts b/service/src/github/stacked-test-pr.ts new file mode 100644 index 0000000..497ca63 --- /dev/null +++ b/service/src/github/stacked-test-pr.ts @@ -0,0 +1,17 @@ +/** Marker in a PR body that tells OpenReview to skip review/coverage. */ +export const OPENREVIEW_SKIP_MARKER = 'openreview: skip'; + +const DEFAULT_BRANCH_PREFIX = 'openreview/tests'; + +/** + * True when `headRef` is an OpenReview stacked test branch + * (e.g. `openreview/tests/pr-42`). + */ +export function isStackedTestBranch( + headRef: string, + branchPrefix = DEFAULT_BRANCH_PREFIX, +): boolean { + const prefix = branchPrefix.replace(/\/+$/, ''); + const escaped = prefix.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + return new RegExp(`^${escaped}/pr-\\d+$`).test(headRef); +} diff --git a/service/src/jobs/processors/coverage-analysis.ts b/service/src/jobs/processors/coverage-analysis.ts index ea3f02a..833fe23 100644 --- a/service/src/jobs/processors/coverage-analysis.ts +++ b/service/src/jobs/processors/coverage-analysis.ts @@ -229,7 +229,7 @@ async function openStackedTestPR(args: { const opened = await author.openOrUpdatePR({ base: job.headRef, head: branch, - title: `[OpenReview] Tests for PR #${job.prNumber}: ${job.title}`, + title: `Unit Test for PR #${job.prNumber}: ${job.title}`, body: buildTestPRBody({ run, headRef: job.headRef, diff --git a/service/src/webhook/handlers/pull-request.test.ts b/service/src/webhook/handlers/pull-request.test.ts index 4ed7baf..b3214b3 100644 --- a/service/src/webhook/handlers/pull-request.test.ts +++ b/service/src/webhook/handlers/pull-request.test.ts @@ -86,6 +86,21 @@ describe('handlePullRequest', () => { expect(d.enqueued).toHaveLength(0); }); + it('ignores OpenReview stacked test PRs (no inline review comments)', async () => { + const d = deps({ coverageServiceEnabled: true }); + const p = payload('opened', { + title: '[OpenReview] Tests for PR #6: feat', + head: { sha: 'cccc3333', ref: 'openreview/tests/pr-6' }, + }); + const result = await handlePullRequest('d5b', p, d); + expect(result).toEqual({ + status: 'ignored', + reason: 'OpenReview stacked test PR (auto-generated tests)', + }); + expect(d.enqueued).toHaveLength(0); + expect(d.forwarded).toHaveLength(0); + }); + it('forwards a normalized payload to the downstream dispatcher', async () => { const d = deps(); await handlePullRequest('d6', payload('opened'), d); diff --git a/service/src/webhook/handlers/pull-request.ts b/service/src/webhook/handlers/pull-request.ts index 0e41460..3b41682 100644 --- a/service/src/webhook/handlers/pull-request.ts +++ b/service/src/webhook/handlers/pull-request.ts @@ -1,4 +1,8 @@ import type { DownstreamDispatcher } from '../../dispatch/downstream.js'; +import { + isStackedTestBranch, + OPENREVIEW_SKIP_MARKER, +} from '../../github/stacked-test-pr.js'; import type { ReviewQueue } from '../../jobs/queue.js'; import type { Logger } from '../../logger.js'; @@ -11,11 +15,11 @@ const VALID_ACTIONS = new Set([ 'ready_for_review', ]); -const SKIP_MARKER = 'openreview: skip'; - export interface PullRequestHandlerOptions { /** Toggle from config — when false, no coverage-analysis job is enqueued. */ coverageServiceEnabled: boolean; + /** Branch prefix for stacked test PRs — used to skip review on those PRs. */ + coverageServiceBranchPrefix?: string; } /** @@ -45,8 +49,19 @@ export async function handlePullRequest( return { status: 'ignored', reason: 'draft PR' }; } - if (typeof pr.body === 'string' && pr.body.includes(SKIP_MARKER)) { - return { status: 'ignored', reason: `PR body contains "${SKIP_MARKER}"` }; + if (typeof pr.body === 'string' && pr.body.includes(OPENREVIEW_SKIP_MARKER)) { + return { + status: 'ignored', + reason: `PR body contains "${OPENREVIEW_SKIP_MARKER}"`, + }; + } + + const headRef = pr.head.ref; + if (isStackedTestBranch(headRef, deps.options.coverageServiceBranchPrefix)) { + return { + status: 'ignored', + reason: 'OpenReview stacked test PR (auto-generated tests)', + }; } const owner = payload.repository.owner.login; @@ -54,7 +69,6 @@ export async function handlePullRequest( const prNumber = pr.number; const headSha = pr.head.sha; const baseSha = pr.base.sha; - const headRef = pr.head.ref; const baseRef = pr.base.ref; await deps.downstream.forwardPullRequest({