dongarandClaude Opus 5.5
fca4c41d95
From nisse-review's draft on #156: it thought points made in a review's summary never reached a later review. They do, as "your review: ...". Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01J5Utyfzw7gF7UhjRDEtjTk
360 lines
13 KiB
Python
360 lines
13 KiB
Python
"""M7.5: nisse-review back on a PR it reviewed (R1, R2, R5): later reviews
|
|
of what changed, replies to what was said, the round cap and its resets,
|
|
and an operator's `stop` and `review`."""
|
|
|
|
from dataclasses import replace
|
|
from pathlib import Path
|
|
from typing import Any
|
|
|
|
import pytest
|
|
from review_fakes import FakeGitea, Scripted, call, pr
|
|
from test_review import REPO, REVIEW, item, world
|
|
|
|
from nisse_core.gitea import GiteaError
|
|
from nisse_review.review import Said, marker, reply_marker
|
|
from nisse_review.scheduler import command
|
|
from nisse_review.state import (
|
|
CAPPED,
|
|
FAILED,
|
|
READY,
|
|
REPLIES,
|
|
REVIEWED,
|
|
RUNNING,
|
|
STOPPED,
|
|
WAITING,
|
|
data,
|
|
pr_of,
|
|
)
|
|
|
|
PUSHED = "def789abc012"
|
|
REPLY_P1 = [
|
|
[call("reply", point="P1", text="Fair: tabs never occur here. Conceded.")],
|
|
[call("done")],
|
|
]
|
|
|
|
|
|
def reviewed(tmp_path: Path, *turns: list[Any], **more: Any) -> dict[str, Any]:
|
|
"""A PR given its first review; ``turns`` are the model's after that."""
|
|
gitea = FakeGitea(repos={REPO: [pr()]})
|
|
w = world(
|
|
tmp_path, Scripted(*REVIEW, *turns), gitea, operators=frozenset({"don"}), **more
|
|
)
|
|
assert w["scheduler"].round() is True
|
|
assert item(w).position == REVIEWED
|
|
return w
|
|
|
|
|
|
def push(w: dict[str, Any], head: str = PUSHED) -> None:
|
|
w["gitea"].repos[REPO] = [pr(head_sha=head)]
|
|
w["gitea"].since = [f"{head} Handle tabs"]
|
|
|
|
|
|
def last_brief(w: dict[str, Any]) -> str:
|
|
model = w["scheduler"]._reviewer._driver._client
|
|
return str(model.seen[-1])
|
|
|
|
|
|
# --- replies (R1) -----------------------------------------------------------------
|
|
|
|
|
|
def test_an_answer_on_its_line_is_replied_to_on_that_line(tmp_path: Path) -> None:
|
|
w = reviewed(tmp_path, *REPLY_P1)
|
|
key = w["gitea"].note("nisse-dev", "split() is fine: no tabs here.", "count.py:2")
|
|
assert w["scheduler"].round() is True # replies wait for no CI
|
|
reply = w["gitea"].posted[-1]
|
|
[line] = reply["comments"]
|
|
assert (line.path, line.line) == ("count.py", 2)
|
|
assert reply_marker(key) in line.body and "Conceded" in line.body
|
|
assert (item(w).position, pr_of(item(w)).rounds) == (REVIEWED, 2)
|
|
assert "P1 — nisse-dev on count.py:2" in last_brief(w)
|
|
assert w["scheduler"].round() is False # its own reply wakes nothing
|
|
|
|
|
|
def test_a_top_level_comment_is_answered_quoting_it(tmp_path: Path) -> None:
|
|
w = reviewed(tmp_path, *REPLY_P1)
|
|
w["gitea"].say("nisse-dev", "Why flag the README?")
|
|
w["scheduler"].round()
|
|
[(_, _, body)] = w["gitea"].comments
|
|
assert body.startswith("> nisse-dev:\n> Why flag the README?")
|
|
|
|
|
|
def test_every_point_is_answered_or_skipped_before_done(tmp_path: Path) -> None:
|
|
turns = [
|
|
[call("done")], # P1 still open: refused, the run goes on
|
|
[call("skip", point="P1", reason="a CI notice; nothing to answer")],
|
|
[call("done")],
|
|
]
|
|
w = reviewed(tmp_path, *turns)
|
|
w["gitea"].say("bot-ci", "Build 12 passed.")
|
|
w["scheduler"].round()
|
|
assert "first answer or skip each point said to you: P1" in str(
|
|
w["scheduler"]._reviewer._driver._client.seen[-1]
|
|
)
|
|
assert item(w).position == REVIEWED
|
|
assert pr_of(item(w)).rounds == 0 # a bot joined in: reset; nothing posted
|
|
|
|
|
|
def test_a_point_answered_before_a_crash_is_not_answered_again(
|
|
tmp_path: Path,
|
|
) -> None:
|
|
w = reviewed(tmp_path) # no more model turns: a run would fail
|
|
key = w["gitea"].note("nisse-dev", "Done.", "count.py:2")
|
|
w["scheduler"].intake()
|
|
w["gitea"].say("nisse-review", f"Thanks.\n\n{reply_marker(key)}")
|
|
known = pr_of(item(w))
|
|
w["store"].update_work_item(
|
|
item(w).id, position=RUNNING, data=data(replace(known, run=REPLIES)), cause="t"
|
|
)
|
|
posts = len(w["gitea"].posted)
|
|
w["scheduler"].round() # recovered, then nothing left to answer
|
|
assert item(w).position == REVIEWED and len(w["gitea"].posted) == posts
|
|
|
|
|
|
# --- later reviews (R1) -----------------------------------------------------------
|
|
|
|
|
|
def test_a_push_gets_a_later_review_of_what_changed(tmp_path: Path) -> None:
|
|
later = [[call("post_review", summary="P1 addressed: tabs handled.")]]
|
|
w = reviewed(tmp_path, *later)
|
|
push(w)
|
|
assert w["scheduler"].round() is True
|
|
brief = last_brief(w)
|
|
assert "The commits since" in brief and f"diff of {PUSHED}" in brief
|
|
assert "split() handles tabs too." in brief # its earlier point on a line
|
|
assert "- your review: Looks right; two small points." in brief # its summary
|
|
review = w["gitea"].posted[-1]
|
|
assert review["commit"] == PUSHED and marker(PUSHED, 2) in review["body"]
|
|
known = pr_of(item(w))
|
|
assert (known.reviewed, known.posts, known.rounds) == (PUSHED, 2, 1)
|
|
|
|
|
|
def test_without_the_commits_since_it_reads_the_whole_diff(tmp_path: Path) -> None:
|
|
w = reviewed(tmp_path, [call("post_review", summary="ok")])
|
|
push(w)
|
|
w["gitea"].since = [] # a force-push dropped the old head
|
|
w["scheduler"].round()
|
|
assert "the whole PR's diff instead" in last_brief(w)
|
|
|
|
|
|
def test_a_sign_off_is_said_once_while_it_stays_happy(tmp_path: Path) -> None:
|
|
gitea = FakeGitea(repos={REPO: [pr()]})
|
|
model = Scripted(
|
|
[call("sign_off", summary="LGTM: small and tested.")],
|
|
[call("still_happy")],
|
|
)
|
|
w = world(tmp_path, model, gitea)
|
|
w["scheduler"].round()
|
|
assert pr_of(item(w)).happy and len(gitea.posted) == 1
|
|
push(w)
|
|
w["scheduler"].round()
|
|
assert len(gitea.posted) == 1 # nothing new: not said again
|
|
assert "still_happy" in model.offered[-1] and "sign_off" not in model.offered[-1]
|
|
assert pr_of(item(w)).reviewed == PUSHED
|
|
|
|
|
|
def test_a_sign_off_with_drafted_comments_is_refused(tmp_path: Path) -> None:
|
|
gitea = FakeGitea(repos={REPO: [pr()]})
|
|
model = Scripted(
|
|
[call("comment", path="count.py", line="2", text="Tabs?")],
|
|
[call("sign_off", summary="LGTM")],
|
|
[call("post_review", summary="One point.")],
|
|
)
|
|
w = world(tmp_path, model, gitea)
|
|
w["scheduler"].round()
|
|
assert "post them with post_review" in str(model.seen[-1])
|
|
assert not pr_of(item(w)).happy and len(gitea.posted[-1]["comments"]) == 1
|
|
|
|
|
|
# --- rounds (R2) -------------------------------------------------------------------
|
|
|
|
|
|
def test_after_the_cap_it_sums_up_and_goes_quiet(tmp_path: Path) -> None:
|
|
summary = [[call("post_summary", summary="Open: README wording. Don decides.")]]
|
|
w = reviewed(tmp_path, *REPLY_P1, *summary, max_rounds=2)
|
|
w["gitea"].note("nisse-dev", "No tabs here.", "count.py:2")
|
|
w["scheduler"].round() # round 2: replies
|
|
w["gitea"].say("nisse-dev", "Still disagree on the README.")
|
|
w["scheduler"].round() # past the cap: the summary
|
|
assert item(w).position == CAPPED
|
|
assert "Don decides" in w["gitea"].posted[-1]["body"]
|
|
w["gitea"].say("nisse-dev", "And another thing.")
|
|
assert w["scheduler"].round() is False # the author alone: still quiet
|
|
assert item(w).position == CAPPED
|
|
|
|
|
|
@pytest.mark.parametrize("who", ["don", "bot-ci"])
|
|
def test_someone_else_joining_in_resets_the_count(tmp_path: Path, who: str) -> None:
|
|
summary = [[call("post_summary", summary="Open: README.")]]
|
|
w = reviewed(tmp_path, *summary, max_rounds=1)
|
|
w["gitea"].say("nisse-dev", "Disagree.")
|
|
w["scheduler"].round()
|
|
assert item(w).position == CAPPED
|
|
w["gitea"].say(who, "What's left here?")
|
|
w["scheduler"].intake()
|
|
assert (item(w).position, pr_of(item(w)).rounds) == (READY, 0)
|
|
|
|
|
|
def test_a_push_resets_the_count(tmp_path: Path) -> None:
|
|
summary = [[call("post_summary", summary="Open: README.")]]
|
|
w = reviewed(tmp_path, *summary, max_rounds=1)
|
|
w["gitea"].say("nisse-dev", "Disagree.")
|
|
w["scheduler"].round()
|
|
push(w)
|
|
w["scheduler"].intake()
|
|
assert (item(w).position, pr_of(item(w)).rounds) == (READY, 0)
|
|
|
|
|
|
# --- stop and review (R5) ----------------------------------------------------------
|
|
|
|
|
|
def test_stop_quiets_it_until_review_asks_again(tmp_path: Path) -> None:
|
|
fresh = [
|
|
[call("read_file", path="count.py", ref="head")],
|
|
[call("post_review", summary="Fresh look: fine.")],
|
|
]
|
|
w = reviewed(tmp_path, *fresh)
|
|
stop = w["gitea"].say("don", "stop")
|
|
w["gitea"].say("nisse-dev", "Any thoughts?")
|
|
push(w)
|
|
assert w["scheduler"].round() is False
|
|
assert item(w).position == STOPPED
|
|
assert (stop, "+1") in w["gitea"].reactions
|
|
w["gitea"].say("don", "Review.")
|
|
assert w["scheduler"].round() is True
|
|
assert "Review o/demo#7" in last_brief(w) # the whole PR, afresh
|
|
assert w["gitea"].posted[-1]["commit"] == PUSHED
|
|
assert (item(w).position, pr_of(item(w)).rounds) == (REVIEWED, 1)
|
|
|
|
|
|
def test_review_retries_a_failed_item(tmp_path: Path) -> None:
|
|
gitea = FakeGitea(repos={REPO: [pr()]})
|
|
w = world(
|
|
tmp_path, Scripted(*REVIEW), gitea, attempts=1, operators=frozenset({"don"})
|
|
)
|
|
w["store"].open_work_item(
|
|
f"{REPO}#7",
|
|
title="t",
|
|
scope=REPO,
|
|
position=FAILED,
|
|
data=data(pr_of_fresh(attempts=1)),
|
|
cause="t",
|
|
)
|
|
gitea.say("don", "review")
|
|
assert w["scheduler"].round() is True
|
|
assert item(w).position == REVIEWED
|
|
|
|
|
|
def pr_of_fresh(**more: Any):
|
|
from nisse_review.state import PR
|
|
|
|
return PR(REPO, 7, pr().head_sha, seen=("comment-1",), **more)
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
("said", "expected"),
|
|
[
|
|
(Said("comment-5", "don", " Stop "), "stop"),
|
|
(Said("comment-5", "don", "`review`"), "review"),
|
|
(Said("comment-5", "don", "please stop"), None),
|
|
(Said("comment-5", "nisse-dev", "stop"), None), # not an operator
|
|
(Said("line-5", "don", "stop", "a.py:1"), None), # not top-level
|
|
],
|
|
)
|
|
def test_a_command_is_an_operators_whole_comment(said: Said, expected) -> None:
|
|
assert command(said, frozenset({"don"})) == expected
|
|
|
|
|
|
# --- upgrades, failures and dead ends ----------------------------------------------
|
|
|
|
|
|
def test_an_item_reviewed_before_m7_5_answers_nothing_old(tmp_path: Path) -> None:
|
|
gitea = FakeGitea(repos={REPO: [pr()]})
|
|
w = world(tmp_path, Scripted(), gitea)
|
|
old = {
|
|
"repo": REPO,
|
|
"number": 7,
|
|
"head": pr().head_sha,
|
|
"reviewed": pr().head_sha,
|
|
"attempts": 0,
|
|
"seen_at": 0.0,
|
|
}
|
|
w["store"].open_work_item(
|
|
f"{REPO}#7",
|
|
title="t",
|
|
scope=REPO,
|
|
position=REVIEWED,
|
|
data={"pr": old},
|
|
cause="t",
|
|
)
|
|
gitea.say("nisse-dev", "An old answer.")
|
|
assert w["scheduler"].round() is False
|
|
known = pr_of(item(w))
|
|
assert (known.fresh, known.posts) == ((), 1)
|
|
|
|
|
|
def test_what_was_said_unreadable_holds_up_no_other_pr(tmp_path: Path) -> None:
|
|
class Flaky(FakeGitea):
|
|
def list_comments(self, repo: str, index: int, since: str | None = None):
|
|
if index == 7:
|
|
raise GiteaError("502")
|
|
return super().list_comments(repo, index)
|
|
|
|
gitea = Flaky(repos={REPO: [pr(7), pr(8, head_sha="c8")]})
|
|
w = world(tmp_path, Scripted(*REVIEW), gitea)
|
|
assert w["scheduler"].round() is True
|
|
assert w["store"].work_item(f"{REPO}#7") is None # next round
|
|
assert item(w, 8).position == REVIEWED
|
|
|
|
|
|
def test_no_position_is_a_dead_end(tmp_path: Path) -> None:
|
|
"""Every position an open item can hold has an event that moves it on."""
|
|
w = reviewed(tmp_path, *REPLY_P1, max_rounds=1)
|
|
s, g = w["scheduler"], w["gitea"]
|
|
|
|
def at(position: str, **more: Any) -> None:
|
|
w["store"].update_work_item(
|
|
item(w).id,
|
|
position=position,
|
|
data=data(replace(pr_of(item(w)), **more)),
|
|
cause="t",
|
|
)
|
|
|
|
at(STOPPED)
|
|
g.say("don", "review")
|
|
s.intake()
|
|
assert item(w).position == READY
|
|
at(CAPPED, full=False)
|
|
push(w, "cap0000")
|
|
s.intake()
|
|
assert item(w).position == READY
|
|
at(FAILED)
|
|
push(w, "fail000")
|
|
s.intake()
|
|
assert item(w).position == READY
|
|
at(REVIEWED, reviewed="fail000")
|
|
g.say("bot-ci", "Coverage 90%.")
|
|
s.intake()
|
|
assert item(w).position == READY
|
|
at(WAITING)
|
|
s.intake()
|
|
assert item(w).position == READY # ready again: back in line
|
|
at(RUNNING, run=REPLIES)
|
|
s.recover()
|
|
assert item(w).position == READY
|
|
|
|
|
|
def test_a_summary_answers_nothing_and_a_first_review_answers_no_points(
|
|
tmp_path: Path,
|
|
) -> None:
|
|
gitea = FakeGitea(repos={REPO: [pr()]})
|
|
model = Scripted(*REVIEW, [call("post_summary", summary="Open: README.")])
|
|
w = world(tmp_path, model, gitea, max_rounds=0)
|
|
w["scheduler"].intake()
|
|
gitea.say("nisse-dev", "Said before any review.")
|
|
w["scheduler"].round() # its first review: no points to answer one by one
|
|
assert "reply" not in model.offered[0] and item(w).position == REVIEWED
|
|
gitea.say("nisse-dev", "Disagree.")
|
|
w["scheduler"].round()
|
|
assert "reply" not in model.offered[-1] and "post_summary" in model.offered[-1]
|
|
assert item(w).position == CAPPED
|