The timing history stopped recording when prebuild was added - #89
Merged
Merged
Conversation
`timings` has been skipped on every run of every consumer since #36. The suite is green, the shards pass, and the history it plans the next run from has not moved since 2026-07-28. Bisected to one commit. `ac87a88` recorded; `f3681d7` (#36) did not; nothing between them. That commit does not touch this job — it is byte-identical across the boundary — it adds `prebuild` and gives `test` `needs: [labels, prebuild]`. The condition is not what fails. On a push to the default branch all three of its own conjuncts are true, and `needs.test.result == 'success'` is provable from the same run: `gate` reads that value and exits 1 if it is anything else, and `gate` is green. A job whose condition is true and which is skipped anyway was not skipped by its condition. It inherited the skip: `prebuild` is off by default, so it is skipped on essentially every run, and a job with no status-check function in its `if` inherits that through the graph even though `test` ran and passed. The two siblings that kept working are the two that carry one. `collect` has `always() && ...`, `gate` has `always()`, `timings` had none. This gives it `!cancelled()`, which is the weaker of the two and enough: `needs.test.result == 'success'` still gates on the shards actually passing, so a red suite records nothing, as before. Not verified by running it — I have no way to dispatch a push to a default branch here. What is verified is the bisection, that the job is byte-identical across it, and that `needs.test.result` is `success` on the runs where this skips.
Contributor
|
📚 Docs preview: https://codes.sota-shimozono.com/TestShards.jl/previews/PR89/ (updates on each push to this PR) |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
A behaviour fix in the shipped workflow, which is this package's product. Patch: nothing is removed, renamed, or changed in meaning — a job that was silently skipped now runs when its condition says it should.
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.
timingshas been skipped on every run of every consumer since #36. Suites are green, shards pass, and the history the next run is planned from has not moved.ci-timingslast writtenBisection
ac87a88(2026-07-28 08:33) recorded.f3681d7(2026-07-28 10:17, #36) did not. Exactly one commit between them, and that run was a push tomainwith all eight shards green andAll shards passedgreen.That commit does not touch this job —
timingsis byte-identical across the boundary. It addsprebuildand changestesttoneeds: [labels, prebuild]with anif:.The condition is not what fails
On a push to the default branch,
github.event_name,github.refanddefault_branchare all trivially true. Andneeds.test.result == 'success'is provable from the same runs:gatereads that exact value andexit 1s if it is anything else — andgateis green on every run wheretimingsskipped.A job whose condition is true and which is skipped anyway was not skipped by its condition. It inherited the skip.
prebuildis off by default, so it is skipped on essentially every run, and a job with no status-check function in itsifinherits that through the graph even thoughtestitself ran and passed.The two siblings that kept working are the two that carry one:
collectalways() && …gatealways()timingsThe change
!cancelled(), the weaker of the two, is enough —needs.test.result == 'success'still gates on the shards actually passing, so a red suite records nothing, exactly as before. This makes the job consistent with the other two in the same file.What is and is not verified
Verified: the bisection, that the job is byte-identical across it, that
needs.test.resultissuccesson the runs where it skips, and the fleet-wide dates above.Not verified: I could not run it. Confirming needs one push to a default branch — this PR's own merge to
mainis that test.A separate observation, not in this PR
labelsreads the history once and fans it out as an artifact (#53's fix for the seven-shards-agree-one-doesn't split, and for the force-push race). That upload iscontinue-on-errorbecause a full artifact quota once turned every consumer red before a test ran (#74).labelsis not a matrix job, so it could carry the history as a job output instead: measured across the fleet the file is 401 B – 17.9 kB (QAtlas.jl, 325 rows), against a 1 MB per-output limit — ~57x headroom at today's largest. That would remove the read-side artifact, its download in each shard, and theneeds.labels.outputs.timings == 'success'conditional, and with them the quota failure mode on that path. The write side genuinely needs artifacts:testis a matrix job and matrix legs overwrite each other's outputs.Happy to open that separately if it is wanted; keeping it out of a bug fix.