Skip to content

test(tui): stabilize async app server tests#23707

Closed
viyatb-oai wants to merge 1 commit into
mainfrom
codex/viyatb/tui-async-app-server-tests
Closed

test(tui): stabilize async app server tests#23707
viyatb-oai wants to merge 1 commit into
mainfrom
codex/viyatb/tui-async-app-server-tests

Conversation

@viyatb-oai

Copy link
Copy Markdown
Collaborator

Why

Several TUI tests drive the embedded app-server through async request paths. Running those cases on the shared async test runtime makes them sensitive to scheduling and stack pressure, which adds noise to TUI CI while permission-profile work is being reviewed.

What changed

  • Run the affected embedded app-server TUI tests on dedicated single-worker runtimes.
  • Give the few stack-heavy cases an explicit test runtime stack size.
  • Keep the existing assertions and request coverage intact while isolating the runtime setup from unrelated async tests.

Validation

  • GitHub CI for this PR.

Context

Split out of #21559 so the permission-profile picker PR stays focused on its product change.

Co-authored-by: Codex noreply@openai.com
@viyatb-oai viyatb-oai closed this May 20, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

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 Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cfca364ce5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1166 to +1169
let runtime = tokio::runtime::Builder::new_multi_thread()
.worker_threads(WORKER_THREADS)
.thread_stack_size(TEST_STACK_SIZE_BYTES)
.enable_all()

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.

P2 Badge Run the test body on the resized stack

When stack pressure comes from the async test body itself, this thread_stack_size does not protect it: Tokio documents this as the stack size for worker threads, and the multi-thread scheduler's Runtime::block_on executes the supplied future on the current test-harness thread while only spawned tasks run on the worker pool. The converted tests all pass their large async bodies directly to runtime.block_on(...), so those stack-heavy paths still use the default test thread stack and can keep overflowing/flaking; wrap the whole test in a std::thread::Builder::stack_size(...) thread or spawn the body onto the runtime before awaiting it.

Useful? React with 👍 / 👎.

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