Repository navigation
Fix flaky ATM test for cached tokens over the max length - #377
Merged
Merged
Conversation
HybridCache returns the token to the caller and then writes it to the distributed cache in the background. The test started instance B straight after instance A returned, so on a busy CI runner B could read the shared cache before A's write landed, fetch its own token and fail. The test now waits for A's write before it starts B.
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The targeted test-only synchronization addresses the reported race without changing production behavior.
0 open findings
What changed in this PR
Fixes the flaky cross-instance token-cache test by waiting for instance A’s distributed-cache write before starting instance B.
Changes:
- Signals completion after synchronous and asynchronous cache writes.
- Waits for that signal with a timeout and cancellation support.
| File | Description |
|---|---|
| access-token-management/test/AccessTokenManagement.Tests/ClientTokenManagementTests.cs | Synchronizes the test with cache-write completion to remove the race. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Erwinvandervalk
approved these changes
Oct 16, 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 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.
Instance_with_lower_limit_is_served_the_cached_token_written_by_another_instance(from #367) fails now and then. It failed once on main (the #366 merge), and most of the time on the 4.3.x backports, where CI runs the net8.0, net9.0 and net10.0 test hosts side by side.HybridCache hands the token to the caller first and writes it to the distributed cache afterwards, in the background. The test started instance B as soon as instance A returned, so B could look in the shared cache before A's write landed. B then fetched its own token, and the assertion saw
TokenRequestsByB = 1, Lengths = 100,100,100. The test now waits for A's write before it starts B.To confirm the cause, I delayed the shared cache write by 200 ms locally: the test failed the same way every time without this change and passed with it.
Once this is merged I'll cherry-pick it into the 4.3.x backport of #367 (#371).