Repository navigation
acpmux tests: stand-in harnesses keep normal CPU class (3 codex-steer reds on loaded fleet Macs) - #30057
Conversation
…c environment Since #27600 a harness goes to PRIO_DARWIN_BG once its start is over. The stand-ins time turns with short sleeps; on a loaded fleet Mac (load1 13-21 on 10 P-cores) a background-class Codex-steer stand-in did not end its turn in 18-30 s, so stop::a_slow_stop_does_not_fold..., forum's steered rule and steer_end's race failed only on macOS (diag: 7057c820, ad77491f, 8432fc15; the same 3 pass with the class normal: a35af9a6, 0b8d6072, 1935c6fb). daemon_env::vars sets ACPMUX_SESSION_CPU_CLASS=normal; tests/cpu_class.rs asks for background in its process and its real daemon. Product default unchanged.
There was a problem hiding this comment.
2 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="cmux-tui/crates/acpmux/tests/cpu_class.rs">
<violation number="1" location="cmux-tui/crates/acpmux/tests/cpu_class.rs:28">
P3: Use the `acpmux::cpu_class::ENV` constant here, as the new `RealDaemon::cmd` line does. A literal would go stale silently if the variable is renamed.</violation>
</file>
<file name="cmux-tui/crates/acpmux/tests/hub_integration/stop.rs">
<violation number="1" location="cmux-tui/crates/acpmux/tests/hub_integration/stop.rs:717">
P3: This test checks only the resolved CPU class for the test process's env, not the class of a running stand-in harness. Rename it to describe the resolution (for example `the_test_environment_resolves_cpu_class_to_normal`), or add a real priority check.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| const HERMETIC: daemon_env::Process = daemon_env::Process::stand_in().with_env(&[ | ||
| ("ACPMUX_TEST_CPU_CLASS_REF", "normal"), | ||
| ("ACPMUX_QOS", "background"), | ||
| ("ACPMUX_SESSION_CPU_CLASS", "background"), |
There was a problem hiding this comment.
P3: Use the acpmux::cpu_class::ENV constant here, as the new RealDaemon::cmd line does. A literal would go stale silently if the variable is renamed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cmux-tui/crates/acpmux/tests/cpu_class.rs, line 28:
<comment>Use the `acpmux::cpu_class::ENV` constant here, as the new `RealDaemon::cmd` line does. A literal would go stale silently if the variable is renamed.</comment>
<file context>
@@ -20,8 +20,13 @@ mod daemon_env;
+const HERMETIC: daemon_env::Process = daemon_env::Process::stand_in().with_env(&[
+ ("ACPMUX_TEST_CPU_CLASS_REF", "normal"),
+ ("ACPMUX_QOS", "background"),
+ ("ACPMUX_SESSION_CPU_CLASS", "background"),
+]);
</file context>
| ("ACPMUX_SESSION_CPU_CLASS", "background"), | |
| (acpmux::cpu_class::ENV, "background"), |
| /// `tests/cpu_class.rs` asks for background itself and checks the product's class. | ||
| #[cfg(target_os = "macos")] | ||
| #[test] | ||
| fn a_stand_in_harness_runs_at_normal_class_in_tests() { |
There was a problem hiding this comment.
P3: This test checks only the resolved CPU class for the test process's env, not the class of a running stand-in harness. Rename it to describe the resolution (for example the_test_environment_resolves_cpu_class_to_normal), or add a real priority check.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At cmux-tui/crates/acpmux/tests/hub_integration/stop.rs, line 717:
<comment>This test checks only the resolved CPU class for the test process's env, not the class of a running stand-in harness. Rename it to describe the resolution (for example `the_test_environment_resolves_cpu_class_to_normal`), or add a real priority check.</comment>
<file context>
@@ -705,3 +705,15 @@ async fn a_long_replayed_history_stays_out_of_the_resumed_sessions_last_message(
+/// `tests/cpu_class.rs` asks for background itself and checks the product's class.
+#[cfg(target_os = "macos")]
+#[test]
+fn a_stand_in_harness_runs_at_normal_class_in_tests() {
+ assert_eq!(acpmux::cpu_class::CpuClass::for_session([]), acpmux::cpu_class::CpuClass::Normal);
+}
</file context>
| fn a_stand_in_harness_runs_at_normal_class_in_tests() { | |
| fn the_test_environment_resolves_cpu_class_to_normal() { |
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
red: cmux-next checks (god files, concurrency, crash safety, l10n); base red: the same failure on the base 821fe22; does not block Shipped in:
|
…-harness-normal-class
|
Review (independent review subagent): SHIP at 0a45903 I reviewed only the PR's own changes (33f01ce, 90e1b98, 37defdb). The merge 0a45903 adds nothing to the 4 files: 1. Cause: background class is proven as the cause. Host load is only inferred.
2. Product bug? Not blocking, but the note to the owner should say more.
3. cpu_class.rs is still covered.
4. The regression check is meaningful, but it is a proxy.
5. Evidence is confirmed.
Nits (not blocking)
|
|
Check classification at head 0a45903:
Fleet at the head: clippy Linux d0a4e04f pass. The other 8 |
|
Found 10 test failures on Blacksmith runners: Failures
|
… on an idle Mac (#30153) * scripts/measure: rust-test-bench.sh runs one cmux-tui Rust test alone on an idle Mac, with the host load before and after The swift-test-bench.sh counterpart for cargo tests (class exclusive, measurement, source=SHA). It answers whether a test fails with no other load on the Mac, which a shared ci-step host cannot: first use, whether the acpmux harness at background CPU class misses a stand-in's short timers on an idle Mac (#30057 review; lead 2026-11-01 04:30Z). * rust-test-bench.sh: the step's own PATH; tee failures and a crashed test binary reported; top failure not fatal (review) --------- Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com>
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Summary
On the fleet Macs, 3 acpmux tests failed only on macOS, on the STILL-RED list (STATUS 2359):
stop::a_slow_stop_does_not_fold_the_next_prompt_into_the_stopped_turna_rule_steered_into_a_running_turn_counts_as_toldhub::steer_end::race_tests::an_unsent_steer_gets_its_own_turn_after_another_steer_finishesCause (test under load).
PRIO_DARWIN_BG(cpu_class.rs, defaultbackground).tests/fake_agent.py) times its Codex-steer turn with short sleeps: it polls 300 x 10 ms for a joined prompt, then waits 0.2 s.normal. With it, all 3 pass on macOS: a35af9a6, 0b8d6072, 1935c6fb.Fix (test environment only; the product default stays
background).daemon_env::vars(the hermetic environment of every acpmux test process and of the daemons the tests start) setsACPMUX_SESSION_CPU_CLASS=normal.tests/cpu_class.rs, which checks the product's class, asks forbackgroundin its own process and in its real daemon.normalin its stand-in's profile env.stop::a_stand_in_harness_runs_at_normal_class_in_tests(macOS).Cause detail (review). The class change alone is proved (093a4d2 changes only
DEFAULT). Host load is only inferred: the daemon logged high load in the diagnostic runs, but not in the 2 reds at 33f01ce. Darwin background timer coalescing of the stand-in's 10 ms sleeps is another possible mechanism, and it does not need load. To decide, one run of the steer race test at background class on an idle mini (owner follow-up).Note for the owner of
cpu_class.rs. A real harness that times its work with short sleeps can also run slowly at background class on a loaded Mac. That is the product's choice (#27600, cmuxterm-hq#4947), and this PR does not change it. It only stops the tests from measuring it by accident.Testing
Background!=Normalstop::a_slow_stop_does_not_fold…5 of 5a_rule_steered…5 of 5cpu_classtests (product class still background)hub/processes.rs:256(fixed on feat by #30009)cpu_classmacOS 5dd8ef18: see the check commentObserved / Not run
Observed: everything above. Not run: Linux (Linux has no CPU class:
for_sessionreturnsnormalthere).Changelog
none
Proof
No capture: a test-environment change.
Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Fixes three acpmux tests that timed out only on loaded fleet Macs because their stand-in harnesses ran at background CPU class once a session's start was over. This test-only change keeps the stand-ins at normal class; the product default (
background) is unchanged.daemon_env::vars) now setsACPMUX_SESSION_CPU_CLASS=normalfor test processes and the daemons they start.tests/cpu_class.rsexplicitly requestsbackgroundin its own process and real daemon, so it still verifies the product's actual class.normalon its stand-in's profile env.Written for commit 0a45903. Summary will update on new commits.