fix(test): remove activation-local reminder waits - #10855
Merged
ReubenBond merged 1 commit intoAug 28, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
Review tier: Lite
Findings: None
What changed in this PR
This PR improves the stability of reminder-related tests by removing an activation-local “counter waiter” mechanism which could block indefinitely during activation churn, and instead relying on reminder diagnostics (TickCompleted) plus persisted counter verification as an activation-independent completion barrier.
Changes:
- Reworked
AdvanceRemindersByTicksAsyncto arm tick diagnostics before time advancement and validate persisted counters after ticks complete. - Removed activation-local counter waiter hooks from reminder test grains (and deleted the waiter implementation).
- Renamed a test to reflect the updated approach (concurrent reminders vs. counter waiters).
| File | Description |
|---|---|
| test/Orleans.Reminders.Tests/TimerTests/ReminderTestsBase.cs | Switches tick advancement/waiting logic from activation-local counter waiters to diagnostic tick completion + persisted counter assertions. |
| test/Orleans.Reminders.Tests/TimerTests/ReminderTests_TableGrain.cs | Renames a test to match the updated reminder synchronization approach. |
| test/Grains/TestInternalGrains/ReminderTestGrain2.cs | Removes test-only activation-local counter waiter signaling/reset logic and the waiter implementation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ReubenBond
force-pushed
the
rb-fix-reminder-suite-timeouts
branch
from
August 27, 2026 21:50
74c4031 to
2d8548c
Compare
ReubenBond
force-pushed
the
rb-fix-reminder-suite-timeouts
branch
from
August 28, 2026 03:02
2d8548c to
0a553b1
Compare
Contributor
Code coverage78.49% line coverage - 99,375 / 126,605 lines Coverage details
|
This was referenced Aug 29, 2026
Merged
This was referenced Sep 9, 2026
Closed
Closed
This was referenced Sep 16, 2026
Open
Merged
Merged
This was referenced Sep 23, 2026
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Problem
The reminder test clock driver waited for persisted counters through a waiter attached to whichever grain activation TryGetGrainContext found. During activation churn, a reminder tick could run on another activation: the callback persisted the target counter and emitted TickCompleted, while the waiter on the stale activation remained blocked until the suite timeout.
Solution
Remove the activation-local counter waiter and its test-grain hooks. The clock driver now arms reminder diagnostic waits before advancing time, awaits TickCompleted, and then verifies that each persisted counter advanced exactly once.
Rationale
TickCompleted is emitted only after ReceiveReminder returns, and the test grains persist their counters before returning. This gives the tests an activation-independent completion barrier which remains valid across ownership changes and grain reactivation.
Fixes #10499
Microsoft Reviewers: Open in CodeFlow