Skip to content

CI runs the tests again, and the 19 they hid are fixed (4 were real bugs) - #255

Open
trissim wants to merge 27 commits into
mainfrom
ci/drop-unprovisionable-3.9
Open

CI runs the tests again, and the 19 they hid are fixed (4 were real bugs)#255
trissim wants to merge 27 commits into
mainfrom
ci/drop-unprovisionable-3.9

Conversation

@trissim

@trissim trissim commented Nov 6, 2026 •
edited
Loading

Copy link
Copy Markdown

Textual CI had not run a single test on any PR: ubuntu-latest (26.04) has no Python 3.9, setup failed, and fail-fast cancelled every other build. Under that cover 19 tests went red on main. This PR makes CI run the tests and fixes all 19 (local, without snapshots: 3849 passed, 0 failed), plus two checks that could never pass.

In numbers: 15 commits. 19 failures → 0: 15 stale tests, 4 real source bugs. 2 dead workflows deleted. Every failure was bisected to the commit that caused it before deciding whether the test or the source was wrong.

CI runs the Pythons the code runs on

  • The matrix is 3.12, 3.13, 3.14. The tree uses 3.12 syntax (type X = at box_model.py:30, def of[Value] at css/native.py:99), so 3.10 and 3.11 cannot even import it.
  • test_message_inheritance_namespace asserted the delivery order between two pumps, which is scheduling. It now asserts the namespaces.
  • Deleted debt-ratchet.yml: it installs agent-comms at 88f4e6ee, which crashes at import on 3.14.10, and agent-comms main no longer ships agent-comms-ratchet (the NRA gate replaced it).
  • Deleted black_format.yml: Python 3.11 plus black 24.1.1 cannot parse PEP 695, and about 30 files would reformat. Both were red on every PR, so red meant nothing.

Real bugs fixed in source

Symptom Cause (bisected) Fix
A pushed Screen's own CSS never applied (a background: transparent screen stayed opaque, so the screens behind it were not drawn) bc570b4: a CSS source change queues restyles only on stacked screens, and push_screen/switch_screen read the screen's CSS before stacking it _load_screen_css restyles the screen it loaded rules for
An empty VerticalScroll is not scrollable: overflow-x: scroll draws no bar (upstream Textualize#1616) ef8f854 moved layout: vertical to DEFAULT_LAYOUT; d19dbd5 deleted those as "redundant" because they equalled Widget's default; is_container/is_scrollable read only the CSS layout Layout.holds_children; ImplicitVerticalLayout is the leaf default; is_container reads the layout and is_scrollable is is_container; the 12 containers declare VerticalLayout again
A full arrangement's cache could not serve the next viewport request: the scroll container was arranged twice b31f2ed made full captures IntrinsicSubtreeGeometry, and only PlacedSubtreeGeometry.matches let a complete capture serve visible_only the rule lives once on SubtreeGeometry.matches; members declare only _comparable (the key, or its intrinsic form)
pilot.pause() returns before the screen's timer frame ad7782f (#252, mine): the pilot now waits only on pumps that owe work no source change; the paint test's frame split is re-recorded (22 → 23 updates, every update after the split byte-identical)

Stale tests repaired against the current contract

  • render_line(y) → render_line(y, paint) (89c3a65).
  • Stepped layout (76119a0 deleted per-widget render steps; composition is one step after the budget pause): the readers test asserts every pause saw the previous scene, and the repaint spy reads damage where the frame consumes it.
  • AwaitMount reads is_leaving; the Mock set private flags instead.
  • restore_editor_state became a coroutine (0212c05); the tests never awaited it.
  • Viewport overlay: asserts "never an intrinsic clip", not a cache entry a scroll-only reflow no longer captures.
  • test_lazy: asserts the first drawn frame has no lazy widget (Lazy's contract), not when run_test returns.
  • Footer: clicks the Quit key; ad356fb puts the palette key first.
  • Hover path key: asserts the hovered colour, not a rule re-check count (065244c).
  • Focus-order acquisition: asserts exactly the unplaced buttons are retained (2094493).

Still owed: pyproject declares python ^3.9; raising it changes poetry.lock's hash, so it lands with a relock.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd

@trissim @claude
CI tests the Pythons it can provision: 3.9 never set up, and fail-fas…
b28799f 
…t cancelled every build

GitHub's ubuntu-latest (26.04) has no Python 3.9, so setup failed on every PR and fail-fast
cancelled every other build: no Textual PR has run its tests in CI (#251, #252 and #253 all
show the cancelled matrix). The matrix starts at 3.10, and the four 3.9-only branches go with it.

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 15:17
Copilot AI reviewed Nov 6, 2026

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.

@trissim @claude
CI starts at Python 3.12: the source already uses 3.12 syntax
43063f9 
box_model.py and css/native.py declare type aliases with 'type X = ...' (PEP 695), so 3.10 and
3.11 cannot import textual: #255's 3.10 build failed collection with SyntaxError at
box_model.py:30 and fail-fast cancelled the rest. The matrix tests the Pythons the code runs on.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim trissim changed the title CI tests the Pythons it can provision: 3.9 never set up, and fail-fast cancelled every build CI tests the Pythons the code runs on (3.12+): 3.9 never set up, 3.10/3.11 cannot parse it Nov 6, 2026
trissim and others added 14 commits November 6, 2026 10:36
@trissim @claude
test_message_inheritance_namespace asserts namespaces, not two pumps'…
40187c1 
… delivery order

Issue Textualize#1814's requirement is that each subclass's Fired reaches its own namespace's handler.
The test also pinned the order two independent pumps reach the app in, which the message loop
does not promise; it failed 5/5 on main (right before left). It now checks each handler ran once.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
Paint tests take the PaintState render_line contract; the publish spy…
48b4dd6 
… forwards the deadline generator

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
Stepped and paint tests read the compositor's one-step frame contract
0705418 
76119a0 deleted the per-widget render steps after a layout's commit: composition is one
atomic step after the budget pause, and it consumes pending repaints there. Under held frames
the commit is now the layout's last step, so the readers test asserts every pause saw the
previous scene instead of counting render steps that no longer exist. The repaint spy reads the
damage where the frame takes the repaints (update_widgets), not before the pause.

The paint digest is re-recorded: ad7782f's pilot returns before the screen's timer frame,
so the first page-up's scrollbar column is written as its own update (22 -> 23). Every update
after that split is byte-identical, under rich 14.2.0 (the lock) and 15.0.0 alike.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
Mount tests set is_leaving, the property AwaitMount reads
f272b9d 
AwaitMount._finish (widget.py) skips the parent refresh when parent.is_leaving or _closed.
The tests' Mock(spec=Widget) hand-set the private _closing/_pruning flags behind is_leaving,
so is_leaving stayed a truthy Mock and refresh was never called.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
Editor-state tests await restore_editor_state
7b268a2 
0212c05 made TextArea.restore_editor_state a coroutine (it closes the temporary editing mode).
The tests still called it bare, so the coroutine never ran and nothing was restored: a fresh
Document and an empty selection.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
Viewport overlay test asserts no intrinsic clip, not a cache entry
9ec4967 
b31f2ed made viewport layout choose its geometry type from visible_only, and a scroll-only
reflow arranges through ScrolledGeometry, which captures no subtree. The test indexed the cache
entry and raised KeyError; its stated invariant is that the overlay's resource is never an
intrinsic clip, so it asserts exactly that.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
Delete the debt-ratchet and black workflows: neither can pass
b9e9b2b 
debt-ratchet installs agent-comms at 88f4e6ee, which crashes at import on Python 3.14.10, and
agent-comms main no longer ships agent-comms-ratchet at all (the NRA gate replaced it).
black_format pins Python 3.11 and black 24.1.1, which cannot parse the tree's PEP 695 syntax
(box_model.py:30) and would reformat about thirty files nobody formats with it. Both were red on
every PR, so a red check here told nobody anything.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
test_lazy checks the first drawn frame, not when run_test returns
c006186 
603a769 runs after-refresh callbacks once the frame after them publishes, which is Lazy's
contract: the first frame is drawn without the lazy widget, then it mounts. run_test now returns
after that second frame, so asserting #foo absent at entry tested pilot timing. The app records
what each written frame contained and the test asserts the first had no #foo.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
A pushed screen is restyled with the CSS loaded for it
4c25c83 
push_screen and switch_screen mount the screen (_get_screen) and only then read its CSS
(_load_screen_css). bc570b4 made a CSS source change queue restyles on the screens in the
stacks, and ScreenResume restyle only those queued nodes; the incoming screen is not on a stack
yet, so its own CSS was never applied. A SeeThrough screen with background: transparent stayed
opaque, and the screens behind it were not drawn. The loader holds the screen the rules are for
and restyles it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
Modal pop test clicks the Quit footer key, not the first FooterKey
7d0e6c7 
ad356fb reconciles footer groups and puts the common keys (the command palette) first, so
clicking the first FooterKey opened the palette. Issue 4656 is about clicking the Quit binding.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
Hover path-key test asserts the hovered colour, not a rule re-check c…
c3dd2b6 
…ount

065244c made ancestor hover part of the path key and restyles the hovered row when hover
changes, so applying it afterwards hits that entry with no rule checks. The colour is the
contract: an entry keyed without hover would answer red.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
Focus-order acquisition test asserts exactly the unplaced buttons are…
732a223 
… retained

2094493 acquires only required geometry paths: buttons already placed in the published
viewport are not re-acquired. The test demanded all forty; it now demands exactly the ones the
viewport lacks, which also catches over-acquisition.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
A complete intrinsic capture serves a viewport request, like a placed…
a21208c 
… one

b31f2ed made a full arrangement capture IntrinsicSubtreeGeometry, but only
PlacedSubtreeGeometry.matches let a complete (non-visible_only) capture answer a visible_only
request. The next reflow_visible at the same scroll re-arranged the scroll container
(arrangement_calls 1, expected 0). The rule lives once on SubtreeGeometry.matches; each member
declares only which key inputs its placements depend on (_comparable: the key itself, or its
placement-free intrinsic form). The test drops its pin on the capture's class and keeps the reuse
and reference-equality assertions. Geometry and compositor tests: 251 passed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
A container is declared by its layout, so an empty one is still a con…
ffb578d 
…tainer

ef8f854 moved layout: vertical out of the containers' CSS into DEFAULT_LAYOUT = VerticalLayout;
d19dbd5 then deleted those nine declarations as redundant, because they equalled Widget's own
default. Equal values, different facts: Widget's default is what a leaf has, a container's is a
declaration. is_container and is_scrollable (one expression written twice) still read only the
CSS child_layout, so an empty VerticalScroll stopped being scrollable: overflow-x: scroll drew no
scrollbar (upstream issue 1616's test, red since ef8f854).

- Layout.holds_children: a declared layout makes its widget a container. ImplicitVerticalLayout
  is the leaf default and holds none.
- is_container reads the widget's layout; is_scrollable is is_container.
- Container, ScrollableContainer, Vertical, VerticalGroup, Center, Right, Middle, CenterMiddle,
  Screen, Markdown, ToastRack and HelpPanel declare VerticalLayout again, as their CSS did.

Textual suite without snapshots: 3849 passed, 0 failed (snap_compare absent locally).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim trissim changed the title CI tests the Pythons the code runs on (3.12+): 3.9 never set up, 3.10/3.11 cannot parse it CI runs the tests again, and the 19 they hid are fixed (4 were real bugs) Nov 6, 2026
trissim and others added 10 commits November 6, 2026 11:15
@trissim @claude
The paint digest test pins its terminal to truecolor
9420da6 
Its digest was recorded under COLORTERM=truecolor; CI has no COLORTERM, so the console downgraded
colours and the same 23 updates hashed differently. The test owns the terminal it records.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
Navigation spellings test checks each arrow's modal spelling by actio…
e25defb 
…n and identity

Binding.for_mode spells an arrow under its modal key keeping every other field, including an
author's key_display; with_key resets key_display, so comparing whole Bindings failed for the
snapshot apps that set one (only collected in CI). The test protects that the spelling exists with
the shared identity, not a display policy.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
Keylines draw for containers that declare their layout in the class
a50a10d 
_render_container drew a keyline only when the CSS declared a layout (geometry.child_layout).
Since ef8f854, Grid, Horizontal and the other containers declare their layout as
DEFAULT_LAYOUT, so a keyline rule on them drew nothing: the keyline snapshots lost every line.
It now asks is_container, the one container predicate. All three keyline snapshots match
upstream pixel for pixel (theme colours normalised).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
(cherry picked from commit fb8a1c7)
@trissim @claude
A key sent to a hidden focused widget still reaches its ancestors
de83ad6 
InputEvent.can_dispatch_to (38e6003) refuses input handlers on a widget
under a display:none ancestor, but Widget._dispatch_message then dropped the
event outright, so it never bubbled. Auto-focus may focus a hidden widget
(upstream does too), and every key after that vanished before Screen and App:
richlog_width's App.key_p never ran and its RichLog stayed empty.

The bubble step moves out of MessagePump._on_message into _bubble_onward,
and a refused receiver passes the event on through it without running its own
handlers. A detached widget still bubbles nothing (is_attached is False).
Proof: a hidden focused RichLog plus App.key_p; before, the key stopped at
RichLog; now RichLog -> Screen -> App.key_p, matching upstream's base.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
A rule set to initial reads as its declared default, never None
a3e3fc1 
The builder stores initial as None so it still overrides less specific rules,
and StyleProperty.__get__ handed that None back. The default theme resolves
$link-background to initial, so Widget's rule read link_background as None and
Stylesheet.css raised AttributeError in Styles.css_lines. Any widget logging the
stylesheet died at mount: scoped_css and unscoped_css lost their widgets. The
crash is upstream's too; the fork's atomic mount made it drop the widgets.
Proof: App().stylesheet.css raises before, succeeds after; test_scoped_css
renders 0 px from upstream's reference.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
Line API examples and snapshot apps take render_line(y, paint)
a87ee2f 
Widget.render_line is (y, paint); two snapshot apps and the four checker guide
examples still defined the deleted one-argument form, so they raised and
rendered blank. The guide now says what paint carries.
Proof: test_scrollbar_thumb_height and test_component_text_opacity render
0 px from upstream's reference (were fully blank).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
The paste snapshot fills the clipboard through its owner
1496e82 
test_text_area_paste wrote App._clipboard, a private string 4cbaf90 replaced
with typed clipboard_content; nothing reads the old name, so ctrl+v pasted an
empty clipboard and the snapshot was blank. copy_to_clipboard is the owner.
Proof: test_text_area_paste renders 312 px from upstream's reference, the
terminal cursor only (was 85,696, blank).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
The app's widgets mount before the screen's own tooltip and toasts
882b6f7 
App._on_compose mounted [*screen._nodes, *app widgets]; mount_all leaves
already-mounted nodes in place, and with the fork's atomic screen composition
the screen's Tooltip (a Static) already exists there. query_one(Static) in
on_mount then found the hidden tooltip: test_static_content_property set
content on it and the app's own Static kept 'Hello, World'. Upstream only
worked because its tooltip did not exist yet at that point. The app's widgets
now mount before the screen's existing nodes; each system widget paints on its
own layer, so order does not change the picture.
Proof: test_static_content_property 0 px from upstream (was 1,223); every
tooltip and notification snapshot stays 0 px.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
Cursor commands are ordinary bindings, not priority ones
a590a8a 
CursorCommand.priority = True (5eaa473) made every list's arrows priority
bindings, and priority resolves from the App inward, so a list nested in
another lost its arrows to the outer one: in test_select_refocus the open
Select overlay was focused while down moved the ListView behind it. Type-to-
search consumes printable keys only, and printable cursor aliases already
precede it through NavigationKey.preempts, so the priority bought nothing.
The priority ClassVar had no other writer and goes too.
Proof: the key trace matches upstream's base key for key; test_select_refocus
0 px (was 386,686); 757 binding, keymap, navigation, list and input tests pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
A named action on an arrow keeps the id its author declared
edbfd5b 
Binding.for_mode gave every binding on an arrow the arrow's shared preference
identity, so Binding('i,up', id='app.increment') split into an app.increment
half and an UpKey half. set_keymap replaced only the first; the footer kept
showing 'up' with its stale key_display. preference_identity now answers on the
binding family: a named action keeps its explicit id and joins the arrow's
preference only without one; native ModeBinding commands always share it.
Proof: test_keymap_bindings_key_display 0 px (was 22,068);
test_navigation_spellings keeps id-less arrow actions on the shared identity.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
@trissim @claude
Every console in a test session is one truecolor terminal
ecdc214 
Rich caches a Style's ANSI codes on the Style the first time any console
renders it, and combined styles are shared through lru caches. The frame-damage
tests run apps with COLORTERM unset, and test_paint_output's truecolor digest
then read their 256-colour bytes for one reverse style: it failed in the full
suite and passed alone. tests/conftest.py now declares the session's terminal
once, before any app exists, and the digest test's own monkeypatch goes.
Proof: test_partial_damage_uses_current_frame_for_spans_and_chops followed by
the digest test: 4 passed (was 1 failed); settled snapshots stay 0 px.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSqJMarXn2ZE8iAL7Sd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@trissim