Repository navigation
Fix the red CI: make the render tests independent of the screen's pixel scale - #5
Open
jonnilundy wants to merge 3 commits into
Open
jonnilundy wants to merge 3 commits into
jonnilundy wants to merge 3 commits into
Conversation
CI has been red since the first run: 81 failures on a 1x runner, none on a 2x Mac. Cause. CodeBlockRenderTests measured the drawn pixels of a bitmap made with bitmapImageRepForCachingDisplay, which takes the host screen's scale. Its ink and gap thresholds were tuned on a 2x display, so a 1x host drew a quarter of the pixels: 30 and 21 against "greater than 40", and a box gap of 7.5 against 8.0. ShellViewTests made its fixture PNG with NSImage.lockFocus, which is also screen scaled: 64 by 32 pixels on 1x, and the test expects the 128 by 64 it gets on 2x. Fix. The code-block capture renders into an explicit 2x bitmap. The fixture PNG is an explicit 128 by 64 pixel bitmap. Neither depends on the host screen now. Evidence. Forcing the old capture to a 1x bitmap on a 2x Mac reproduces the CI numbers exactly: 80 failures (the 7.5 against 8.0 gap and the 30 and 21 ink counts) plus the image test make the 81 seen on CI. With the fix the same tests pass at native scale (10 tests, 1362 checks) and the image test passes (16 checks). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
After the scale fix CI had one failure left: testCodeGetsNoSubstitutions. It types text and expects macOS text substitution to rewrite prose and leave code alone. The CI runner has no text checking service, so nothing is substituted anywhere. Prose failed its own control, and the code cases would only pass because nothing changes. Prose is now a precondition: when it is not substituted the test skips with the typed text in the reason. On a Mac with the service the test runs as before (10 tests, 1361 checks pass). A host with substitution off skips instead of failing. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
testViewsInAWindowInvalidateTheirRectsWhenTheyMoveOrHide closes a window it created in code. NSWindow releases itself on close by default, and ARC releases it again, so the process dies. Every other test that closes a window sets isReleasedWhenClosed = false. The crash is intermittent: the test passes on a Mac most runs, and the FloStateNativeTests process died inside it on the CI runner (a bare "TESTS FAIL: exit 1", the log ends at "started"). It was hidden in the first PR run behind the earlier failure. Evidence. The old test body run 400 times in one process crashes at once (exit 139). With isReleasedWhenClosed = false it runs 400 times without a crash. FindTests is the only other test that makes a window without the flag, and it never closes it. Co-Authored-By: Claude Sonnet 5.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.
What was broken
CI has failed on every run: 81 failures on the 1x GitHub runner, none on a 2x Mac. Five tests: four in
CodeBlockRenderTests(80 failed checks) andShellViewTests.testPDFAndImageOpenInViewerTabs(1).Root cause
Both measured things drawn at the host screen's pixel scale, and the expected values assume 2x.
CodeBlockRenderTests.captureusedbitmapImageRepForCachingDisplay, which follows the screen's scale. The ink and gap thresholds were tuned on a 2x display. A 1x host draws a quarter of the pixels, so "30 is not greater than 40" and the box gap reads 7.5 against 8.0.ShellViewTestsbuilt its fixture PNG withNSImage.lockFocus, also screen scaled. It is 64x32 pixels on 1x. The test expects the 128x64 it gets on 2x.Fix
The code-block capture renders into an explicit 2x bitmap. The fixture is an explicit 128x64 pixel bitmap. Test code only.
Evidence
7.5 against 8.0gap failures and the30and21ink counts. The image test is the 81st.Not yet verified
The real proof is this PR's CI run on the 1x runner. The fix makes the tests independent of the host scale, but I cannot create a 1x host locally.
Other tests also use
bitmapImageRepForCachingDisplay(ImageResizeTests,FindTests,LocalizationTests). They pass on CI, so I left them alone.🤖 Generated with Claude Code