GH-3816: fail the build when a CI target runs nowhere - #4037
Merged
Conversation
SlowTests existed, was maintained, and was cited in issue write-ups while
running in ZERO CI jobs. Nothing reported that. This is the guard for it.
The reason the issue asked for a second attempt is the shape of the obvious
version: diffing declared targets against names grepped out of the workflow
files reports CIMessageRouting on its FIRST run -- named in no workflow, yet
running on every push via `Target CI => .DependsOn(CoreTests, CIMessageRouting)`
and dotnet.yml's `./build.sh ci`. A guard that cries wolf immediately is how a
signal channel gets ignored, which is the failure this whole reporting effort
exists to undo.
So this walks the real graph instead. Nuke's own ExecutableTarget carries
ExecutionDependencies, Triggers and TriggerDependencies, which is the actual
answer to "if CI runs, what runs with it" rather than a re-derivation of it
from source text. TriggeredBy is recorded on the triggered target, so that edge
is inverted before the walk. NukeBuild.ExecutableTargets is internal, so it is
read by reflection -- deliberately, and it throws if that ever moves rather than
quietly reporting everything as covered.
Two things beyond a plain reachability check, both needed to keep it honest:
- Target names are matched case-insensitively, because dotnet.yml invokes
`./build.sh ci` in lower case and that is the root most of the graph hangs
from. A case-sensitive match loses almost everything.
- A target with no actions of its own whose dependencies are all covered is
itself covered. CIAWS is exactly this -- `.DependsOn(CIAWSSqs,
CIAWSSqsCompliance, CIAWSSns)`, invoked by no workflow, running nothing its
dependencies do not already run. Without this rule it is reported, and that
report would be wrong.
Verified in three directions, because a guard that passes proves nothing until
it has been seen to fail:
1. Clean on the repo as it stands: 43 invoked targets reach 64, 40 CI*
targets declared, no orphans. No day-one false positive.
2. A scratch `Target CIScratchOrphan` that no workflow names IS reported, and
fails the build.
3. With the aggregate rule disabled, CIAWS is reported -- so that rule is
load-bearing, not decorative.
Wired into `Target CI` rather than tests.yml: it costs under a second, and
dotnet.yml runs on every push and PR, so an orphan is caught on the cheap
workflow instead of behind the 20 minute matrix. Confirmed reachable from
`./build.sh ci`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Closes #3816.
SlowTestsexisted, was maintained, and was cited in issue write-ups while running in zero CI jobs. Nothing anywhere reported that. This is the guard.Why it walks the graph
The issue was filed because the obvious version doesn't work. Diffing declared targets against names grepped out of the workflow files reports
CIMessageRoutingon its first run — named in no workflow, yet running on every push viaTarget CI => .DependsOn(CoreTests, CIMessageRouting)anddotnet.yml's./build.sh ci. A guard that cries wolf immediately is how a signal channel gets ignored, which is the exact failure this reporting effort exists to undo.So it asks Nuke instead of re-deriving the graph from source text.
ExecutableTargetcarriesExecutionDependencies,TriggersandTriggerDependencies, which is the real answer to "ifCIruns, what runs with it".TriggeredByis recorded on the triggered target, so that edge is inverted before the walk.NukeBuild.ExecutableTargetsis internal, so it is read by reflection. That is deliberate — the alternative is parsingDependsOnout of the source, which is the brittleness the issue was filed about — and it throws if the API ever moves, rather than quietly reporting everything as covered.Two rules beyond plain reachability
Both were needed to make it accurate, and both are load-bearing:
Case-insensitive target matching.
dotnet.ymlinvokes./build.sh ciin lower case, and Nuke resolves names case-insensitively. A case-sensitive match loses the root that most of the graph hangs from.A pure aggregate whose dependencies are all covered is covered.
CIAWSis exactly that —.DependsOn(CIAWSSqs, CIAWSSqsCompliance, CIAWSSns), invoked by no workflow, with no actions of its own, so it can run nothing its dependencies do not already run. Without this rule it gets reported, and that report would be wrong.Verified in three directions
A guard that passes proves nothing until it has been seen to fail.
CI*targets declared; no orphans. No day-one false positive, which was the issue's central requirement.Target CIScratchOrphanthat no workflow names is reported by name and fails the build:CIAWSis reported — reproducing the false positive on demand.Where it runs
Wired into
Target CIrather thantests.yml. It costs under a second, anddotnet.ymlruns on every push and PR, so an orphan is caught on the cheap workflow rather than behind the 20-minute matrix. Confirmed reachable:./build.sh ciexecutes it.On the allow-list
DeliberatelyManualTargetsships empty, and should stay that way — an entry is a claim that a target is meant to be manual and needs a reason beside it.CISlowTestsis deliberately not in it: it isworkflow_dispatch-only, butslow-tests.ymlnames it, so the walk finds it honestly.Also guarded: if no workflow names any known target at all — a moved or renamed workflow directory — it throws saying so, rather than reporting every target as orphaned.
🤖 Generated with Claude Code