chore: remove test sleeps, fix flaky test (#194)

Signed-off-by: Todd Baert <[email protected]>
Co-authored-by: Austin Drenski <[email protected]>

Author: toddbaert

test/OpenFeature.Tests/OpenFeatureEventTests.cs

@@ -76,34 +76,33 @@ public async Task API_Level_Event_Handlers_Should_Be_Registered()
             testProvider.SendEvent(ProviderEventTypes.ProviderError);
             testProvider.SendEvent(ProviderEventTypes.ProviderStale);
 
-            Thread.Sleep(1000);
-            eventHandler
+            await Utils.AssertUntilAsync(_ => eventHandler
                 .Received()
                 .Invoke(
                     Arg.Is<ProviderEventPayload>(
                         payload => payload.ProviderName == testProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderReady
-                    ));
+                    )));
 
-            eventHandler
+            await Utils.AssertUntilAsync(_ => eventHandler
                 .Received()
                 .Invoke(
                     Arg.Is<ProviderEventPayload>(
                         payload => payload.ProviderName == testProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderConfigurationChanged
-                    ));
+                    )));
 
-            eventHandler
+            await Utils.AssertUntilAsync(_ => eventHandler
                 .Received()
                 .Invoke(
                     Arg.Is<ProviderEventPayload>(
                         payload => payload.ProviderName == testProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderError
-                    ));
+                    )));
 
-            eventHandler
+            await Utils.AssertUntilAsync(_ => eventHandler
                 .Received()
                 .Invoke(
                     Arg.Is<ProviderEventPayload>(
                         payload => payload.ProviderName == testProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderStale
-                    ));
+                    )));
         }
 
         [Fact]
@@ -122,13 +121,12 @@ public async Task API_Level_Event_Handlers_Should_Be_Informed_About_Ready_State_
 
             Api.Instance.AddHandler(ProviderEventTypes.ProviderReady, eventHandler);
 
-            Thread.Sleep(1000);
-            eventHandler
+            await Utils.AssertUntilAsync(_ => eventHandler
                 .Received()
                 .Invoke(
                     Arg.Is<ProviderEventPayload>(
                         payload => payload.ProviderName == testProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderReady
-                    ));
+                    )));
         }
 
         [Fact]
@@ -149,13 +147,12 @@ public async Task API_Level_Event_Handlers_Should_Be_Informed_About_Error_State_
 
             Api.Instance.AddHandler(ProviderEventTypes.ProviderError, eventHandler);
 
-            Thread.Sleep(1000);
-            eventHandler
+            await Utils.AssertUntilAsync(_ => eventHandler
                 .Received()
                 .Invoke(
                     Arg.Is<ProviderEventPayload>(
                         payload => payload.ProviderName == testProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderError
-                    ));
+                    )));
         }
 
         [Fact]
@@ -175,13 +172,12 @@ public async Task API_Level_Event_Handlers_Should_Be_Informed_About_Stale_State_
 
             Api.Instance.AddHandler(ProviderEventTypes.ProviderStale, eventHandler);
 
-            Thread.Sleep(1000);
-            eventHandler
+            await Utils.AssertUntilAsync(_ => eventHandler
                 .Received()
                 .Invoke(
                     Arg.Is<ProviderEventPayload>(
                         payload => payload.ProviderName == testProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderStale
-                    ));
+                    )));
         }
 
         [Fact]
@@ -207,9 +203,12 @@ public async Task API_Level_Event_Handlers_Should_Be_Exchangeable()
 
             newTestProvider.SendEvent(ProviderEventTypes.ProviderConfigurationChanged);
 
-            Thread.Sleep(1000);
-            eventHandler.Received(2).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderReady));
-            eventHandler.Received(2).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderConfigurationChanged));
+            await Utils.AssertUntilAsync(
+                _ => eventHandler.Received(2).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderReady))
+            );
+            await Utils.AssertUntilAsync(
+                _ => eventHandler.Received(2).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderConfigurationChanged))
+            );
         }
 
         [Fact]
@@ -257,8 +256,12 @@ public async Task API_Level_Event_Handlers_Should_Be_Executed_When_Other_Handler
             var testProvider = new TestProvider(fixture.Create<string>());
             await Api.Instance.SetProvider(testProvider);
 
-            failingEventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
-            eventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
+            await Utils.AssertUntilAsync(
+                _ => failingEventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name))
+            );
+            await Utils.AssertUntilAsync(
+                _ => eventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name))
+            );
         }
 
         [Fact]
@@ -305,10 +308,12 @@ public async Task Client_Level_Event_Handlers_Should_Be_Executed_When_Other_Hand
             var testProvider = new TestProvider();
             await Api.Instance.SetProvider(myClient.GetMetadata().Name, testProvider);
 
-            Thread.Sleep(1000);
-
-            failingEventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
-            eventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
+            await Utils.AssertUntilAsync(
+                _ => failingEventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name))
+            );
+            await Utils.AssertUntilAsync(
+                _ => eventHandler.Received().Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name))
+            );
         }
 
         [Fact]
@@ -368,10 +373,10 @@ public async Task Client_Level_Event_Handlers_Should_Be_Receive_Events_From_Name
 
             defaultProvider.SendEvent(ProviderEventTypes.ProviderConfigurationChanged);
 
-            Thread.Sleep(1000);
-
             // verify that the client received the event from the default provider as there is no named provider registered yet
-            clientEventHandler.Received(1).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == defaultProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderConfigurationChanged));
+            await Utils.AssertUntilAsync(
+                _ => clientEventHandler.Received(1).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == defaultProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderConfigurationChanged))
+            );
 
             // set the other provider specifically for the client
             await Api.Instance.SetProvider(client.GetMetadata().Name, clientProvider);
@@ -380,12 +385,14 @@ public async Task Client_Level_Event_Handlers_Should_Be_Receive_Events_From_Name
             defaultProvider.SendEvent(ProviderEventTypes.ProviderConfigurationChanged);
             clientProvider.SendEvent(ProviderEventTypes.ProviderConfigurationChanged);
 
-            Thread.Sleep(1000);
-
             // now the client should have received only the event from the named provider
-            clientEventHandler.Received(1).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == clientProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderConfigurationChanged));
+            await Utils.AssertUntilAsync(
+                _ => clientEventHandler.Received(1).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == clientProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderConfigurationChanged))
+            );
             // for the default provider, the number of received events should stay unchanged
-            clientEventHandler.Received(1).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == defaultProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderConfigurationChanged));
+            await Utils.AssertUntilAsync(
+                _ => clientEventHandler.Received(1).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == defaultProvider.GetMetadata().Name && payload.Type == ProviderEventTypes.ProviderConfigurationChanged))
+            );
         }
 
         [Fact]
@@ -431,15 +438,15 @@ public async Task Client_Level_Event_Handlers_Should_Be_Removable()
             await Api.Instance.SetProvider(myClient.GetMetadata().Name, testProvider);
 
             // wait for the first event to be received
-            Thread.Sleep(1000);
-            myClient.RemoveHandler(ProviderEventTypes.ProviderReady, eventHandler);
+            await Utils.AssertUntilAsync(_ => myClient.RemoveHandler(ProviderEventTypes.ProviderReady, eventHandler));
 
             // send another event from the provider - this one should not be received
             testProvider.SendEvent(ProviderEventTypes.ProviderReady);
 
             // wait a bit and make sure we only have received the first event, but nothing after removing the event handler
-            Thread.Sleep(1000);
-            eventHandler.Received(1).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name));
+            await Utils.AssertUntilAsync(
+                _ => eventHandler.Received(1).Invoke(Arg.Is<ProviderEventPayload>(payload => payload.ProviderName == testProvider.GetMetadata().Name))
+            );
         }
     }
 }

test/OpenFeature.Tests/TestUtils.cs

@@ -0,0 +1,54 @@
+using System;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Threading;
+using System.Threading.Tasks;
+
+internal class Utils
+{
+    /// <summary>
+    /// Repeatedly runs the supplied assertion until it doesn't throw, or the timeout is reached.
+    /// </summary>
+    /// <param name="assertionFunc">Function which makes an assertion</param>
+    /// <param name="timeoutMillis">Timeout in millis (defaults to 1000)</param>
+    /// <param name="pollIntervalMillis">Poll interval (defaults to 100</param>
+    /// <returns></returns>
+    public static async Task AssertUntilAsync(Action<CancellationToken> assertionFunc, int timeoutMillis = 1000, int pollIntervalMillis = 100)
+    {
+        using (var cts = CancellationTokenSource.CreateLinkedTokenSource(default(CancellationToken)))
+        {
+
+            cts.CancelAfter(timeoutMillis);
+
+            var exceptions = new List<Exception>();
+            var message = "AssertUntilAsync timeout reached.";
+
+            while (!cts.IsCancellationRequested)
+            {
+                try
+                {
+                    assertionFunc(cts.Token);
+                    return;
+                }
+                catch (TaskCanceledException) when (cts.IsCancellationRequested)
+                {
+                    throw new AggregateException(message, exceptions);
+                }
+                catch (Exception e)
+                {
+                    exceptions.Add(e);
+                }
+
+                try
+                {
+                    await Task.Delay(pollIntervalMillis, cts.Token).ConfigureAwait(false);
+                }
+                catch (TaskCanceledException)
+                {
+                    throw new AggregateException(message, exceptions);
+                }
+            }
+            throw new AggregateException(message, exceptions);
+        }
+    }
+}

test/OpenFeature.Tests/TestUtilsTest.cs

@@ -0,0 +1,23 @@
+using System;
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using OpenFeature.Model;
+using Xunit;
+
+namespace OpenFeature.Tests
+{
+    public class TestUtilsTest
+    {
+        [Fact]
+        public async void Should_Fail_If_Assertion_Fails()
+        {
+            await Assert.ThrowsAnyAsync<Exception>(() => Utils.AssertUntilAsync(_ => Assert.True(1.Equals(2)), 100, 10)).ConfigureAwait(false);
+        }
+
+        [Fact]
+        public async void Should_Pass_If_Assertion_Fails()
+        {
+            await Utils.AssertUntilAsync(_ => Assert.True(1.Equals(1))).ConfigureAwait(false);
+        }
+    }
+}