Skip to content

Make the test suite environment-independent, fix UTF-8 redirection, and a UI-call race - #1752

Merged
tleonhardt merged 4 commits into
consolidated_toolbarfrom
suite-env-robustness
Sep 7, 2026
Merged

Make the test suite environment-independent, fix UTF-8 redirection, and a UI-call race#1752
tleonhardt merged 4 commits into
consolidated_toolbarfrom
suite-env-robustness

Conversation

@tleonhardt

Copy link
Copy Markdown
Member

Three fixes, all pre-existing on consolidated_toolbar and none specific to the
reserved-row toolbar work. Found while investigating a reviewer's report of failures in
unchanged tests.

1. The suite inherited the caller's colour environment

Rich and cmd2 both consult environment variables when deciding whether to emit styling, and
the suite inherited them. Exporting any one made large numbers of unrelated tests fail
depending on who ran it:

Variable Tests failed before
NO_COLOR=1 15
FORCE_COLOR=1 53
TTY_COMPATIBLE=1 53

The failures look like product regressions, which makes them expensive to diagnose — this
cost a reviewer and me a full round trip. An autouse fixture now neutralizes them, and a
guard test fails if any reaches a test again. Tests that exercise these variables still set
them explicitly, because a test's own monkeypatching runs after the fixture.

2. Redirection and piping failed on non-UTF-8 systems (user-visible)

Command output is rendered by Rich and routinely contains non-ASCII, but redirection targets
and pipes were opened with the locale's encoding. On a system whose default is not UTF-8 —
a Windows console using a legacy code pagehelp > out.txt raised UnicodeEncodeError
and left the user an empty file plus advice to set PYTHONIOENCODING.

Both now use UTF-8 explicitly. Two tests that read redirected output back were relying on the
locale for decoding as well, so they name it too. CHANGELOG updated.

3. A race reported a timeout for a successful UI call (back-port)

Cherry-picked from the reserved-row branch, where it was found; it is not specific to that
work. CommandToolbar._call_in_ui() polls the pending future with a 0.1s timeout and
re-raises when the future is already done. That branch exists because
concurrent.futures.TimeoutError is TimeoutError on Python 3.11+, so a callback
raising a timeout of its own cannot be told apart by type from the poll expiring.

Re-raising the caught exception conflates the two: when the callback completes in the window
between the poll expiring and the future being inspected, the caller is told the call timed
out although it succeeded. It now asks the future for its outcome instead.

test_command_toolbar_ui_call_propagates_failures failed once in 40 runs on this branch
before the fix and not once in 60 after. The added regression test drives the interleaving
deterministically rather than relying on timing.

Verification

Full suite under every environment that previously broke it, three runs of the combined case:

baseline                      1937 passed, 2 skipped
NO_COLOR=1                    1937 passed, 2 skipped
FORCE_COLOR=1                 1937 passed, 2 skipped
TTY_COMPATIBLE=1              1937 passed, 2 skipped
TTY_INTERACTIVE=0             1937 passed, 2 skipped
non-UTF8 locale, no -Xutf8    1937 passed, 2 skipped
all hostile at once  x3       1937 passed, 2 skipped

make check, make test and make docs-test all pass.

Rich and cmd2 consult several environment variables when deciding whether to
emit styling, and the suite inherited them. Exporting any one of them made large
numbers of unrelated tests fail depending on who ran the suite: NO_COLOR failed
15 tests, and FORCE_COLOR and TTY_COMPATIBLE 53 each. The failures look like
product regressions, which makes them expensive to diagnose.

Neutralize them for every test. Tests that exercise these variables set them
explicitly, which still works because a test's own monkeypatching runs after the
fixture. A guard test fails if any of them reaches a test again.
Command output is rendered by Rich and routinely contains non-ASCII, but
redirection targets and pipes were opened with the locale's encoding. On any
system whose default is not UTF-8 -- a Windows console using a legacy code page,
for instance -- redirecting output raised UnicodeEncodeError, and the user was
left with an empty file and advice to set PYTHONIOENCODING.

Open both with UTF-8 explicitly. Two tests that read redirected output back were
relying on the locale encoding for decoding as well, so they now name it too.
_call_in_ui() polls the pending future with a 0.1s timeout and, on expiry,
re-raises when the future is already done. That branch exists because
concurrent.futures.TimeoutError is TimeoutError on Python 3.11+, so a callback
raising a timeout of its own cannot be told apart by type from the poll expiring.

Re-raising the caught exception conflates the two. When the callback completes
in the window between the poll expiring and the future being inspected, the
caller is told the call timed out even though it succeeded. Ask the future for
its outcome instead: a callback that raised a timeout still propagates it, and
one that produced a value now returns it.

Found while investigating an intermittent failure of
test_command_toolbar_ui_call_propagates_failures, which reproduced once in 60
runs before this change and not once in 120 after. The added regression test
drives the interleaving deterministically rather than relying on timing.

(cherry picked from commit 84bc19e)
@codecov

codecov Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.67%. Comparing base (dee4c9b) to head (7b70fbc).

Additional details and impacted files
@@                  Coverage Diff                  @@
##           consolidated_toolbar    #1752   +/-   ##
=====================================================
  Coverage                 99.67%   99.67%           
=====================================================
  Files                        25       25           
  Lines                      6464     6464           
=====================================================
  Hits                       6443     6443           
  Misses                       21       21           
Flag Coverage Δ
unittests 99.67% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@tleonhardt tleonhardt self-assigned this Sep 7, 2026
The test piped through `cat`, which cmd.exe does not provide. On a Windows
system without Unix utilities installed it would fail before reaching the
encoding behavior it exists to check -- and Windows is exactly what the UTF-8
redirection fix targets.

Use a sys.executable pass-through instead, matching the pipe tests already in
tests/test_command_toolbar.py. Reverting either the pipe or the redirect
encoding still fails these tests.
@tleonhardt
tleonhardt merged commit eeef9fa into consolidated_toolbar Sep 7, 2026
29 checks passed
@tleonhardt
tleonhardt deleted the suite-env-robustness branch September 7, 2026 16:46
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.

1 participant