Repository navigation
Prepare Motus CI for a merge queue - #42
Merged
Merged
Conversation
Turning on a merge queue means requiring checks, and the workflow could not support that as it stood. Its trigger-level path filters meant a pull request that touched only docs never got the checks, so it could never merge or enter the queue, and the merge_group event takes no path filter at all. Requiring the per-OS ".NET (...)" checks would also fail on its own: a matrix job skipped at the job level reports one check under its unexpanded name, so the expanded names never arrive. The workflow now runs on push, pull_request and merge_group alike. A small first job lists the changed files with plain git, against the range each event carries, and the build and test jobs run only when the code paths the filters used to name were touched. Anything it cannot decide runs everything. A single CI job runs last on every event and passes only when every other job succeeded or was skipped, which makes it the one check to require. Signed-off-by: Torrey Betts <torrey.betts@gmail.com>
SummarySummary
CoverageMotus - 80.9%
Motus.Abstractions - 87.7%
Motus.Analyzers - 91%
Motus.Cli - 82.2%
Motus.Codegen - 89.8%
Motus.Mcp - 91.9%
Motus.Mcp.Http - 98.6%
Motus.Recorder - 77.8%
Motus.Testing - 75.4%
Motus.Testing.MSTest - 79%
Motus.Testing.MSTest.v4 - 22.8%
Motus.Testing.xUnit - 63.6%
|
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.
We want to turn on a merge queue for main, which means requiring status checks. The CI workflow couldn't support that as it stood, for two reasons.
First, its
pushandpull_requesttriggers carriedpathsfilters. Once checks are required, a pull request that touches only docs would never get them, so it could never merge or enter the queue. Andmerge_grouptakes no path filter at all, so the queue's temporary branches would run everything or nothing.Second, the obvious checks to require,
.NET (ubuntu-latest)and its siblings, don't survive a skip. A matrix job skipped at the job level reports a single check under its unexpanded name,.NET (${{ matrix.os }}), so the per-OS names would never report and every skipping change would wait forever.The change
All of this is in
.github/workflows/motus-ci.yml.motus-release.ymlis untouched.pushandpull_request(both on main) keep their branch filters and lose their path filters.merge_group(types: [checks_requested]) is added so CI runs on the queue's temporary branches. Those branches aregh-readonly-queue/main/..., which thepushfilter doesn't match, so a queued change isn't built twice for the same commit.Changesjob decides what runs. It checks out full history withfilter: blob:none(commits and trees only, which is all a list of paths needs) and runsgit diff --name-only --no-renames base...headover the same paths the filters named:src,tests,samples,Motus.slnand the workflow file. The range depends on the event: a pull request usespull_request.base.sha...head.sha, a merge group usesmerge_group.base_sha...head_sha, and a push usesbefore...after. The three-dot form compares against the merge base, so commits that landed on main in the meantime aren't counted as the change's own. Renames are listed on both sides, so a file moved out of a code path still counts. If there's no range (an all-zerobefore, an unknown event) or the diff fails (abeforelost to a force push), it runs everything: a needless run costs minutes, but a wrongly skipped one could let a break through. It's plain git with no third-party action.dotnetnowneeds: changesand runs only whenneeds.changes.outputs.code == 'true'. The two integration jobs already needdotnet, so they skip with it. Apart from that, the existing jobs, steps, timeouts, the pinnedCHROME_VERSIONand the coverage steps are unchanged.CI. It runs last withif: always(), needschanges,dotnet,integrationandintegration-firefox, and passes only when each of them issuccessorskipped. It prints every result and fails onfailureorcancelled. That includes a failure inChangesitself, which would otherwise skip everything downstream and look like a pass.I picked the aggregate job over the alternatives because it's the only shape where one stable check name covers a skipping matrix. Requiring every per-OS check breaks on the unexpanded-name trap described above. Adding "always report" stub jobs under the same names doubles the job list, and it has to be kept in step every time the matrix changes. With the aggregate job, adding or removing a matrix leg, or a whole job, means editing its
needslist and nothing in the ruleset.github.event_nameconditions: the only one is the coverage summary step, which posts a sticky comment only onpull_request. That's still right for a merge group: there's no single pull request to comment on, and the comment already went up on the PR's own run. On a merge group the coverage report is still built and uploaded as an artifact.How it behaves
Changesfinds no code paths and reportscode=false, the three build and test jobs show as skipped, andCIpasses in well under a minute. The PR can merge or enter the queue.CIgoes green only if they all pass.CIreports on the group's head commit, which is what the queue waits for.before...afterand runs the suite if code changed, as the path filter did.For the ruleset
Require one status check:
CI. It's best pinned to the GitHub Actions app as its source, so nothing else can report a check of that name. Don't requireChanges,.NET (...)or the integration checks individually. Themerge_grouptrigger is already in place, so no other workflow change is needed for the queue. There's no existing ruleset or branch protection on main today, so nothing old needs removing.Tests
actionlint1.7.7 over both workflow files: clean.Changesscript locally against this repo's history: a docs-only commit givescode=false, a range touchingtests/givescode=trueand lists the files, an all-zerobeforeand an unknown event run everything, and an unreachablebeforefails the diff and runs everything.Changesreportscode=true, the full suite runs, andCIaggregates the results. The docs-only and merge-group paths can't be exercised until a docs PR is opened and the queue is turned on.