mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 05:43:14 +00:00
test(e2e): isolate reviewer verdict coverage
This commit is contained in:
parent
99b376bbe2
commit
fe03b05ad4
5 changed files with 83 additions and 32 deletions
|
|
@ -71,6 +71,7 @@ def slack_messages(channel: str) -> list[dict[str, Any]]:
|
||||||
PULLS: list[dict[str, Any]] = []
|
PULLS: list[dict[str, Any]] = []
|
||||||
CHECK_RUNS: list[dict[str, Any]] = []
|
CHECK_RUNS: list[dict[str, Any]] = []
|
||||||
REVIEW_DISPATCHES: list[dict[str, Any]] = []
|
REVIEW_DISPATCHES: list[dict[str, Any]] = []
|
||||||
|
EMAIL_MAPPING_LOOKUPS: list[str] = []
|
||||||
_pr_seq = [0]
|
_pr_seq = [0]
|
||||||
_check_seq = [0]
|
_check_seq = [0]
|
||||||
|
|
||||||
|
|
@ -214,6 +215,7 @@ def reset() -> None:
|
||||||
PULLS.clear()
|
PULLS.clear()
|
||||||
CHECK_RUNS.clear()
|
CHECK_RUNS.clear()
|
||||||
REVIEW_DISPATCHES.clear()
|
REVIEW_DISPATCHES.clear()
|
||||||
|
EMAIL_MAPPING_LOOKUPS.clear()
|
||||||
_pr_seq[0] = 0
|
_pr_seq[0] = 0
|
||||||
_check_seq[0] = 0
|
_check_seq[0] = 0
|
||||||
seed_bare_remote()
|
seed_bare_remote()
|
||||||
|
|
|
||||||
|
|
@ -62,7 +62,7 @@ _SLACK_USERS: dict[str, dict[str, str]] = {
|
||||||
from agent.api.app import app # noqa: E402
|
from agent.api.app import app # noqa: E402
|
||||||
from agent.dashboard import routes as dashboard_routes # noqa: E402
|
from agent.dashboard import routes as dashboard_routes # noqa: E402
|
||||||
from agent.dashboard.oauth import COOKIE_NAME, issue_session # noqa: E402
|
from agent.dashboard.oauth import COOKIE_NAME, issue_session # noqa: E402
|
||||||
from agent.utils import github_checks # noqa: E402
|
from agent.utils import github_checks, github_org_membership # noqa: E402
|
||||||
from agent.utils.thread_ids import generate_thread_id_from_slack_thread # noqa: E402
|
from agent.utils.thread_ids import generate_thread_id_from_slack_thread # noqa: E402
|
||||||
from agent.webhooks import common as webhook_common # noqa: E402
|
from agent.webhooks import common as webhook_common # noqa: E402
|
||||||
|
|
||||||
|
|
@ -74,6 +74,7 @@ CURRENT_THREAD: dict[str, str | None] = {"channel": DEMO_CHANNEL, "thread_ts": N
|
||||||
|
|
||||||
fakes.seed_bare_remote()
|
fakes.seed_bare_remote()
|
||||||
_real_dispatch_agent_run = webhook_common.dispatch_agent_run
|
_real_dispatch_agent_run = webhook_common.dispatch_agent_run
|
||||||
|
_real_email_for_login = webhook_common.email_for_login
|
||||||
|
|
||||||
|
|
||||||
async def _fake_installation_token(*_args: object, **_kwargs: object) -> str:
|
async def _fake_installation_token(*_args: object, **_kwargs: object) -> str:
|
||||||
|
|
@ -103,6 +104,15 @@ async def _fake_started_comment(*_args: object, **_kwargs: object) -> None:
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
async def _fake_active_org_member(username: str, org: str) -> bool:
|
||||||
|
return bool(username and org)
|
||||||
|
|
||||||
|
|
||||||
|
async def _record_email_mapping_lookup(login: str) -> str | None:
|
||||||
|
fakes.EMAIL_MAPPING_LOOKUPS.append(login)
|
||||||
|
return await _real_email_for_login(login)
|
||||||
|
|
||||||
|
|
||||||
async def _record_review_dispatch(
|
async def _record_review_dispatch(
|
||||||
thread_id: str,
|
thread_id: str,
|
||||||
prompt: str,
|
prompt: str,
|
||||||
|
|
@ -151,6 +161,9 @@ webhook_common._reviewer_token_for_repo = _fake_reviewer_token
|
||||||
webhook_common.react_to_github_comment = _fake_reaction
|
webhook_common.react_to_github_comment = _fake_reaction
|
||||||
webhook_common.post_review_started_comment = _fake_started_comment
|
webhook_common.post_review_started_comment = _fake_started_comment
|
||||||
webhook_common.dispatch_agent_run = _record_review_dispatch
|
webhook_common.dispatch_agent_run = _record_review_dispatch
|
||||||
|
webhook_common.email_for_login = _record_email_mapping_lookup
|
||||||
|
github_org_membership.is_user_active_org_member = _fake_active_org_member
|
||||||
|
webhook_common.is_user_active_org_member = _fake_active_org_member
|
||||||
dashboard_routes._fetch_user_installations_and_repos = _fake_installations_and_repos
|
dashboard_routes._fetch_user_installations_and_repos = _fake_installations_and_repos
|
||||||
github_checks._GITHUB_API_BASE = e2e_env.FAKE_GITHUB_API
|
github_checks._GITHUB_API_BASE = e2e_env.FAKE_GITHUB_API
|
||||||
|
|
||||||
|
|
@ -175,6 +188,11 @@ async def control_review_dispatches() -> JSONResponse:
|
||||||
return JSONResponse(fakes.REVIEW_DISPATCHES)
|
return JSONResponse(fakes.REVIEW_DISPATCHES)
|
||||||
|
|
||||||
|
|
||||||
|
@app.get("/control/email-mapping-lookups")
|
||||||
|
async def control_email_mapping_lookups() -> JSONResponse:
|
||||||
|
return JSONResponse(fakes.EMAIL_MAPPING_LOOKUPS)
|
||||||
|
|
||||||
|
|
||||||
@app.get("/control/check-runs")
|
@app.get("/control/check-runs")
|
||||||
async def control_check_runs() -> JSONResponse:
|
async def control_check_runs() -> JSONResponse:
|
||||||
return JSONResponse(fakes.CHECK_RUNS)
|
return JSONResponse(fakes.CHECK_RUNS)
|
||||||
|
|
|
||||||
|
|
@ -56,33 +56,54 @@ async function expectTranscriptVisible(page: Page) {
|
||||||
}).toPass({ timeout: 60000 });
|
}).toPass({ timeout: 60000 });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async function resetAutoVerdictPolicies(
|
||||||
|
page: Page,
|
||||||
|
baseURL: string | undefined,
|
||||||
|
) {
|
||||||
|
await loginAs(page, SAME_USER);
|
||||||
|
const headers = { origin: baseURL ?? "" };
|
||||||
|
const currentResponse = await page.request.get(
|
||||||
|
"/dashboard/api/team-settings",
|
||||||
|
);
|
||||||
|
expect(currentResponse.ok()).toBeTruthy();
|
||||||
|
const current = await currentResponse.json();
|
||||||
|
const teamResponse = await page.request.put("/dashboard/api/team-settings", {
|
||||||
|
data: { ...current, auto_verdict: false },
|
||||||
|
headers,
|
||||||
|
});
|
||||||
|
expect(teamResponse.ok(), await teamResponse.text()).toBeTruthy();
|
||||||
|
const repoResponse = await page.request.put(
|
||||||
|
"/dashboard/api/auto-verdict-repos",
|
||||||
|
{
|
||||||
|
data: { full_name: "fakeorg/demo", enabled: false },
|
||||||
|
headers,
|
||||||
|
},
|
||||||
|
);
|
||||||
|
expect(repoResponse.ok()).toBeTruthy();
|
||||||
|
}
|
||||||
|
|
||||||
test.describe("Review verdict settings (real dashboard UI and API)", () => {
|
test.describe("Review verdict settings (real dashboard UI and API)", () => {
|
||||||
|
test.beforeEach(async ({ page, baseURL }) => {
|
||||||
|
await resetAutoVerdictPolicies(page, baseURL);
|
||||||
|
});
|
||||||
|
|
||||||
|
test.afterEach(async ({ page, baseURL }) => {
|
||||||
|
await resetAutoVerdictPolicies(page, baseURL);
|
||||||
|
});
|
||||||
|
|
||||||
test("admin controls persist team and per-repository auto-verdict policy", async ({
|
test("admin controls persist team and per-repository auto-verdict policy", async ({
|
||||||
page,
|
page,
|
||||||
baseURL,
|
baseURL,
|
||||||
}) => {
|
}) => {
|
||||||
await loginAs(page, SAME_USER);
|
|
||||||
const mutationHeaders = { origin: baseURL ?? "" };
|
const mutationHeaders = { origin: baseURL ?? "" };
|
||||||
|
|
||||||
const resetTeam = await page.request.put("/dashboard/api/team-settings", {
|
|
||||||
data: { auto_verdict: false },
|
|
||||||
headers: mutationHeaders,
|
|
||||||
});
|
|
||||||
expect(resetTeam.ok(), await resetTeam.text()).toBeTruthy();
|
|
||||||
const resetRepo = await page.request.put(
|
|
||||||
"/dashboard/api/auto-verdict-repos",
|
|
||||||
{
|
|
||||||
data: { full_name: "fakeorg/demo", enabled: false },
|
|
||||||
headers: mutationHeaders,
|
|
||||||
},
|
|
||||||
);
|
|
||||||
expect(resetRepo.ok()).toBeTruthy();
|
|
||||||
|
|
||||||
await page.goto("/review");
|
await page.goto("/review");
|
||||||
await expect(
|
await expect(
|
||||||
page.getByRole("heading", { name: "Open SWE Review" }),
|
page.getByRole("heading", { name: "Open SWE Review" }),
|
||||||
).toBeVisible();
|
).toBeVisible();
|
||||||
const teamToggle = page.getByRole("switch").first();
|
const teamToggle = page.getByRole("switch", {
|
||||||
|
name: "Allow automatic verdicts team-wide",
|
||||||
|
});
|
||||||
await expect(teamToggle).not.toBeChecked();
|
await expect(teamToggle).not.toBeChecked();
|
||||||
const teamSaved = page.waitForResponse(
|
const teamSaved = page.waitForResponse(
|
||||||
(response) =>
|
(response) =>
|
||||||
|
|
|
||||||
|
|
@ -72,11 +72,11 @@ test.describe("Reviewer verification contracts", () => {
|
||||||
repo: { owner: "fakeorg", name: "demo" },
|
repo: { owner: "fakeorg", name: "demo" },
|
||||||
pr_number: pr.number,
|
pr_number: pr.number,
|
||||||
});
|
});
|
||||||
expect(dispatches[0].configurable).not.toHaveProperty("email");
|
const emailMappingLookups = await (
|
||||||
expect(dispatches[0].prompt).toContain(
|
await page.request.get("/control/email-mapping-lookups")
|
||||||
"focus on authorization boundaries",
|
).json();
|
||||||
);
|
expect(emailMappingLookups).toEqual([]);
|
||||||
|
expect(dispatches[0].prompt).toContain("focus on authorization boundaries");
|
||||||
});
|
});
|
||||||
|
|
||||||
test("review outcomes settle checks as success, failure, or neutral", async ({
|
test("review outcomes settle checks as success, failure, or neutral", async ({
|
||||||
|
|
@ -102,6 +102,16 @@ test.describe("Reviewer verification contracts", () => {
|
||||||
conclusion: "failure",
|
conclusion: "failure",
|
||||||
title: "Changes requested",
|
title: "Changes requested",
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
outcome: {
|
||||||
|
verdict_submitted: false,
|
||||||
|
verdict_authorization: "consistent",
|
||||||
|
verdict_ignored_reason: "approve_with_open_findings",
|
||||||
|
blocking_finding_count: 2,
|
||||||
|
},
|
||||||
|
conclusion: "failure",
|
||||||
|
title: "Found 2 blocking issues",
|
||||||
|
},
|
||||||
{
|
{
|
||||||
outcome: {
|
outcome: {
|
||||||
verdict_submitted: false,
|
verdict_submitted: false,
|
||||||
|
|
@ -130,16 +140,15 @@ test.describe("Reviewer verification contracts", () => {
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
const checks = await (
|
const checks = await (await page.request.get("/control/check-runs")).json();
|
||||||
await page.request.get("/control/check-runs")
|
|
||||||
).json();
|
|
||||||
expect(checks.map((check: { conclusion: string }) => check.conclusion)).toEqual([
|
|
||||||
"success",
|
|
||||||
"failure",
|
|
||||||
"neutral",
|
|
||||||
]);
|
|
||||||
expect(
|
expect(
|
||||||
checks.map((check: { status: string }) => check.status),
|
checks.map((check: { conclusion: string }) => check.conclusion),
|
||||||
).toEqual(["completed", "completed", "completed"]);
|
).toEqual(["success", "failure", "failure", "neutral"]);
|
||||||
|
expect(checks.map((check: { status: string }) => check.status)).toEqual([
|
||||||
|
"completed",
|
||||||
|
"completed",
|
||||||
|
"completed",
|
||||||
|
"completed",
|
||||||
|
]);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -147,6 +147,7 @@ function ReviewPage() {
|
||||||
description="Allow trusted, non-fork pull requests to receive reviewer verdicts by default. Repository-specific opt-ins remain available when this is off."
|
description="Allow trusted, non-fork pull requests to receive reviewer verdicts by default. Repository-specific opt-ins remain available when this is off."
|
||||||
control={
|
control={
|
||||||
<Switch
|
<Switch
|
||||||
|
aria-label="Allow automatic verdicts team-wide"
|
||||||
checked={current.auto_verdict}
|
checked={current.auto_verdict}
|
||||||
onCheckedChange={(v) => persist({ auto_verdict: v })}
|
onCheckedChange={(v) => persist({ auto_verdict: v })}
|
||||||
disabled={!canEdit}
|
disabled={!canEdit}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue