Skip to content

Widen the subscription timer reset test's outer deadline to 30 seconds - #10352

Merged
glen-84 merged 1 commit into
mainfrom
gai/subscription-timer-test-budget
Sep 3, 2026
Merged

glen-84 merged 1 commit into
mainfrom
gai/subscription-timer-test-budget

Conversation

@glen-84

@glen-84 glen-84 commented Sep 3, 2026

Copy link
Copy Markdown
Member

Summary

  • SubscriptionEventTimeoutTests.Timer_Should_Be_Reset_Between_Events guarded its whole run with a 10-second CTS and hit it in CI inside a Task.Delay(600), after every event had already completed without errors. The outer guard is now 30 seconds, matching the other tests in the file.
  • The lost time is thread-pool queueing, not the per-event timer. Every await in the test resumes on the thread pool (in-memory subscription channel hops and Task.Delay), and when Issue9500Tests runs concurrently its 100k-node DataLoader query fills the pool for the 13 to 18 seconds it takes under coverage. A probe measured a 600 ms delay resuming after 3 seconds with 4335 pending work items. The per-event 1-second budget is armed only after an event is dequeued, so the wider outer guard does not weaken what the test asserts.

Test plan

  • Ran SubscriptionEventTimeoutTests alone and alongside Issue9500Tests and StarWarsCodeFirstTests on 4 pinned cores with --coverage: 1.8 s alone, 4.6 to 5.4 s under contention, all green.
  • In the failing CI run the test took 10.08 s while overlapping Issue9500Tests (18 s); the re-run had no overlap and the test took 2.18 s.

Copilot AI lite review requested due to automatic review settings September 3, 2026 16:05

Copilot AI 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.

🟢 Approval recommended

The change is a minimal, test-only timeout adjustment that improves reliability without altering the asserted per-event timeout behavior.

Pull request overview

This PR adjusts an overly tight outer test deadline in SubscriptionEventTimeoutTests.Timer_Should_Be_Reset_Between_Events to reduce CI flakiness caused by thread-pool scheduling delays under concurrent test load.

Changes:

  • Increased the test-wide CancellationTokenSource timeout from 10 seconds to 30 seconds for Timer_Should_Be_Reset_Between_Events.
File summaries
File Description
src/HotChocolate/Core/test/Execution.Tests/Pipeline/SubscriptionEventTimeoutTests.cs Widens the test’s outer cancellation deadline to better tolerate thread-pool contention while keeping the per-event timeout assertion intact.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@glen-84
glen-84 merged commit 691bd29 into main Sep 3, 2026
152 checks passed
@glen-84
glen-84 deleted the gai/subscription-timer-test-budget branch September 3, 2026 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants