watchpr: describe arming as a one-way latch, not a window restart
The docs claimed every later head/base move restarts the conflict window. arm() returns early once armed, so only the first move sets armedAt and every move after it is a no-op. State the real trade-off instead: repeated moves do not extend the debounce, so a conflict can be confirmed while the newest recompute is younger than the window. Also narrow the --json stderr claim to the notices watchpr writes itself (cobra's terminal Error: line is plain text), rename the test to what it covers, and stop the unknown-mergeability baseline implying a false answer arms the rule.
This commit is contained in:
@@ -105,10 +105,18 @@ reported while the commits stay put: nothing in Gitea's payload separates it fro
|
|||||||
a merge check still in flight, and the baseline line is the only notice of it.
|
a merge check still in flight, and the baseline line is the only notice of it.
|
||||||
That is a narrower gap than it looks, because `base.sha` is the base branch's
|
That is a narrower gap than it looks, because `base.sha` is the base branch's
|
||||||
tip — it moves for every open PR whenever anything merges to the base branch, so
|
tip — it moves for every open PR whenever anything merges to the base branch, so
|
||||||
the commits rarely stay put for long. A move like that arms the rule, and the
|
the commits rarely stay put for long. The first such move arms the rule, and is
|
||||||
move itself is silent: the window restarts from it, so what is eventually
|
itself silent: the window then runs from that arming poll rather than from the
|
||||||
reported is a conflict still standing two minutes after that fresh merge
|
falses that predate it, so what is eventually reported is a conflict that
|
||||||
computation began, never the run of falses that preceded it.
|
outlasted the merge computation the move started.
|
||||||
|
|
||||||
|
Arming is a one-way latch. Moves after that one — and every move seen by a watch
|
||||||
|
that was already armed at the baseline — neither re-arm nor restart the window,
|
||||||
|
because a restart on every `base.sha` move could never complete on a busy base
|
||||||
|
branch. That is the cost side of the same trade: on a busy base branch a
|
||||||
|
conflict can be confirmed while the newest merge recompute is less than two
|
||||||
|
minutes old. The window guarantees that the run of falses outlasted *a* merge
|
||||||
|
computation this watch saw start, not that it outlasted the most recent one.
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
# Watch until something meaningful happens (default interval 60s)
|
# Watch until something meaningful happens (default interval 60s)
|
||||||
|
|||||||
+12
-6
@@ -8,8 +8,12 @@
|
|||||||
// so. A conflict introduced by the push immediately before the watch started is
|
// so. A conflict introduced by the push immediately before the watch started is
|
||||||
// not reported while the commits stay put, because nothing in Gitea's payload
|
// not reported while the commits stay put, because nothing in Gitea's payload
|
||||||
// separates it from a merge check still in flight; the baseline line is the only
|
// separates it from a merge check still in flight; the baseline line is the only
|
||||||
// notice of it. A later move of the head or base arms the rule without alerting,
|
// notice of it. The first later move of the head or base arms the rule without
|
||||||
// and restarts the two-minute window from itself.
|
// alerting, and the window then runs from that arm rather than from the falses
|
||||||
|
// that predate it. Moves after that neither re-arm nor restart the window -- a
|
||||||
|
// restart on every base move could never complete on a busy base branch -- so a
|
||||||
|
// conflict can be confirmed while the newest merge recompute is less than two
|
||||||
|
// minutes old.
|
||||||
//
|
//
|
||||||
// watchpr owner/repo#12 owner/repo:15
|
// watchpr owner/repo#12 owner/repo:15
|
||||||
// watchpr --once --json owner/repo#12
|
// watchpr --once --json owner/repo#12
|
||||||
@@ -108,9 +112,11 @@ func clientFor(jsonMode bool) *agent.GiteaClient {
|
|||||||
return agent.NewGiteaClient(token)
|
return agent.NewGiteaClient(token)
|
||||||
}
|
}
|
||||||
|
|
||||||
// warnRecord is the --json form of a warning. Under --json stderr carries
|
// warnRecord is the --json form of a warning. Under --json every notice watchpr
|
||||||
// nothing but NDJSON records, so a caller parsing it line by line never has to
|
// itself writes to stderr -- warnings and the baseline -- is an NDJSON record,
|
||||||
// guess which shape a line is.
|
// so a caller parsing them line by line never has to guess which shape a line
|
||||||
|
// is. A terminal failure is the one exception: SilenceErrors stays off, so
|
||||||
|
// cobra prints it as a plain "Error: ..." line and the exit status is non-zero.
|
||||||
type warnRecord struct {
|
type warnRecord struct {
|
||||||
Warning string `json:"warning"`
|
Warning string `json:"warning"`
|
||||||
}
|
}
|
||||||
@@ -255,7 +261,7 @@ func suppressedAtBaseline(st agent.PRState) string {
|
|||||||
case agent.MergeNo:
|
case agent.MergeNo:
|
||||||
conds = append(conds, "already non-mergeable (a real conflict, or Gitea still recomputing)")
|
conds = append(conds, "already non-mergeable (a real conflict, or Gitea still recomputing)")
|
||||||
case agent.MergeUnknown:
|
case agent.MergeUnknown:
|
||||||
conds = append(conds, "mergeability unknown (the conflict rule is disarmed until Gitea answers or the commits move)")
|
conds = append(conds, "mergeability unknown (the conflict rule is disarmed until Gitea reports mergeable or the commits move)")
|
||||||
}
|
}
|
||||||
if st.CIStatus == "failure" || st.CIStatus == "error" {
|
if st.CIStatus == "failure" || st.CIStatus == "error" {
|
||||||
conds = append(conds, "CI already "+st.CIStatus)
|
conds = append(conds, "CI already "+st.CIStatus)
|
||||||
|
|||||||
@@ -257,10 +257,12 @@ func TestBaselineIsEmittedInJSONMode(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Under --json stderr is the baseline and warning channel, so it has to be one
|
// Under --json stderr is the baseline and warning channel, so every notice
|
||||||
// shape: a caller parsing it line by line must never meet a bare `warning:`
|
// watchpr writes there has to be one shape: a caller parsing it line by line
|
||||||
// line between two NDJSON records.
|
// must never meet a bare `warning:` line between two NDJSON records. Cobra's
|
||||||
func TestJSONModeStderrIsAllRecords(t *testing.T) {
|
// terminal `Error: ...` line is not covered here -- SilenceErrors stays off, so
|
||||||
|
// it is plain text on stderr alongside a non-zero exit.
|
||||||
|
func TestJSONModeWarningsAndBaselineAreRecords(t *testing.T) {
|
||||||
st := agent.PRState{
|
st := agent.PRState{
|
||||||
Ref: agent.PRRef{Owner: "unkin", Repo: "repo", Number: 7},
|
Ref: agent.PRRef{Owner: "unkin", Repo: "repo", Number: 7},
|
||||||
State: "open",
|
State: "open",
|
||||||
|
|||||||
Reference in New Issue
Block a user