#268 fix flakey acceptance test (#282)

* #268 added waiter to test, altho i wasn't able to replicate flakeyness with wait anyway! Hopefully this will be solid now!

* #268 fixed a warning

* #268 more code coverage

src/Ocelot/Configuration/Repository/ConsulFileConfigurationPoller.cs

@@ -17,10 +17,16 @@ namespace Ocelot.Configuration.Repository
         private string _previousAsJson;
         private readonly Timer _timer;
         private bool _polling;
+        private readonly IConsulPollerConfiguration _config;
 
-        public ConsulFileConfigurationPoller(IOcelotLoggerFactory factory, IFileConfigurationRepository repo, IFileConfigurationSetter setter)
+        public ConsulFileConfigurationPoller(
+            IOcelotLoggerFactory factory, 
+            IFileConfigurationRepository repo, 
+            IFileConfigurationSetter setter, 
+            IConsulPollerConfiguration config)
         {
             _setter = setter;
+            _config = config;
             _logger = factory.CreateLogger<ConsulFileConfigurationPoller>();
             _repo = repo;
             _previousAsJson = "";
@@ -34,7 +40,7 @@ namespace Ocelot.Configuration.Repository
                 _polling = true;
                 await Poll();
                 _polling = false;
-            }, null, 0, 1000);
+            }, null, 0, _config.Delay);
         }
         
         private async Task Poll()
@@ -63,8 +69,7 @@ namespace Ocelot.Configuration.Repository
         /// <summary>
         /// We could do object comparison here but performance isnt really a problem. This might be an issue one day!
         /// </summary>
-        /// <param name="config"></param>
+        /// <returns>hash of the config</returns>
-        /// <returns></returns>
         private string ToJson(FileConfiguration config)
         {
             var currentHash = JsonConvert.SerializeObject(config);

src/Ocelot/Configuration/Repository/IConsulPollerConfiguration.cs

@@ -0,0 +1,8 @@
+namespace Ocelot.Configuration.Repository
+{
+    public interface IConsulPollerConfiguration
+    {
+        int Delay { get; }
+    }
+
+    }

src/Ocelot/Configuration/Repository/InMemoryConsulPollerConfiguration.cs

@@ -0,0 +1,7 @@
+namespace Ocelot.Configuration.Repository
+{
+    public class InMemoryConsulPollerConfiguration : IConsulPollerConfiguration
+    {
+        public int Delay => 1000;
+    }
+}

src/Ocelot/DependencyInjection/OcelotBuilder.cs

@@ -148,6 +148,7 @@ namespace Ocelot.DependencyInjection
 
             // We add this here so that we can always inject something into the factory for IoC..
             _services.AddSingleton<IServiceTracer, FakeServiceTracer>();
+            _services.TryAddSingleton<IConsulPollerConfiguration, InMemoryConsulPollerConfiguration>();
         }
 
         public IOcelotAdministrationBuilder AddAdministration(string path, string secret)

src/Ocelot/Infrastructure/Wait.cs

@@ -1,4 +1,4 @@
-namespace Ocelot.UnitTests
+namespace Ocelot.Infrastructure
 {
     public class Wait
     {

src/Ocelot/Infrastructure/Waiter.cs

@@ -1,7 +1,7 @@
 using System;
 using System.Diagnostics;
 
-namespace Ocelot.UnitTests
+namespace Ocelot.Infrastructure
 {
     public class Waiter
     {

test/Ocelot.AcceptanceTests/ConfigurationInConsulTests.cs

@@ -9,8 +9,10 @@ using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Http;
 using Newtonsoft.Json;
 using Ocelot.Configuration.File;
+using Shouldly;
 using TestStack.BDDfy;
 using Xunit;
+using static Ocelot.Infrastructure.Wait;
 
 namespace Ocelot.AcceptanceTests
 {
@@ -261,21 +263,27 @@ namespace Ocelot.AcceptanceTests
                 .And(x => _steps.WhenIGetUrlOnTheApiGateway("/cs/status"))
                 .And(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK))
                 .And(x => _steps.ThenTheResponseBodyShouldBe("Hello from Laura"))
-                .And(x => GivenTheConsulConfigurationIs(secondConsulConfig))
+                .When(x => GivenTheConsulConfigurationIs(secondConsulConfig))
-                .And(x => GivenIWaitForTheConfigToReplicateToOcelot())
+                .Then(x => ThenTheConfigIsUpdatedInOcelot())
-                .When(x => _steps.WhenIGetUrlOnTheApiGateway("/cs/status/awesome"))
-                .Then(x => _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK))
-                .And(x => _steps.ThenTheResponseBodyShouldBe("Hello from Laura"))
                 .BDDfy();
         }
 
-        private void GivenIWaitForTheConfigToReplicateToOcelot()
+        private void ThenTheConfigIsUpdatedInOcelot()
         {
-            var stopWatch = Stopwatch.StartNew();
+            var result = WaitFor(20000).Until(() => {
-            while (stopWatch.ElapsedMilliseconds < 10000)
+                try
-            {
+                {
-                //do nothing!
+                    _steps.WhenIGetUrlOnTheApiGateway("/cs/status/awesome");
-            }
+                    _steps.ThenTheStatusCodeShouldBe(HttpStatusCode.OK);
+                    _steps.ThenTheResponseBodyShouldBe("Hello from Laura");
+                    return true;
+                }
+                catch (Exception)
+                {
+                    return false;
+                }
+            });
+            result.ShouldBeTrue();
         }
 
         private void GivenTheConsulConfigurationIs(FileConfiguration config)

test/Ocelot.UnitTests/Configuration/ConsulFileConfigurationPollerTests.cs

@@ -1,16 +1,17 @@
 using System;
 using System.Collections.Generic;
-using System.Diagnostics;
+using System.Threading;
 using Moq;
 using Ocelot.Configuration.File;
 using Ocelot.Configuration.Repository;
 using Ocelot.Configuration.Setter;
 using Ocelot.Logging;
 using Ocelot.Responses;
+using Ocelot.UnitTests.Responder;
 using TestStack.BDDfy;
 using Xunit;
 using Shouldly;
-using static Ocelot.UnitTests.Wait;
+using static Ocelot.Infrastructure.Wait;
 
 namespace Ocelot.UnitTests.Configuration
 {
@@ -21,6 +22,7 @@ namespace Ocelot.UnitTests.Configuration
         private Mock<IFileConfigurationRepository> _repo;
         private Mock<IFileConfigurationSetter> _setter;
         private FileConfiguration _fileConfig;
+        private Mock<IConsulPollerConfiguration> _config;
 
         public ConsulFileConfigurationPollerTests()
         {
@@ -30,8 +32,10 @@ namespace Ocelot.UnitTests.Configuration
             _repo = new Mock<IFileConfigurationRepository>();
             _setter = new Mock<IFileConfigurationSetter>();
             _fileConfig = new FileConfiguration();
+            _config = new Mock<IConsulPollerConfiguration>();
             _repo.Setup(x => x.Get()).ReturnsAsync(new OkResponse<FileConfiguration>(_fileConfig));
-            _poller = new ConsulFileConfigurationPoller(_factory.Object, _repo.Object, _setter.Object);
+            _config.Setup(x => x.Delay).Returns(10);
+            _poller = new ConsulFileConfigurationPoller(_factory.Object, _repo.Object, _setter.Object, _config.Object);
         }
 
         public void Dispose()
@@ -42,7 +46,7 @@ namespace Ocelot.UnitTests.Configuration
         [Fact]
         public void should_start()
         {
-           this.Given(x => ThenTheSetterIsCalled(_fileConfig))
+           this.Given(x => ThenTheSetterIsCalled(_fileConfig, 1))
                 .BDDfy();
         }
 
@@ -65,22 +69,82 @@ namespace Ocelot.UnitTests.Configuration
                 }
             };
 
-            this.Given(x => WhenTheConfigIsChangedInConsul(newConfig))
+            this.Given(x => WhenTheConfigIsChangedInConsul(newConfig, 0))
-                .Then(x => ThenTheSetterIsCalled(newConfig))
+                .Then(x => ThenTheSetterIsCalled(newConfig, 1))
                 .BDDfy();
         }
 
-        private void WhenTheConfigIsChangedInConsul(FileConfiguration newConfig)
+        [Fact]
+        public void should_not_poll_if_already_polling()
         {
-            _repo.Setup(x => x.Get()).ReturnsAsync(new OkResponse<FileConfiguration>(newConfig));
+            var newConfig = new FileConfiguration
+            {
+                ReRoutes = new List<FileReRoute>
+                {
+                    new FileReRoute
+                    {
+                        DownstreamHostAndPorts = new List<FileHostAndPort>
+                        {
+                            new FileHostAndPort
+                            {
+                                Host = "test"
+                            }
+                        },
+                    }
+                }
+            };
+
+            this.Given(x => WhenTheConfigIsChangedInConsul(newConfig, 10))
+                .Then(x => ThenTheSetterIsCalled(newConfig, 1))
+                .BDDfy();
         }
 
-        private void ThenTheSetterIsCalled(FileConfiguration fileConfig)
+        [Fact]
+        public void should_do_nothing_if_call_to_consul_fails()
+        {
+            var newConfig = new FileConfiguration
+            {
+                ReRoutes = new List<FileReRoute>
+                {
+                    new FileReRoute
+                    {
+                        DownstreamHostAndPorts = new List<FileHostAndPort>
+                        {
+                            new FileHostAndPort
+                            {
+                                Host = "test"
+                            }
+                        },
+                    }
+                }
+            };
+
+            this.Given(x => WhenConsulErrors())
+                .Then(x => ThenTheSetterIsCalled(newConfig, 0))
+                .BDDfy();
+        }
+
+        private void WhenConsulErrors()
+        {
+            _repo
+                .Setup(x => x.Get())
+                .ReturnsAsync(new ErrorResponse<FileConfiguration>(new AnyError()));
+        }
+
+        private void WhenTheConfigIsChangedInConsul(FileConfiguration newConfig, int delay)
+        {
+            _repo
+                .Setup(x => x.Get())
+                .Callback(() => Thread.Sleep(delay))
+                .ReturnsAsync(new OkResponse<FileConfiguration>(newConfig));
+        }
+
+        private void ThenTheSetterIsCalled(FileConfiguration fileConfig, int times)
         {
             var result = WaitFor(2000).Until(() => {
                 try
                 {
-                    _setter.Verify(x => x.Set(fileConfig), Times.Once);
+                    _setter.Verify(x => x.Set(fileConfig), Times.Exactly(times));
                     return true;
                 }
                 catch(Exception)

test/Ocelot.UnitTests/ServerHostedMiddlewareTest.cs

@@ -1,68 +0,0 @@
-namespace Ocelot.UnitTests
-{
-    using System;
-    using System.IO;
-    using System.Net.Http;
-    using Microsoft.AspNetCore.TestHost;
-    using Microsoft.AspNetCore.Hosting;
-    using Microsoft.Extensions.DependencyInjection;
-    using Microsoft.AspNetCore.Builder;
-    using Moq;
-    using Ocelot.Infrastructure.RequestData;
-
-    public abstract class ServerHostedMiddlewareTest : IDisposable
-    {
-        protected TestServer Server { get; private set; }
-        protected HttpClient Client { get; private set; }
-        protected string Url { get; private set; }
-        protected HttpResponseMessage ResponseMessage { get; private set; }
-        protected Mock<IRequestScopedDataRepository> ScopedRepository { get; private set; }
-
-        public ServerHostedMiddlewareTest()
-        {
-            Url = "http://localhost:51879";
-            ScopedRepository = new Mock<IRequestScopedDataRepository>();
-        }
-
-        protected virtual void GivenTheTestServerIsConfigured()
-        {
-            var builder = new WebHostBuilder()
-              .ConfigureServices(x => GivenTheTestServerServicesAreConfigured(x))
-              .UseUrls(Url)
-              .UseKestrel()
-              .UseContentRoot(Directory.GetCurrentDirectory())
-              .UseIISIntegration()
-              .Configure(app => GivenTheTestServerPipelineIsConfigured(app));
-
-            Server = new TestServer(builder);
-            Client = Server.CreateClient();
-        }
-
-        protected virtual void GivenTheTestServerServicesAreConfigured(IServiceCollection services)
-        {
-            // override this in your test fixture to set up service dependencies
-        }
-
-        protected virtual void GivenTheTestServerPipelineIsConfigured(IApplicationBuilder app)
-        {
-            // override this in your test fixture to set up the test server pipeline
-        }
-
-        protected void WhenICallTheMiddleware()
-        {
-            ResponseMessage = Client.GetAsync(Url).Result;
-        }
-
-        protected void WhenICallTheMiddlewareWithTheRequestIdKey(string requestIdKey, string value)
-        {
-            Client.DefaultRequestHeaders.Add(requestIdKey, value);
-            ResponseMessage = Client.GetAsync(Url).Result;
-        }
-
-        public void Dispose()
-        {
-            Client.Dispose();
-            Server.Dispose();
-        }
-    }
-}