fix(robomp): harden lifecycle contract review cases
This commit is contained in:
@@ -1,8 +1,13 @@
|
||||
"""Contract test for the robomp server API response, producing status-contract.json."""
|
||||
"""Contract test for the robomp server API response vs status-contract.json.
|
||||
|
||||
Regenerate the committed fixture with:
|
||||
ROBOMP_UPDATE_STATUS_CONTRACT=1 pytest tests/test_status_contract.py
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
@@ -14,10 +19,11 @@ from robomp.db import get_database
|
||||
from robomp.server import create_app
|
||||
|
||||
|
||||
# Runtime/timestamp fields vary every run; normalize them so an ordinary pytest
|
||||
# run regenerates a byte-identical committed fixture instead of dirtying it.
|
||||
# Runtime/timestamp fields vary every run; normalize them so the live payload
|
||||
# can be compared against (or regenerated into) a byte-stable committed fixture.
|
||||
_VOLATILE_TS_KEYS = {"received_at", "started_at", "last_tool_ts", "updated_at"}
|
||||
_FIXED_TS = "2024-01-01T00:00:00+00:00"
|
||||
_UPDATE_ENV = "ROBOMP_UPDATE_STATUS_CONTRACT"
|
||||
|
||||
|
||||
def _normalize_for_fixture(value: Any) -> Any:
|
||||
@@ -191,12 +197,16 @@ def test_status_contract(settings: Settings) -> None:
|
||||
# check runtime:
|
||||
assert data["runtime"]["repo_allowlist"] == ["octo/widget"]
|
||||
|
||||
# Write fixture:
|
||||
# Compare the normalized payload against the committed fixture. Normal
|
||||
# pytest runs assert equality; regenerate only when ROBOMP_UPDATE_STATUS_CONTRACT=1.
|
||||
fixture_path = Path(__file__).parent.parent / "web/test/fixtures/status-contract.json"
|
||||
fixture_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
fixture_path.write_text(
|
||||
json.dumps(_normalize_for_fixture(data), indent=2), encoding="utf-8"
|
||||
)
|
||||
actual = _normalize_for_fixture(data)
|
||||
if os.environ.get(_UPDATE_ENV) == "1":
|
||||
fixture_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
fixture_path.write_text(json.dumps(actual, indent=2), encoding="utf-8")
|
||||
else:
|
||||
expected = json.loads(fixture_path.read_text(encoding="utf-8"))
|
||||
assert actual == expected
|
||||
|
||||
|
||||
def _enable_replay(monkeypatch: pytest.MonkeyPatch) -> str:
|
||||
|
||||
@@ -8,9 +8,11 @@ export type IssueState =
|
||||
| "new"
|
||||
| "reproducing"
|
||||
| "fixing"
|
||||
| "reviewing"
|
||||
| "opened"
|
||||
| "merged"
|
||||
| "closed"
|
||||
| "needs_info"
|
||||
| "abandoned";
|
||||
|
||||
export interface RuntimeInfo {
|
||||
@@ -37,7 +39,7 @@ export interface IssueRow {
|
||||
number: number;
|
||||
branch: string | null;
|
||||
pr_number: number | null;
|
||||
state: IssueState | string;
|
||||
state: IssueState;
|
||||
classification: string | null;
|
||||
updated_at: string;
|
||||
latest_event: LatestEvent | null;
|
||||
|
||||
@@ -470,8 +470,15 @@ describe("buildWorkItems", () => {
|
||||
expect(stageOrdinal("nonsense")).toBe(0);
|
||||
expect(stageOrdinal("new")).toBe(0);
|
||||
expect(stageOrdinal("reproducing")).toBe(1);
|
||||
// needs_info is set by mark_unable_to_reproduce during reproduction and
|
||||
// cleared back to reproducing by repro_record in host_tools; it stays in
|
||||
// the reproduction phase (ordinal 1), not advanced past fixing.
|
||||
expect(stageOrdinal("needs_info")).toBe(1);
|
||||
expect(stageOrdinal("fixing")).toBe(2);
|
||||
// reviewing is PR review work (tasks.py sets it on PR-opened issues); it
|
||||
// lives on the PR step, same stage as opened (ordinal 3).
|
||||
expect(stageOrdinal("opened")).toBe(3);
|
||||
expect(stageOrdinal("reviewing")).toBe(3);
|
||||
expect(stageOrdinal("merged")).toBe(4);
|
||||
});
|
||||
|
||||
@@ -570,4 +577,41 @@ describe("buildWorkItems", () => {
|
||||
expect(items[0].latestEvent?.state).toBe("running");
|
||||
});
|
||||
|
||||
test("live running event outranks a newer queued latest_event for the same issue", () => {
|
||||
const items = buildWorkItems(
|
||||
status({
|
||||
issues: [
|
||||
issue({
|
||||
key: "owner/repo#22",
|
||||
number: 22,
|
||||
latest_event: latestEvent({
|
||||
delivery_id: "queued-newer",
|
||||
state: "queued",
|
||||
received_at: "2026-06-17T00:09:00Z",
|
||||
}),
|
||||
}),
|
||||
],
|
||||
running_events: [
|
||||
runningEvent({
|
||||
delivery_id: "live-older-3",
|
||||
issue_key: "owner/repo#22",
|
||||
received_at: "2026-06-17T00:01:00Z",
|
||||
started_at: "2026-06-17T00:02:00Z",
|
||||
}),
|
||||
],
|
||||
}),
|
||||
);
|
||||
expect(items).toHaveLength(1);
|
||||
expect(items[0]).toMatchObject({
|
||||
key: "owner/repo#22",
|
||||
bucket: "running",
|
||||
deliveryId: "live-older-3",
|
||||
inflightOnly: false,
|
||||
error: null,
|
||||
});
|
||||
expect(items[0].live?.delivery_id).toBe("live-older-3");
|
||||
expect(items[0].latestEvent?.state).toBe("running");
|
||||
expect(items[0].latestEvent?.delivery_id).toBe("live-older-3");
|
||||
});
|
||||
|
||||
});
|
||||
|
||||
@@ -38,8 +38,10 @@ export const SIMPLE_CLASSIFICATIONS: ReadonlySet<string> = new Set([
|
||||
const STATE_ORDINAL: Record<string, number> = {
|
||||
new: 0,
|
||||
reproducing: 1,
|
||||
needs_info: 1,
|
||||
fixing: 2,
|
||||
opened: 3,
|
||||
reviewing: 3,
|
||||
merged: 4,
|
||||
closed: 4,
|
||||
abandoned: 4,
|
||||
@@ -78,10 +80,10 @@ export function buildWorkItems(status: StatusResponse): WorkItem[] {
|
||||
const latest = issue.latest_event;
|
||||
|
||||
// A matching live running_events entry is authoritative over the issue's
|
||||
// own latest_event, which may be a newer failed/done row that the live run
|
||||
// has not yet superseded. Render the live run so the card stays running,
|
||||
// cancel-capable (deliveryId from the live delivery), and free of the
|
||||
// stale failure. Non-live rows keep latest_event authority below.
|
||||
// own latest_event. That summary row can be a newer queued retry/comment,
|
||||
// or a failed/done row the live run has not superseded yet. Render the live
|
||||
// run so the card stays running, cancel-capable (deliveryId from the live
|
||||
// delivery), and free of stale latest-event state.
|
||||
if (live) {
|
||||
items.push({
|
||||
key,
|
||||
|
||||
Reference in New Issue
Block a user