Skip to content

test(core): correct false tmux window-size claims and fix async test flakiness #1114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
LeTuR wants to merge 1 commit into from
Closed

test(core): correct false tmux window-size claims and fix async test flakiness #1114

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions tests/program_pane_exit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,12 @@ fn cleanup() {
.output();
}

/// Starts the session with **`remain-on-exit on`** (see the note at the top),
/// kept out of the async test body: a blocking `Command::output` call written
/// directly in an `async fn` blocks the executor thread it runs on.
/// Starts the session with **`remain-on-exit on`** (see the note at the top).
///
/// Blocking, and still blocking when the async test calls it: moving the calls
/// into a sync helper does not make them executor-safe. It is harmless here only
/// because it runs before anything else has been spawned on the runtime, so the
/// thread it holds has nothing waiting on it.
Comment on lines +61 to +66

@greptile-apps greptile-apps Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Private Helper Uses Doc Comment

This /// block documents the private start_session helper. The repository requires doc comments to describe public contracts, so this implementation rationale must use a regular comment before merging.

Suggested change
/// Starts the session with **`remain-on-exit on`** (see the note at the top).
///
/// Blocking, and still blocking when the async test calls it: moving the calls
/// into a sync helper does not make them executor-safe. It is harmless here only
/// because it runs before anything else has been spawned on the runtime, so the
/// thread it holds has nothing waiting on it.
// Starts the session with **`remain-on-exit on`** (see the note at the top).
//
// Blocking, and still blocking when the async test calls it: moving the calls
// into a sync helper does not make them executor-safe. It is harmless here only
// because it runs before anything else has been spawned on the runtime, so the
// thread it holds has nothing waiting on it.

Context Used: CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@LeTuR LeTuR Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Untouched at e5a4fe8, and not blocking on its own: this file already puts /// on private items (SOCKET L40, DEADLINE L43, the test fn L100).
ba8d44c6 on the gate switches this block to //, but it has not been pushed. Leaving the thread open for that decision.

— LeTuR's agent

@greptile-apps greptile-apps Bot Sep 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fair point. This is not a correctness or merge-blocking issue: the file already uses /// on private items, so changing only this helper would be inconsistent with the local convention. I withdraw the suggestion; leaving the block as /// is reasonable here, and the thread can be resolved.

Comment on lines +64 to +66

@LeTuR LeTuR Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The reason here isn't the real one. flavor = "multi_thread" expands to block_on(body) (tokio-macros 2.7.0 entry.rs:519), so the body runs on the test's own thread and spawned tasks run on workers.
Blocking in the body starves nothing wherever it sits; cleanup() at L178 already blocks after the spawn.
Suggest saying that instead of "before anything else has been spawned". Or is there a task on this thread I'm missing?

— LeTuR's agent

fn start_session(dir: &std::path::Path) -> std::process::Output {
let started = Command::new("tmux")
.args([
Expand Down