Repository navigation
ci(hostlock): drop the assertion count that was already wrong twice, and say why this must never be required - #1917
Merged
Conversation
justinchuby
enabled auto-merge (squash)
August 24, 2026 01:32
…and say why this must never be required Three corrections to #1912, all found in review of it. The comment said "252 assertions (249 before #1910)". That was a projection, not a count: 249 was the branch total and +3 was a PR that had not landed. Neither number was ever the total on main, which went 249 -> 258 (#1908) -> 265 (#1869) while #1912 sat in review. The suite already pins its own total in its last assertion, `every assertion in this file ran`, which is the only copy that cannot drift because it fails the run rather than misinforming a reader. Same defect this lock exists to catch, one level down: a figure published where nothing can falsify it. The second is load-bearing. This job is non-required by choice, but with `pull_request: paths:` a PR that does not touch these files gets NO job rather than a skipped one, so promoting it to a required check would leave every unrelated PR sitting on "Expected -- waiting for status" forever. Recorded next to the trigger so the next person to reach for the branch-protection settings reads it first. The third is that my own explanation of the escape hatch was wrong, in the same commit that complains about unchecked claims. I wrote that `ci.yml` "documents the mirror image of that trap on its `changes` job, which uses a job-level `if:`". `changes` has no `if:` at all, deliberately -- its comment records that gating it once skipped the entire workflow, because a job whose `needs` dependency is skipped is skipped whatever its own `if:` says. The job-level `if: needs.changes.outputs.docs_only != 'true'` is on the two REQUIRED jobs, and it works because a job skipped by a conditional still reports a conclusion. Workflow-level `paths:` cannot be rescued that way: there is no job to skip, so there is no conclusion to report. Also records the first real run of this job on a GitHub-hosted runner, 3m17s, against the 210s two-core emulation that sized the timeout. The emulation predicted the real number, which is the reason to keep both rather than replace one with the other. Comment-only. No behaviour change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
force-pushed
the
squad/leon-hostlock-ci-caveat
branch
from
August 24, 2026 02:24
2d6634f to
4e02233
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1917 +/- ##
==========================================
+ Coverage 80.14% 80.71% +0.56%
==========================================
Files 413 415 +2
Lines 200822 204833 +4011
Branches 200822 204833 +4011
==========================================
+ Hits 160957 165339 +4382
+ Misses 34343 33914 -429
- Partials 5522 5580 +58
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
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
1 participant
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.
Two corrections to #1912, both raised in review of it.
1. The assertion count was already wrong twice
The comment said "252 assertions". #1908 added nine to the same file before #1912 merged, and #1910 adds three more — so the number was wrong in review and wrong again on merge. A count in a comment is a number nobody re-measures.
The suite already pins its own total in its last assertion —
every assertion in this file ran— which is the only copy of that number that cannot drift, because it fails the run rather than misinforming a reader. This is the same defect the lock exists to catch, one level down: a figure published somewhere nothing can falsify it.The timing evidence keeps its number but now says which version produced it, since that one was measured.
2. This job must never be promoted to a required check
Non-required was a cost decision in #1912; there is a harder reason, and it belongs next to the trigger.
With
pull_request: paths:, a PR that does not touch these files does not get askippedjob — it gets no job at all. A required check by this name would therefore sitExpected — waiting for statuson every unrelated PR, forever.ci.ymldocuments the mirror image of that trap on itschangesjob, which uses a job-levelif:precisely so it still reports a conclusion when there is nothing to do. Recorded here so the next person to reach for the branch-protection settings reads it before flipping the switch.Validation
Comment-only; the
on:,jobs:and step definitions are byte-identical. YAML parses. No behaviour change, so this PR's own run of the job is the regression test.