Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions service/src/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export function createApp(deps: AppDeps): Express {
logger: deps.logger,
pullRequestOptions: {
coverageServiceEnabled: deps.cfg.coverageServiceEnabled,
coverageServiceBranchPrefix: deps.cfg.coverageServiceBranchPrefix,
},
},
deps.logger,
Expand Down
5 changes: 5 additions & 0 deletions service/src/dispatch/coverage/summary.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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('<!-- openreview: skip -->');
});
});
3 changes: 3 additions & 0 deletions service/src/dispatch/coverage/summary.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { OPENREVIEW_SKIP_MARKER } from '../../github/stacked-test-pr.js';

import type { PrRun } from './types.js';

/**
Expand Down Expand Up @@ -101,6 +103,7 @@ export function buildTestPRBody(input: SummaryInput & { headRef: string }): stri
'',
'> Merge this PR *into the feature branch* (not into the default branch).',
'',
`<!-- ${OPENREVIEW_SKIP_MARKER} -->`,
'<!-- openreview:coverage:test-pr -->',
);
return lines.join('\n');
Expand Down
32 changes: 32 additions & 0 deletions service/src/github/stacked-test-pr.test.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
17 changes: 17 additions & 0 deletions service/src/github/stacked-test-pr.ts
Original file line number Diff line number Diff line change
@@ -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);
}
2 changes: 1 addition & 1 deletion service/src/jobs/processors/coverage-analysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
15 changes: 15 additions & 0 deletions service/src/webhook/handlers/pull-request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
24 changes: 19 additions & 5 deletions service/src/webhook/handlers/pull-request.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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;
}

/**
Expand Down Expand Up @@ -45,16 +49,26 @@ 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;
const repo = payload.repository.name;
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({
Expand Down
Loading