Fix multi dispose scheduler and timer #799
Conversation
| /// Implements a IDisposable to cancel timers | ||
| /// </summary> | ||
| public sealed class DisposableTimer : IDisposable | ||
| { |
There was a problem hiding this comment.
I think this class should have been internal actually
Codecov ReportBase: 78.06% // Head: 78.00% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## dev #799 +/- ##
==========================================
- Coverage 78.06% 78.00% -0.07%
==========================================
Files 152 152
Lines 3999 4005 +6
Branches 402 404 +2
==========================================
+ Hits 3122 3124 +2
- Misses 717 721 +4
Partials 160 160
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
|
||
| public class DisposableTimerTest | ||
| { | ||
| [Fact] |
There was a problem hiding this comment.
Suggestion:
Because this is really a helper class that should be internal it does not seem to deserve its own test class. You could move this test to SchedulerExtensionTest.SchedulePeriodicStopsAfterDisposeOfSubscriber() and dispose the result of disposable.RunEvery there twice
* Check if already disposed and skip if so * Add unit test to check dispose functionality
* Check if already disposed and skip if so * Add unit test to check dispose functionality
3 participants
Proposed change
Check before running the dispose code if the object has already been disposed previously. If so skip the code.
Also added a simple unit test to check if its possible to dispose twice without exception.
Type of change
Additional information
Checklist