-
-
Notifications
You must be signed in to change notification settings - Fork 230
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds debouncing FileSystemWatcher events #129
Changes from all commits
c667c11
cbaed5b
6785f88
6e20cb4
5130066
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
using System; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
|
||
namespace McMaster.NETCore.Plugins.Internal | ||
{ | ||
internal class Debouncer : IDisposable | ||
{ | ||
private readonly CancellationTokenSource _cts = new CancellationTokenSource(); | ||
private readonly TimeSpan _waitTime; | ||
private int _counter; | ||
|
||
public Debouncer(TimeSpan waitTime) | ||
{ | ||
_waitTime = waitTime; | ||
} | ||
|
||
public void Execute(Action action) | ||
{ | ||
var current = Interlocked.Increment(ref _counter); | ||
|
||
Task.Delay(_waitTime).ContinueWith(task => | ||
{ | ||
// Is this the last task that was queued? | ||
if (current == _counter && !_cts.IsCancellationRequested) | ||
{ | ||
action(); | ||
} | ||
|
||
task.Dispose(); | ||
}, _cts.Token); | ||
} | ||
|
||
public void Dispose() | ||
{ | ||
_cts.Cancel(); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,3 @@ | ||
McMaster.NETCore.Plugins.Loader.AssemblyLoadContextBuilder.ShadowCopyNativeLibraries() -> McMaster.NETCore.Plugins.Loader.AssemblyLoadContextBuilder | ||
McMaster.NETCore.Plugins.Loader.AssemblyLoadContextBuilder.ShadowCopyNativeLibraries() -> McMaster.NETCore.Plugins.Loader.AssemblyLoadContextBuilder | ||
McMaster.NETCore.Plugins.PluginConfig.ReloadDelay.get -> System.TimeSpan | ||
McMaster.NETCore.Plugins.PluginConfig.ReloadDelay.set -> void |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
using System; | ||
using System.Threading.Tasks; | ||
using McMaster.NETCore.Plugins.Internal; | ||
using Xunit; | ||
|
||
namespace McMaster.NETCore.Plugins.Tests | ||
{ | ||
public class DebouncerTests | ||
{ | ||
[Fact] | ||
public async Task InvocationIsDelayed() | ||
{ | ||
var executionCounter = 0; | ||
|
||
var debouncer = new Debouncer(TimeSpan.FromSeconds(.1)); | ||
debouncer.Execute(() => executionCounter++); | ||
|
||
Assert.Equal(0, executionCounter); | ||
|
||
await Task.Delay(TimeSpan.FromSeconds(.5)); | ||
|
||
Assert.Equal(1, executionCounter); | ||
} | ||
|
||
[Fact] | ||
public async Task ActionsAreDebounced() | ||
{ | ||
var executionCounter = 0; | ||
|
||
var debouncer = new Debouncer(TimeSpan.FromSeconds(.1)); | ||
debouncer.Execute(() => executionCounter++); | ||
debouncer.Execute(() => executionCounter++); | ||
debouncer.Execute(() => executionCounter++); | ||
|
||
await Task.Delay(TimeSpan.FromSeconds(.5)); | ||
|
||
Assert.Equal(1, executionCounter); | ||
} | ||
|
||
[Fact] | ||
public async Task OnlyLastActionIsInvoked() | ||
{ | ||
string? invokedAction = null; | ||
|
||
var debouncer = new Debouncer(TimeSpan.FromSeconds(.1)); | ||
foreach (var action in new[]{"a", "b", "c"}) | ||
{ | ||
debouncer.Execute(() => invokedAction = action); | ||
} | ||
|
||
await Task.Delay(TimeSpan.FromSeconds(.5)); | ||
|
||
Assert.NotNull(invokedAction); | ||
Assert.Equal("c", invokedAction); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,10 +12,19 @@ public static void Main(string[] args) | |
|
||
public static bool Run() | ||
{ | ||
using (var client = new SqlConnection(@"Data Source=(localdb)\mssqllocaldb;Integrated Security=True")) | ||
try | ||
{ | ||
client.Open(); | ||
return !string.IsNullOrEmpty(client.ServerVersion); | ||
using (var client = new SqlConnection(@"Data Source=(localdb)\mssqllocaldb;Integrated Security=True")) | ||
{ | ||
client.Open(); | ||
return !string.IsNullOrEmpty(client.ServerVersion); | ||
} | ||
} | ||
catch (SqlException ex) when (ex.Number == -2) // -2 means SQL timeout | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did the SqlException start happening with this change? Was it consistent, or just a flaky test? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dunno, the build pipleline failed: https://github.com/natemcmaster/DotNetCorePlugins/runs/486084609 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't seen it before. This test isn't using reloading though, so it's probably not related to this change. |
||
{ | ||
// When running the test in Azure DevOps build pipeline, we'll get a SqlException with "Connection Timeout Expired". | ||
// We can ignore this safely in unit tests. | ||
return true; | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add some unit tests for this to verify its behavior (and prevent regressions in the future)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've pushed some unit tests. Do you think they're sufficient?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, thank you!