Raise the TestKit expect default for starved CI schedulers - #2014
Merged
Conversation
The stock akka.test.single-expect-default is 3 seconds. This value measures scheduler load on a starved CI runner. It does not measure correctness. DiscordSessionBindingContractTests.Proactive_thread_can_apply_session_title_updates flaked on Windows CI. The ack sits behind actor spawn, Akka.Persistence recovery, and two stream materializations. A starved 4-vCPU pool can exceed 3 seconds for this path. Production allows 30 seconds for the same handshake in ProactiveSendFormatting.ProactiveThreadAckTimeout. This commit raises akka.test.single-expect-default to 15 seconds in the affected test classes. SessionBindingContractTests gains a Config override that covers all three channel subclasses. The sibling ProactiveThreadAck tests in SlackProactiveThreadTests, DiscordProactiveThreadTests, and MattermostProactiveThreadTests get the same raise, as does SlackThreadBackfillIntegrationTests. ErrorCorrelationTests uses the Config property instead of ConfigureAkka, because LlmSessionTestBase seals ConfigureAkka. ErrorCorrelationTests also drops hardcoded 3-second timeouts on ExpectMsgAsync<TurnCompleted> calls, so those calls inherit the new default. The Ask calls in that file keep an explicit 15-second timeout instead of inheriting a default, because Ask falls back to akka.actor.ask-timeout (infinite by default), not single-expect-default. Deleting that argument would trade a flaky wait for a hang. Production code is untouched. Green tests pay no added time. The raise only slows failure reporting.
Aaronontheweb
enabled auto-merge (squash)
August 19, 2026 04:06
Aaronontheweb
disabled auto-merge
August 19, 2026 04:28
Aaronontheweb
enabled auto-merge (squash)
August 19, 2026 04:28
Aaronontheweb
added a commit
that referenced
this pull request
Aug 19, 2026
…2019) * Use the class Ask timeout in the double-approval sub-agent test SubAgentActorTests.Approve_once_does_not_leak_between_subagent_tool_calls used a 5-second literal on its outer Ask. Every sibling approval test in the file uses the class constant ApprovalAskTimeout, which is 30 seconds. This test drives two sequential approval round-trips. It makes more hops than any sibling, so it is the least able to tolerate a 5-second budget. A starved Windows CI runner made it flake. The literal now uses the class constant. The Ask keeps an explicit timeout, because Ask falls back to akka.actor.ask-timeout, which is infinite by default. Deletion would trade a flaky wait for a hang. * Raise the ErrorOutput expect budget in ErrorCorrelationTests ErrorCorrelationTests kept four 5-second literals on ExpectMsgAsync<ErrorOutput>. Each one waits for a full provider-failure turn: actor spawn, Akka.Persistence recovery, a failed stream, and the error classification. A starved CI runner can exceed 5 seconds on that path. The literal measures scheduler load, not correctness. This commit adds the same Config-property raise that PR #2014 applies to this class, and drops the four literals so they inherit it. LlmSessionTestBase seals ConfigureAkka, so the Config property is the only seam. The comment text matches #2014 word for word, so the two changes agree where they overlap. The Ask literals in this file stay explicit. Ask falls back to akka.actor.ask-timeout, which is infinite by default. Deletion would trade a flaky wait for a hang. Production code is untouched. A green test pays no added time. The raise only slows failure reporting. * Size the shared approval-test budgets for starved CI schedulers Two shared test helpers held 5-second budgets. Each one bounds a multi-hop actor operation, not a correctness property. A short budget on a shared helper makes a whole group of tests flake at once. ShellApprovalHarness stops the approval actor twice: once to prove a persistent grant survives a restart, and once on dispose. Both stops wait for a persistence flush and an actor teardown. Every shell-approval test runs through this harness. ApprovalRehydrationTests resolves the session child at three sites, one of them the shared ColdRespawnAsync helper. The resolve waits for the actor spawn and the Akka.Persistence recovery. About twenty cold-respawn tests depend on it. Each budget moves to 15 seconds and carries a comment that states what it bounds. Production code is untouched. A green test pays no added time. The raise only slows failure reporting.
Aaronontheweb
disabled auto-merge
August 19, 2026 20:54
…efault # Conflicts: # src/Netclaw.Actors.Tests/Sessions/ErrorCorrelationTests.cs
Aaronontheweb
enabled auto-merge (squash)
August 19, 2026 20:57
This was referenced Aug 20, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root cause
akka.test.single-expect-defaultis the stock 3-second wait in everyAkka.Hosting.TestKittest.DiscordSessionBindingContractTests.Proactive_thread_can_apply_session_title_updatesflaked on Windows CI because its parameterless
ExpectMsgAsync<ProactiveThreadAck>inherits that 3-second default. The ack sits behind actor spawn,
Akka.Persistence recovery, and two stream materializations. Production
budgets 30 seconds for this same ack-after-work handshake
(
ProactiveSendFormatting.ProactiveThreadAckTimeout). A starved4-vCPU CI pool can exceed 3 seconds for a path production itself
treats as needing 30.
Reproduction
DiscordSessionBindingContractTests.Proactive_thread_can_apply_session_title_updatesExpectMsgAsync<ProactiveThreadAck>()SlackThreadBackfillIntegrationTests(~L728)ExpectMsgAsync<ProactiveThreadAck>()SlackProactiveThreadTests,DiscordProactiveThreadTests,MattermostProactiveThreadTestsExpectMsgAsync<ProactiveThreadAck>()/MattermostProactiveThreadAckErrorCorrelationTestsTimeSpan.FromSeconds(3)on several waitsLlmSessionActorturnsWhy this is an environment knob, not timeout-jiggling
Production already decided the correct budget for this handshake is 30
seconds. The test default of 3 seconds was never chosen to match the
handshake's real cost — it is the library's out-of-the-box value. This
change aligns the test default with what production already proves is
correct for the same code path, on a CI runner that is materially
slower than a dev machine. It does not touch production code, does not
loosen an assertion, and does not add a retry or a sleep. Green tests
pay zero added time because
ExpectMsgAsyncresolves as soon as themessage arrives — the higher ceiling only changes how long a genuinely
broken test takes to report failure.
Changes (test config only)
SessionBindingContractTests(abstract base for the Discord/Slack/Mattermostsession-binding contract suites) gains a
Configoverride raisingakka.test.single-expect-defaultto 15s. None of the three subclassesoverride
Config, so this covers all three without touching those files.SlackProactiveThreadTests,DiscordProactiveThreadTests,MattermostProactiveThreadTests: same raise added to each*ProactiveThreadActorTests' existingConfigoverride, alongside theexisting
akka.test.default-timeout = 5s.SlackThreadBackfillIntegrationTests: same raise added viabuilder.AddHocon(..., HoconAddMode.Prepend)inConfigureAkka(thisclass has no
Configproperty override to extend).ErrorCorrelationTests:LlmSessionTestBase.ConfigureAkkaissealed,so this class raises the default through the
Configproperty instead.Hardcoded
TimeSpan.FromSeconds(3)arguments onExpectMsgAsync<TurnCompleted>calls are removed so they inherit the new default. The
Ask<T>calls inthe same file keep an explicit timeout, bumped to 15s rather than deleted
—
Askfalls back toakka.actor.ask-timeout(infinite by default), notsingle-expect-default, so deleting that argument would trade a flakywait for a silent hang.
No production code changed.
Test plan
dotnet build --nologo -v q— 0 warnings, 0 errorsdotnet test src/Netclaw.Actors.Tests --no-build --filter "FullyQualifiedName~SessionBindingContractTests|FullyQualifiedName~ProactiveThread|FullyQualifiedName~ErrorCorrelation|FullyQualifiedName~SlackThreadBackfill"— 181 passed, 0 failed, 6s wall time (no ballooning; the raise only affects failure-path latency)dotnet slopwatch analyze— 0 issues./scripts/Add-FileHeaders.ps1 -Verify— all files have headers