add comments and improve the regression test

test/regress/2020-06-08-key-backup-bug.cpp

@@ -5,8 +5,9 @@
 #include <service/context.hpp>
 #include <catch2/catch.hpp>
 
+/// make a llarp_main* with 1 endpoint that specifies a keyfile
 static llarp_main*
-make_context(fs::path keyfile)
+make_context(std::optional<fs::path> keyfile)
 {
   auto config = llarp_default_config();
   config->impl.network.m_endpointType = "null";
@@ -17,12 +18,19 @@ make_context(fs::path keyfile)
   return ptr;
 }
 
+/// test that we dont back up all keys when self.signed is missing or invalid as client
 TEST_CASE("key backup bug regression test", "[regress]")
 {
+  // kill logging, this code is noisy
   llarp::LogSilencer shutup;
-  for (const fs::path& path : {"regress-1.private", "regress-2.private", ""})
+  // test 2 explicitly provided keyfiles, empty keyfile and no keyfile
+  for (std::optional<fs::path> path : {std::optional<fs::path>{"regress-1.private"},
+                                       std::optional<fs::path>{"regress-2.private"},
+                                       std::optional<fs::path>{""},
+                                       {std::nullopt}})
   {
     llarp::service::Address endpointAddress{};
+    // try 10 start up and shut downs and see if our key changes or not
     for (size_t index = 0; index < 10; index++)
     {
       auto context = make_context(path);
@@ -34,29 +42,33 @@ TEST_CASE("key backup bug regression test", "[regress]")
         if (index == 0)
         {
           REQUIRE(endpointAddress.IsZero());
-          // first iteration, we are getting our identity
+          // first iteration, we are getting our identity that we start with
           endpointAddress = ep->GetIdentity().pub.Addr();
           REQUIRE(not endpointAddress.IsZero());
         }
         else
         {
           REQUIRE(not endpointAddress.IsZero());
-          if (path.empty())
+          if (path.has_value() and not path->empty())
           {
-            // we want the keys to shift
-            REQUIRE(endpointAddress != ep->GetIdentity().pub.Addr());
+            // we have a keyfile provided
+            // after the first iteration we expect the keys to stay the same
+            REQUIRE(endpointAddress == ep->GetIdentity().pub.Addr());
           }
           else
           {
-            // after the first iteration we expect the keys to stay the same
-            REQUIRE(endpointAddress == ep->GetIdentity().pub.Addr());
+            // we want the keys to shift because no keyfile was provided
+            REQUIRE(endpointAddress != ep->GetIdentity().pub.Addr());
           }
         }
+        // close the router "later" so llarp_main_run exits
         ctx->CloseAsync();
       });
       REQUIRE(llarp_main_run(context, llarp_main_runtime_opts{}) == 0);
       llarp_main_free(context);
     }
-    fs::remove(path);
+    // remove keys if provied
+    if (path.has_value() and not path->empty())
+      fs::remove(*path);
   }
 }