Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts a regression test’s timeout to reduce flakiness when validating that ScheduledTaskExecutor does not deadlock under inline continuations (context: Polly PR #2953 follow-up).
Changes:
- Increased the timeout used by
ScheduleTask_InlineContinuationDoesNotDeadlockfrom 250ms to 1s.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2956 +/- ##
=======================================
Coverage 96.15% 96.15%
=======================================
Files 309 309
Lines 7128 7128
Branches 1005 1005
=======================================
Hits 6854 6854
Misses 221 221
Partials 53 53
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
I see that this still timed out on net481 -- I would be curious to see if it fails/how long it takes if you bump it up to 30s or so, and do a couple of CI runs just to rule out the fact that it's actually still deadlocking. I personally can't reproduce the issue with the test locally on .NET9/10 (and I'm on a Mac so testing net481 would be more difficult if that's actually relevant here -- it seems possible that retrieving a thread from the pool is significantly slower) |
|
I've got Copilot working on it. |
…81 (#2957) * Initial plan * Refactor ScheduleTask_InlineContinuationDoesNotDeadlock test to avoid nested blocking Wait Co-authored-by: martincostello <1439341+martincostello@users.noreply.github.com> * Remove PR branch filters Allow running CI for PRs targeting any branch, not just main. * Fix ScheduleTask_InlineContinuationDoesNotDeadlock test - restore correct deadlock detection with increased timeout Co-authored-by: martincostello <1439341+martincostello@users.noreply.github.com> * Change assert - Wait on both tasks with `Task.WaitAll()`. - Suppress `xUnit1031`. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: martincostello <1439341+martincostello@users.noreply.github.com> Co-authored-by: Martin Costello <martin@martincostello.com>
|
Will merge assuming fixed if there's 5 CI (re)runs in a row without the test failing. |
|
Oops, forgot I'd enabled auto-merge... 🙃 |
See #2953 (comment).