Skip to content

Commit f3e0f6b

Browse files
authored
Fix a flaky Extensions.ML test. (#4458)
* Fix a flaky Extensions.ML test. Make the reload model tests more resistant to timing changes. * PR feedback.
1 parent c1e190a commit f3e0f6b

File tree

2 files changed

+16
-20
lines changed

2 files changed

+16
-20
lines changed

test/Microsoft.Extensions.ML.Tests/FileLoaderTests.cs

+6-10
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
using System;
66
using System.IO;
7-
using System.Threading.Tasks;
7+
using System.Threading;
88
using Microsoft.Extensions.DependencyInjection;
99
using Microsoft.Extensions.Logging;
1010
using Microsoft.Extensions.Options;
@@ -48,10 +48,8 @@ public void can_load_model()
4848
Assert.True(prediction.Sentiment);
4949
}
5050

51-
//TODO: This is a quick test to give coverage of the main scenarios. Refactoring and re-implementing of tests should happen.
52-
//Right now this screams of probably flakeyness
5351
[Fact]
54-
public async Task can_reload_model()
52+
public void can_reload_model()
5553
{
5654
var services = new ServiceCollection()
5755
.AddOptions()
@@ -61,16 +59,14 @@ public async Task can_reload_model()
6159
var loaderUnderTest = ActivatorUtilities.CreateInstance<FileLoaderMock>(sp);
6260
loaderUnderTest.Start("testdata.txt", true);
6361

64-
var changed = false;
65-
var changeTokenRegistration = ChangeToken.OnChange(
62+
using AutoResetEvent changed = new AutoResetEvent(false);
63+
using IDisposable changeTokenRegistration = ChangeToken.OnChange(
6664
() => loaderUnderTest.GetReloadToken(),
67-
() => changed = true);
65+
() => changed.Set());
6866

6967
File.WriteAllText("testdata.txt", "test");
7068

71-
await Task.Delay(1000);
72-
73-
Assert.True(changed);
69+
Assert.True(changed.WaitOne(1000), "FileLoader ChangeToken didn't fire before the allotted time.");
7470
}
7571

7672

test/Microsoft.Extensions.ML.Tests/UriLoaderTests.cs

+10-10
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,12 @@ public void can_reload_model()
4040
var loaderUnderTest = ActivatorUtilities.CreateInstance<UriLoaderMock>(sp);
4141
loaderUnderTest.Start(new Uri("http://microsoft.com"), TimeSpan.FromMilliseconds(1));
4242

43-
var changed = false;
44-
var changeTokenRegistration = ChangeToken.OnChange(
43+
using AutoResetEvent changed = new AutoResetEvent(false);
44+
using IDisposable changeTokenRegistration = ChangeToken.OnChange(
4545
() => loaderUnderTest.GetReloadToken(),
46-
() => changed = true);
47-
Thread.Sleep(30);
48-
Assert.True(changed);
46+
() => changed.Set());
47+
48+
Assert.True(changed.WaitOne(1000), "UriLoader ChangeToken didn't fire before the allotted time.");
4949
}
5050

5151
[Fact]
@@ -62,12 +62,12 @@ public void no_reload_no_change()
6262

6363
loaderUnderTest.Start(new Uri("http://microsoft.com"), TimeSpan.FromMilliseconds(1));
6464

65-
var changed = false;
66-
var changeTokenRegistration = ChangeToken.OnChange(
65+
using AutoResetEvent changed = new AutoResetEvent(false);
66+
using IDisposable changeTokenRegistration = ChangeToken.OnChange(
6767
() => loaderUnderTest.GetReloadToken(),
68-
() => changed = true);
69-
Thread.Sleep(30);
70-
Assert.False(changed);
68+
() => changed.Set());
69+
70+
Assert.False(changed.WaitOne(100), "UriLoader ChangeToken fired but shouldn't have.");
7171
}
7272
}
7373

0 commit comments

Comments
 (0)