RateLimiting: add typed builder/guard tests and external limiter lifetime test#2711
Conversation
|
What missing coverage is this change adding exactly? The code coverage score is 100% for the library and it also has a mutation score of 100. Those number suggest there's no missing tests. |
|
@martincostello Thanks for the review! You’re right that coverage and mutation scores are already 100%. These tests aren’t to raise the percentage, but to lock in a few behavioral contracts: |
|
Moved the tests into RateLimiterResiliencePipelineBuilderExtensionsTests.cs as requested and removed the two standalone files. All TFMs pass locally (net9.0, net8.0, net481). Ready for another look. See PR #2711. @martincostello |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2711 +/- ##
==========================================
- Coverage 96.23% 96.12% -0.11%
==========================================
Files 311 309 -2
Lines 7322 7118 -204
Branches 1012 1008 -4
==========================================
- Hits 7046 6842 -204
Misses 222 222
Partials 54 54
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
The issue or feature being addressed
Add small, focused unit tests for RateLimiting to increase coverage. No product code changes.
Details on the issue fix or feature implementation
ResiliencePipelineBuilder<int>().AddConcurrencyLimiter(...).Build()producesRateLimiterResilienceStrategy.limiter/optionsthrowArgumentNullException.DefaultRateLimiterOptions) throwValidationException.RateLimiterpassed to.AddRateLimiter(limiter)is NOT disposed when aResiliencePipelineRegistryis disposed.Files added
test/Polly.RateLimiting.Tests/RateLimiterResiliencePipelineBuilderExtensions_TypedAndNullTests.cstest/Polly.RateLimiting.Tests/RateLimiterExternalLimiterLifetimeTests.csNotes
net9.0,net8.0,net481).Confirm the following