Skip to content

Keymap discovery runs its fresh-interpreter case in a new interpreter - #1593

Merged
trissim merged 2 commits into
mainfrom
fix/keymap-discovery-fresh-interpreter
Nov 6, 2026
Merged

trissim merged 2 commits into
mainfrom
fix/keymap-discovery-fresh-interpreter

Conversation

@trissim

@trissim trissim commented Nov 6, 2026

Copy link
Copy Markdown

Regression from toad#1592 (mine). The app test template now imports every toad module before it forks. That means the forked child test_unopened_screen_invocations_are_discovered_in_a_fresh_process runs in already holds toad.screens.welcome, so its first assertion (tests/textual/test_keymap_discovery.py:11) failed before discovery ran. The full tests/textual run on 8 workers after #1592 shows it as the one new red.

Fix: the case is about an interpreter that has never imported the screen, so it now starts one. It's a new sys.executable with this case's workspace roots, and it asserts the same four facts: welcome isn't imported, capture imports it, NewSessionAction's title matches, and its defaults match.

tests/textual/test_keymap_discovery.py: 1 passed (10.1 s, including the template's one-time warm-up).

🤖 Generated with Claude Code

https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd

toad#1592 made the app test template import every toad module before it forks, so the
forked child this case ran in already held toad.screens.welcome and its first assertion
failed before discovery ran. The case is about an interpreter that never imported the
screen, so it now runs in one: a new interpreter with this case's roots, asserting the
same discovery facts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
Copilot AI balanced review requested due to automatic review settings November 6, 2026 19:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

The check was a python -c string, so a rename of KeymapCatalog.capture would miss it and no
linter or type checker read it; it is now tests/support/keymap_discovery.py. The path insert
restated the editable install the test environment already provides, and is deleted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim
trissim enabled auto-merge November 6, 2026 19:12
@trissim
trissim merged commit 19bad38 into main Nov 6, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants