watchpr: start the conflict window at the arm
ci/woodpecker/pr/build Pipeline was successful
ci/woodpecker/pr/pre-commit Pipeline was successful
ci/woodpecker/pr/test Pipeline was successful

The window ran from the first non-mergeable poll even while the rule was
disarmed, so the poll that armed it confirmed a run it played no part in.
Measure from the later of the run's start and the arm; re-arming stays a
no-op so a base moving under every poll still confirms.
This commit is contained in:
2026-09-26 22:14:25 +10:00
parent 7ffa123e3d
commit 5928233a97
6 changed files with 218 additions and 41 deletions
+58
View File
@@ -1253,6 +1253,19 @@ func TestWatchConflictSequences(t *testing.T) {
name: "a base moving under every poll still confirms a conflict",
interval: normal, baseline: yes, polls: movingBase(MergeNo, 20), want: conflict,
},
{
name: "a base move does not confirm a run it took no part in",
interval: normal, baseline: no, polls: []PRState{no, no, no, movedNo}, want: "",
},
{
name: "a push does not confirm a run it took no part in",
interval: normal, baseline: no, polls: []PRState{no, no, no, pushedNo}, want: "",
},
{
name: "an armed run confirms a window after the arm, not before",
interval: normal, baseline: no,
polls: []PRState{no, no, no, movedNo, movedNo, movedNo}, want: conflict,
},
{
name: "a conflict that predates the watch stays silent forever",
interval: relaxed, baseline: no, polls: repeat(no, 50), want: "",
@@ -1331,3 +1344,48 @@ func TestWatchConflictIsolatedFromANeighboursPushes(t *testing.T) {
t.Errorf("alert names %s, want %s", res.Ref, conflicted)
}
}
// The window is measured from the arm, not from the run the arming poll walked
// in on: a base branch that moves under a long-conflicted PR arms the rule and
// then has to outlast the recompute itself before anything is reported.
func TestWatchConflictWindowStartsAtTheArm(t *testing.T) {
stuck := base()
stuck.Mergeable = MergeNo
moved := stuck
moved.BaseSHA = "base111"
f := &fakeFetcher{states: []PRState{stuck, stuck, stuck, stuck, moved, moved, moved}}
res, err := Watch(f, []PRRef{stuck.Ref}, "unkin-agent", spacedTicks(6, time.Minute), nil, nil)
if err != nil {
t.Fatalf("Watch: %v", err)
}
if res.Reason != "PR lost mergeability (conflict)" {
t.Fatalf("reason = %q, want the mergeability loss two minutes after the base moved", res.Reason)
}
if f.calls != 7 {
t.Errorf("fetch calls = %d, want 7: the window runs from the arming poll, not from the run it inherited", f.calls)
}
}
// The zero time is a legal clock value, so it must not double as the "no run in
// progress" sentinel: a caller whose ticks start at the zero time gets the same
// debounce as any other.
func TestWatchConflictWindowCountsFromTheZeroTime(t *testing.T) {
ok := base()
conflicted := base()
conflicted.Mergeable = MergeNo
ticks := make(chan time.Time, 2)
ticks <- time.Time{}
ticks <- time.Time{}.Add(10 * time.Minute)
close(ticks)
f := &fakeFetcher{states: []PRState{ok, conflicted, conflicted}}
res, err := Watch(f, []PRRef{ok.Ref}, "unkin-agent", ticks, nil, nil)
if err != nil {
t.Fatalf("Watch: %v", err)
}
if res.Reason != "PR lost mergeability (conflict)" {
t.Fatalf("reason = %q, want the mergeability loss; a run starting at the zero time still counts", res.Reason)
}
}