Repository navigation
fix(cli): a check that hits its deadline keeps the findings it already made - #6190
Merged
Merged
Conversation
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Verdict: APPROVE · 0 blocker · 0 important · 3 nits · head 5cae478e
Partial results are read lazily: the grid closure reads perTime, and helper-dropped samples are deleted before they're re-queued. skipped stays true when nothing was kept. I ran the new test: it fails with the old catch and passes at this head. The check, deadline and browser suites pass locally (181 tests). CI was still running when I posted and had no failures yet.
Nits. Each one only shows up on the deadline path:
checkPipeline.ts:1667: a partial report clearsbrowserSkipped, so the "empty placeholders, not a clean pass" warning goes away. Motion and craft are still empty with nothing marking them as unrun (motion.samples: 0). The deadline error keepsokfalse, but a JSON reader could take an empty Motion section as clean. Apartial: trueflag, or keeping the warning for unreached sections, would close that.checkBrowser.ts:316: the partial runtime findings skipdropMotionBlurConsoleEcho. If the deadline fires aftercollectMotionBlurDrafts, the report shows the[motion-blur] off:console warning alongside the blur finding.- Under
--component, partial findings never getcomponentMatch. The filter at:2019keepsundefined, so a filtered run that hits the deadline shows unfiltered layout and contrast findings.
Contributor
Edit accuracy: accurate 271 (base branch 271), smooth 114 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (1)
|
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 changes for you
When
checkhits its deadline (no progress for 120 s), the report now keeps what it had already found: the layout and contrast findings from the samples that finished, and the page's own console and network findings. The deadline error is still in the report and still names the step it stopped at, for example "while running the overlap pass", with the seconds spent per step. Before, all of that was dropped and the report held the deadline error alone.Why
A large project can run
checkfor a long time. When the deadline fired, the work of every finished sample was thrown away, so the person got no result at all.How
CheckDeadline.keepPartiallets a stage register how to read what it has so far. The sample grid registers its finished samples, and the browser step registers the runtime findings. When the check fails, the pipeline builds the report from those instead of an empty result, andbrowserSkippedis false when anything was kept.Tests
A deadline during the overlap pass keeps the layout finding from the finished samples, marks the browser as not skipped, and still carries the deadline finding. The test fails without the change. The check, deadline, browser and progress suites pass 3 runs in a row (182 tests).
What I did not do
This covers the CLI's own deadline. A caller that kills
checkfrom outside after its own wall-clock limit gets no report, because the process never gets to write one. I could not reproduce a slowcheck: a 40-minute project with about 1,760 elements finished in 43 s. The step that is slow on the reporter's project is still unknown.PR_SIZE_OK: an 88-line change in one concern.