Bound the recent-files menu, and stop a test resolving a path two ways - #195
Merged
Merged
Conversation
Two headless editor tests failed on macOS and were made Linux-only rather than diagnosed. Neither was about macOS. MenuTests.OpeningARecentFileLoadsIt clicked a recent file whose menu label was its whole path. A menu is as wide as its widest label, and a submenu that will not fit beside the menu that opened it is placed on top of that menu - which puts the item that opened it under the submenu rather than under the pointer, so ImGui takes the pointer to have left and closes the submenu on the frame after it opens. The item was therefore drawn exactly once and never again, and the click landed on a stale rectangle. macOS only picked itself out by reaching its temporary directory through a path 44 characters deeper than /tmp; the same thing happened on Linux with TMPDIR set to a macOS-shaped path, and it happened to any user who had saved a schema somewhere deep enough, on any platform, leaving Open Recent unusable for them. So the label is elided to a width that fits, the whole path stays as the item's id and in a tooltip, and forty characters is measured rather than chosen: the submenu grows about eight pixels a character, which leaves it inside the 700-pixel window the narrowest of these tests drives, where sixty does not. FileBrowserTests.SavingRecordsTheChosenPathAsRecentlyUsed was a different failure that happened to share a menu - an assertion, not the stale-item exception. It expected the name it created its scratch directory under, but macOS reaches the temporary directory through /var, a symbolic link to /private/var, and the working directory resolves the link, so the browser answered with the other spelling of the same directory. It now reads the working directory back after setting it, which reproduces and fixes under a symlinked TMPDIR on Linux too. EditorHarness.Click asked whether the probe had ever recorded a name rather than whether the item was on screen, so an item drawn once and then gone still satisfied the wait. It now asks IsOnScreen on both sides of the settle frames. That is a better message rather than a fix: with the wrong question the failure was inside the click, with the right one it is at the wait, and only the elision makes it pass. The elision is pinned frameless in Schema.Editor.Test, so the one platform-sensitive fact here is covered everywhere the frame-driven suite cannot run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0131bjn9xRwnWMQB2yJFP1cN
|
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
2 participants
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 headless editor tests failed on macOS and were made Linux-only rather than diagnosed. Neither was about macOS, and they were two different failures that happened to share a menu — the CI log shows one stale-item exception and one plain assertion, not the same one twice.
Both now reproduce on Linux, which is how they were diagnosed.
MenuTests.OpeningARecentFileLoadsItA menu is as wide as its widest label, and Open Recent's labels were absolute paths. A submenu that will not fit beside the menu that opened it is placed on top of that menu — so the item that opened it sits under the submenu rather than under the pointer, ImGui takes the pointer to have left, and the submenu closes on the frame after it opens. Instrumenting the probe frame by frame shows the item drawn on exactly one frame and never again; the click then landed on a stale rectangle.
macOS only picked itself out because it reaches its temporary directory through a path 44 characters deeper than
/tmp. The same failure, byte-identical, appears on Linux withand it applied to any user who had saved a schema somewhere deep enough, on any platform — Open Recent was simply unusable for them.
The fix is in the editor, not the test: the label is elided, the whole path stays as the item's ImGui id and moves into a tooltip. Forty characters is measured rather than chosen — the submenu grows about eight pixels a character, which leaves it inside the 700-pixel window the narrowest of these tests drives, where sixty still overflows (verified both ways).
FileBrowserTests.SavingRecordsTheChosenPathAsRecentlyUsedA different failure: an assertion,
/var/folders/…expected against/private/var/folders/…actual. macOS reaches the temporary directory through/var, a symbolic link to/private/var, andDirectory.SetCurrentDirectoryresolves the link — so the browser, which opens on the working directory, answered with the other spelling of the same directory. The test now reads the working directory back after setting it. Reproduced and confirmed fixed under a symlinkedTMPDIRon Linux.EditorHarness.ClickIt asked whether the probe had ever recorded a name, not whether the item was on screen — the thing
IsOnScreen's own doc comment warns about. It now asksIsOnScreenon both sides of the settle frames.This was the standing prime suspect, and it is not the fix: with the wrong question the failure was inside the click on a stale rectangle, with the right one it is at the wait, and only the elision makes it pass. Kept because the message is better and the question is the correct one. All 189 existing frame tests still pass with it.
Coverage
MenuTests.ARecentFileTooDeepToLabelWholeStillOpenspins the mechanism — verified to fail without the elision and pass with it.Schema.Editor.Test/RecentFileLabelTests.cspins the elision itself. It needs no frame, so it is in the suite that runs on every platform. That is the answer to whether the frame-driven suite should be promoted: what was platform-sensitive here was a string, and a string can be tested where the rasterizer cannot go. The workflow is untouched.Verification
Schema.Editor.UITests(defaultTMPDIR)Schema.Editor.UITests(macOS-lengthTMPDIR)Schema.Editor.UITests(symlinkedTMPDIR)Schema.TestSchema.Cpp.TestSchema.Editor.TestRelease build, run through the produced executables.
🤖 Generated with Claude Code
https://claude.ai/code/session_0131bjn9xRwnWMQB2yJFP1cN
Generated by Claude Code