Skip to content

Simplify TUI startup test coverage#22573

Merged
etraut-openai merged 2 commits into
mainfrom
etraut/tui-test-architecture-cleanup
May 14, 2026
Merged

Simplify TUI startup test coverage#22573
etraut-openai merged 2 commits into
mainfrom
etraut/tui-test-architecture-cleanup

Conversation

@etraut-openai

@etraut-openai etraut-openai commented May 13, 2026

Copy link
Copy Markdown
Collaborator

Why

The TUI startup test surface had drifted into expensive, brittle coverage:

  • tui/tests/suite/no_panic_on_startup.rs was already ignored as flaky while still spawning a PTY to exercise malformed exec-policy rules.
  • tui/tests/suite/model_availability_nux.rs used a seeded session, cursor-query spoofing, and repeated interrupts to verify a narrow resume-path invariant.
  • app/tests.rs had started accumulating unrelated startup and summary coverage in one flat module even after the surrounding app code was split into feature modules.

This keeps those behaviors covered while making the tests cheaper to understand and less likely to rot. It also preserves the malformed-rules regression from #8803 without requiring a terminal orchestration test.

What changed

  • Replaced the malformed rules startup PTY case with a direct exec-policy loader regression:
    rules_path_file_returns_read_dir_error
  • Made the existing fresh-session-only startup tooltip behavior explicit with
    should_prepare_startup_tooltip_override,
    then added focused coverage for the resume/fork gate and the persisted NUX counter.
  • Split startup and session-summary coverage out of tui/src/app/tests.rs into dedicated modules so the test layout better mirrors the current app architecture.
  • Converted one single-message goal validation snapshot into semantic assertions where layout was not the behavior under test.
  • Removed the two PTY-heavy suite files that the narrower tests now supersede.

Verification

  • cargo test -p codex-core rules_path_file_returns_read_dir_error
  • cargo test -p codex-tui startup_
  • cargo test -p codex-tui session_summary_
  • cargo test -p codex-tui goal_slash_command_rejects_oversized_objective

@etraut-openai etraut-openai marked this pull request as ready for review May 13, 2026 23:56
@etraut-openai etraut-openai requested a review from a team as a code owner May 13, 2026 23:56
@etraut-openai etraut-openai changed the title [codex] simplify TUI startup test coverage Simplify TUI startup test coverage May 14, 2026
Comment thread codex-rs/tui/src/app.rs
Comment on lines -761 to -763
let startup_tooltip_override =
prepare_startup_tooltip_override(&mut config, &available_models, is_first_run)
.await;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Codex flagged this, and it seems legitimate albeit minor- NUX messages may be marked as displayed when startup fails before they are displayed to the user.

[P2] Defer tooltip impression until startup succeeds — /Users/kbond/code/codex/codex-rs/tui/src/app.rs:760-760. When starting a fresh session with an eligible model-availability NUX and thread initialization fails (for example, the malformed rules path covered by the new exec-policy test), this now persists the NUX shown-count before start_thread returns, so one of the limited tooltip impressions is consumed even though the chat widget never renders it. Keep the preparation/persistence after successful thread startup, or otherwise defer the count update until the tooltip can actually be shown.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Oops, I just noticed this comment after merging. I'll fold a fix into a follow-up PR.

@etraut-openai etraut-openai merged commit 35451ba into main May 14, 2026
31 checks passed
@etraut-openai etraut-openai deleted the etraut/tui-test-architecture-cleanup branch May 14, 2026 01:16
@github-actions github-actions Bot locked and limited conversation to collaborators May 14, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants