Repository navigation
A run stranded at merge_requested can record what it observes - #105
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
The second wave after the closed loop deployed got one state further and stopped again, for the same reason in a different place.
A run stranded at
merge_requested— a pass that died between the merge request and the merge — is resumed by the loop and derived. It now finds its pull request (#104) and sees it merged. It then tries to record that:So
latest_pr_statestays unobserved, the ending rule reads that as open and refuses the terminal fact, and the run comes back on the next wave to do it all again. The loop resumes from four states but could only record an observation from three.This adds
merge_requestedto that from-set, for the reason the same table already gives forplanned: a CI observation is a fact about the world, not about the phase the run is in, and the ending rule depends on its being writable from every state the loop resumes at. The authorisation for a merge happened atrequest_merge, which is already past, so nothing here can retroactively authorise anything.To stop the from-set and the resume set drifting apart a third time, the four states are now a named constant,
BACK_HALF_DERIVING_STATES, with a test asserting the from-set covers it.Effect on the merge gate
None.
_ci_is_greenis consulted when a merge is authorised, atrequest_merge; these observations are recorded after that point, on a run whose merge was already authorised or already done. No change to_merge_gate,merge_ready_pr, the kernel's merge authorisation, or what evidence any of them require.Testing
bash batch/run-queue.sh --self-testpasses on LinuxPython suite 1990 passed, 3 skipped. No bash changed; CI ran the self-test on Linux for this branch and it passed (6m16s).
Three tests: an observation is accepted from
merge_requested; a run there whose pull request merged can then end; and the from-set covers every state the loop derives at. The first two fail without the change — they are the live case above, reduced. The third is the one that matters longest: it fails if anyone adds a resume state without widening the from-set.Anything a reviewer should look at twice
mergedandcancelledbelong in the set too. They do not:_resume_back_halfintercepts both before the step loop runs, so no observation is ever attempted there, and the ending rule does not apply to them anyway.planned, thenmerge_requested). Both were invisible to every test and every review, and both surfaced on the first waves after deploy, which is an argument for watching the next few waves rather than trusting the suite.