Cleanup duplicated retry logic. Adds layered and chain tests

DistTestCore/DistTest.cs

@@ -128,16 +128,26 @@ namespace DistTestCore
             return Get().CodexStarter.BringOnline((CodexSetup)codexSetup);
         }
 
+        public IEnumerable<IOnlineCodexNode> GetAllOnlineCodexNodes()
+        {
+            return Get().CodexStarter.RunningGroups.SelectMany(g => g.Nodes);
+        }
+
+        protected BaseLog GetTestLog()
+        {
+            return Get().Log;
+        }
+
         protected void Log(string msg)
         {
             TestContext.Progress.WriteLine(msg);
-            Get().Log.Log(msg);
+            GetTestLog().Log(msg);
         }
 
         protected void Debug(string msg)
         {
             TestContext.Progress.WriteLine(msg);
-            Get().Log.Debug(msg);
+            GetTestLog().Debug(msg);
         }
 
         private TestLifecycle Get()

DistTestCore/Http.cs

@@ -107,25 +107,7 @@ namespace DistTestCore
 
         private T Retry<T>(Func<T> operation)
         {
-            var retryCounter = 0;
+            return Time.Retry(operation, timeSet.HttpCallRetryTimeout(), timeSet.HttpCallRetryDelay());
-
-            while (true)
-            {
-                try
-                {
-                    return operation();
-                }
-                catch (Exception exception)
-                {
-                    timeSet.HttpCallRetryDelay();
-                    retryCounter++;
-                    if (retryCounter > timeSet.HttpCallRetryCount())
-                    {
-                        Assert.Fail(exception.ToString());
-                        throw;
-                    }
-                }
-            }
         }
 
         private static T TryJsonDeserialize<T>(string json)

DistTestCore/Marketplace/ContainerInfoExtractor.cs

@@ -56,30 +56,6 @@ namespace DistTestCore.Marketplace
             return marketplaceAbi;
         }
 
-        private string Retry(Func<string> fetch)
-        {
-            var result = string.Empty;
-            Time.WaitUntil(() =>
-            {
-                result = Catch(fetch);
-                return !string.IsNullOrEmpty(result);
-            }, TimeSpan.FromMinutes(1), TimeSpan.FromSeconds(3));
-
-            return result;
-        }
-
-        private string Catch(Func<string> fetch)
-        {
-            try
-            {
-                return fetch();
-            }
-            catch
-            {
-                return string.Empty;
-            }
-        }
-
         private string FetchAccountsCsv()
         {
             return workflow.ExecuteCommand(container, "cat", GethContainerRecipe.AccountsFilename);
@@ -116,6 +92,11 @@ namespace DistTestCore.Marketplace
             var privateKey = tokens[1];
             return new GethAccount(account, privateKey);
         }
+
+        private static string Retry(Func<string> fetch)
+        {
+            return Time.Retry(fetch);
+        }
     }
 
     public class PubKeyFinder : LogHandler, ILogHandler

DistTestCore/Timing.cs

@@ -1,5 +1,4 @@
 using NUnit.Framework;
-using Utils;
 
 namespace DistTestCore
 {
@@ -11,8 +10,8 @@ namespace DistTestCore
     public interface ITimeSet
     {
         TimeSpan HttpCallTimeout();
-        int HttpCallRetryCount();
+        TimeSpan HttpCallRetryTimeout();
-        void HttpCallRetryDelay();
+        TimeSpan HttpCallRetryDelay();
         TimeSpan WaitForK8sServiceDelay();
         TimeSpan K8sOperationTimeout();
         TimeSpan WaitForMetricTimeout();
@@ -25,14 +24,14 @@ namespace DistTestCore
             return TimeSpan.FromSeconds(10);
         }
 
-        public int HttpCallRetryCount()
+        public TimeSpan HttpCallRetryTimeout()
         {
-            return 5;
+            return TimeSpan.FromSeconds(10);
         }
 
-        public void HttpCallRetryDelay()
+        public TimeSpan HttpCallRetryDelay()
         {
-            Time.Sleep(TimeSpan.FromSeconds(3));
+            return TimeSpan.FromSeconds(3);
         }
 
         public TimeSpan WaitForK8sServiceDelay()
@@ -58,14 +57,14 @@ namespace DistTestCore
             return TimeSpan.FromHours(2);
         }
 
-        public int HttpCallRetryCount()
+        public TimeSpan HttpCallRetryTimeout()
         {
-            return 2;
+            return TimeSpan.FromHours(5);
         }
 
-        public void HttpCallRetryDelay()
+        public TimeSpan HttpCallRetryDelay()
         {
-            Time.Sleep(TimeSpan.FromMinutes(5));
+            return TimeSpan.FromMinutes(5);
         }
 
         public TimeSpan WaitForK8sServiceDelay()

Tests/PeerDiscoveryTests/LayeredDiscoveryTests.cs

@@ -0,0 +1,51 @@
+using DistTestCore;
+using NUnit.Framework;
+
+namespace Tests.PeerDiscoveryTests
+{
+    [TestFixture]
+    public class LayeredDiscoveryTests : DistTest
+    {
+        [Test]
+        public void TwoLayersTest()
+        {
+            var root = SetupCodexNode();
+            var l1Source = SetupCodexNode(s => s.WithBootstrapNode(root));
+            var l1Node = SetupCodexNode(s => s.WithBootstrapNode(root));
+            var l2Target = SetupCodexNode(s => s.WithBootstrapNode(l1Node));
+
+            AssertAllNodesConnected();
+        }
+
+        [Test]
+        public void ThreeLayersTest()
+        {
+            var root = SetupCodexNode();
+            var l1Source = SetupCodexNode(s => s.WithBootstrapNode(root));
+            var l1Node = SetupCodexNode(s => s.WithBootstrapNode(root));
+            var l2Node = SetupCodexNode(s => s.WithBootstrapNode(l1Node));
+            var l3Target = SetupCodexNode(s => s.WithBootstrapNode(l2Node));
+
+            AssertAllNodesConnected();
+        }
+
+        [TestCase(3)]
+        [TestCase(5)]
+        [TestCase(10)]
+        public void NodeChainTest(int chainLength)
+        {
+            var node = SetupCodexNode();
+            for (var i = 1; i < chainLength; i++)
+            {
+                node = SetupCodexNode(s => s.WithBootstrapNode(node));
+            }
+
+            AssertAllNodesConnected();
+        }
+
+        private void AssertAllNodesConnected()
+        {
+            PeerTestHelpers.AssertFullyConnected(GetAllOnlineCodexNodes(), GetTestLog());
+        }
+    }
+}

Tests/PeerDiscoveryTests/PeerDiscoveryTests.cs

@@ -1,7 +1,5 @@
-using DistTestCore.Codex;
+using DistTestCore;
-using DistTestCore;
 using NUnit.Framework;
-using Utils;
 
 namespace Tests.PeerDiscoveryTests
 {
@@ -11,9 +9,9 @@ namespace Tests.PeerDiscoveryTests
         [Test]
         public void TwoNodes()
         {
-            var node = SetupCodexNode();
+            SetupCodexNode();
 
-            AssertKnowEachother(BootstrapNode, node);
+            AssertAllNodesConnected();
         }
 
         [TestCase(2)]
@@ -21,9 +19,9 @@ namespace Tests.PeerDiscoveryTests
         [TestCase(10)]
         public void VariableNodes(int number)
         {
-            var nodes = SetupCodexNodes(number);
+            SetupCodexNodes(number);
 
-            AssertFullyConnected(nodes);
+            AssertAllNodesConnected();
         }
 
         [TestCase(2)]
@@ -31,74 +29,17 @@ namespace Tests.PeerDiscoveryTests
         [TestCase(10)]
         public void VariableNodesInPods(int number)
         {
-            var bootstrap = SetupCodexBootstrapNode();
-
-            var nodes = new List<IOnlineCodexNode>();
             for (var i = 0; i < number; i++)
             {
-                nodes.Add(SetupCodexNode(s => s.WithBootstrapNode(bootstrap)));
+                SetupCodexNode();
             }
 
-            AssertFullyConnected(nodes);
+            AssertAllNodesConnected();
         }
 
-        private void AssertFullyConnected(IEnumerable<IOnlineCodexNode> nodes)
+        private void AssertAllNodesConnected()
         {
-            Retry(() =>
+            PeerTestHelpers.AssertFullyConnected(GetAllOnlineCodexNodes(), GetTestLog());
-            {
-                var array = nodes.ToArray();
-
-                foreach (var node in array) AssertKnowEachother(node, BootstrapNode);
-
-                for (var x = 0; x < array.Length; x++)
-                {
-                    for (var y = x + 1; y < array.Length; y++)
-                    {
-                        AssertKnowEachother(array[x], array[y]);
-                    }
-                }
-            });
-        }
-
-        private static void Retry(Action action)
-        {
-            try
-            {
-                action();
-                return;
-            }
-            catch
-            {
-                Time.Sleep(TimeSpan.FromMinutes(1));
-            }
-
-            action();
-        }
-
-        private void AssertKnowEachother(IOnlineCodexNode a, IOnlineCodexNode b)
-        {
-            AssertKnowEachother(a.GetDebugInfo(), b.GetDebugInfo());
-        }
-
-        private void AssertKnowEachother(CodexDebugResponse a, CodexDebugResponse b)
-        {
-            AssertKnows(a, b);
-            AssertKnows(b, a);
-        }
-
-        private void AssertKnows(CodexDebugResponse a, CodexDebugResponse b)
-        {
-            //var enginePeers = string.Join(",", a.enginePeers.Select(p => p.peerId));
-            //var switchPeers = string.Join(",", a.switchPeers.Select(p => p.peerId));
-            var tableNodes = string.Join(",", a.table.nodes.Select(n => n.nodeId));
-
-            //Debug($"{a.id} is looking for {b.id} in engine-peers [{enginePeers}]");
-            //Debug($"{a.id} is looking for {b.id} in switch-peers [{switchPeers}]");
-            Debug($"{a.table.localNode.nodeId} is looking for {b.table.localNode.nodeId} in table-nodes [{tableNodes}]");
-
-            //Assert.That(a.enginePeers.Any(p => p.peerId == b.id), $"{a.id} was looking for '{b.id}' in engine-peers [{enginePeers}] but it was not found.");
-            //Assert.That(a.switchPeers.Any(p => p.peerId == b.id), $"{a.id} was looking for '{b.id}' in switch-peers [{switchPeers}] but it was not found.");
-            Assert.That(a.table.nodes.Any(n => n.nodeId == b.table.localNode.nodeId), $"{a.table.localNode.nodeId} was looking for '{b.table.localNode.nodeId}' in table-nodes [{tableNodes}] but it was not found.");
         }
     }
 }

Tests/PeerDiscoveryTests/PeerTestHelpers.cs

@@ -0,0 +1,57 @@
+using DistTestCore.Codex;
+using DistTestCore;
+using NUnit.Framework;
+using Utils;
+using Logging;
+
+namespace Tests.PeerDiscoveryTests
+{
+    public static class PeerTestHelpers
+    {
+        public static void AssertFullyConnected(IEnumerable<IOnlineCodexNode> nodes, BaseLog? log = null)
+        {
+            AssertFullyConnected(log, nodes.ToArray());
+        }
+
+        public static void AssertFullyConnected(BaseLog? log = null, params IOnlineCodexNode[] nodes)
+        {
+            Time.Retry(() =>
+            {
+                for (var x = 0; x < nodes.Length; x++)
+                {
+                    for (var y = x + 1; y < nodes.Length; y++)
+                    {
+                        AssertKnowEachother(nodes[x], nodes[y], log);
+                    }
+                }
+            });
+        }
+
+        private static void AssertKnowEachother(IOnlineCodexNode a, IOnlineCodexNode b, BaseLog? log)
+        {
+            AssertKnowEachother(a.GetDebugInfo(), b.GetDebugInfo(), log);
+        }
+
+        private static void AssertKnowEachother(CodexDebugResponse a, CodexDebugResponse b, BaseLog? log)
+        {
+            AssertKnows(a, b, log);
+            AssertKnows(b, a, log);
+        }
+
+        private static void AssertKnows(CodexDebugResponse a, CodexDebugResponse b, BaseLog? log)
+        {
+            //var enginePeers = string.Join(",", a.enginePeers.Select(p => p.peerId));
+            //var switchPeers = string.Join(",", a.switchPeers.Select(p => p.peerId));
+            var tableNodes = string.Join(",", a.table.nodes.Select(n => n.nodeId));
+
+            if (log != null)
+            {
+                log.Debug($"{a.table.localNode.nodeId} is looking for {b.table.localNode.nodeId} in table-nodes [{tableNodes}]");
+            }
+
+            //Assert.That(a.enginePeers.Any(p => p.peerId == b.id), $"{a.id} was looking for '{b.id}' in engine-peers [{enginePeers}] but it was not found.");
+            //Assert.That(a.switchPeers.Any(p => p.peerId == b.id), $"{a.id} was looking for '{b.id}' in switch-peers [{switchPeers}] but it was not found.");
+            Assert.That(a.table.nodes.Any(n => n.nodeId == b.table.localNode.nodeId), $"{a.table.localNode.nodeId} was looking for '{b.table.localNode.nodeId}' in table-nodes [{tableNodes}] but it was not found.");
+        }
+    }
+}

Utils/Time.cs

@@ -38,5 +38,74 @@
                 state = predicate();
             }
         }
+
+        public static void Retry(Action action)
+        {
+            Retry(action, TimeSpan.FromMinutes(1));
+        }
+
+        public static T Retry<T>(Func<T> action)
+        {
+            return Retry(action, TimeSpan.FromMinutes(1));
+        }
+
+        public static void Retry(Action action, TimeSpan timeout)
+        {
+            Retry(action, timeout, TimeSpan.FromSeconds(1));
+        }
+
+        public static T Retry<T>(Func<T> action, TimeSpan timeout)
+        {
+            return Retry(action, timeout, TimeSpan.FromSeconds(1));
+        }
+
+        public static void Retry(Action action, TimeSpan timeout, TimeSpan retryTime)
+        {
+            var start = DateTime.UtcNow;
+            var exceptions = new List<Exception>();
+            while (true)
+            {
+                if (DateTime.UtcNow - start > timeout)
+                {
+                    throw new TimeoutException("Retry timed out.", new AggregateException(exceptions));
+                }
+
+                try
+                {
+                    action();
+                    return;
+                }
+                catch (Exception ex)
+                {
+                    exceptions.Add(ex);
+                }
+
+                Sleep(retryTime);
+            }
+        }
+
+        public static T Retry<T>(Func<T> action, TimeSpan timeout, TimeSpan retryTime)
+        {
+            var start = DateTime.UtcNow;
+            var exceptions = new List<Exception>();
+            while (true)
+            {
+                if (DateTime.UtcNow - start > timeout)
+                {
+                    throw new TimeoutException("Retry timed out.", new AggregateException(exceptions));
+                }
+
+                try
+                {
+                    return action();
+                }
+                catch (Exception ex)
+                {
+                    exceptions.Add(ex);
+                }
+
+                Sleep(retryTime);
+            }
+        }
     }
 }