Raise exception for invalid parameters in StarZone

We've talked about using exceptions for invalid parameters in user'
calls.
Adjust for StarZone.
Bonus: We can test it in UT with catch

diff --git a/src/kernel/routing/StarZone.cpp b/src/kernel/routing/StarZone.cpp
index c311f34..72fb1e4 100644 (file)
--- a/src/kernel/routing/StarZone.cpp
+++ b/src/kernel/routing/StarZone.cpp
@@ -8,6 +8,7 @@
 #include "simgrid/kernel/routing/RoutedZone.hpp"
 #include "src/surf/network_interface.hpp"
 #include "src/surf/xml/platf_private.hpp" // RouteCreationArgs and friends
+#include "xbt/string.hpp"
 
 XBT_LOG_NEW_DEFAULT_SUBCATEGORY(surf_route_star, surf, "Routing part of surf");
 
@@ -92,25 +93,35 @@ void StarZone::check_add_route_param(const NetPoint* src, const NetPoint* dst, c
   const char* src_name = src ? src->get_cname() : "nullptr";
   const char* dst_name = dst ? dst->get_cname() : "nullptr";
 
-  xbt_assert((src || dst) && (not dst || not src || src == dst),
-             "Cannot add route from %s to %s. In a StarZone, route must be:  i) from source host to everyone, ii) from "
-             "everyone to a single host or iii) loopback, same source and destination",
-             src_name, dst_name);
-  xbt_assert(not symmetrical || src,
-             "Cannot add route from %s to %s. In a StarZone, symmetrical routes must be set from source to everyone "
-             "(not the contrary).",
-             src_name, dst_name);
+  if ((not src && not dst) || (dst && src && src != dst))
+    throw std::invalid_argument(xbt::string_printf(
+        "Cannot add route from %s to %s. In a StarZone, route must be:  i) from source netpoint to everyone, ii) from "
+        "everyone to a single netpoint or iii) loopback, same source and destination",
+        src_name, dst_name));
+
+  if (symmetrical && not src)
+    throw std::invalid_argument(xbt::string_printf("Cannot add route from %s to %s. In a StarZone, symmetrical routes "
+                                                   "must be set from source to everyone (not the contrary)",
+                                                   src_name, dst_name));
 
   if (src && src->is_netzone()) {
-    xbt_assert(gw_src, "add_route(): source %s is a netzone but gw_src isn't configured", src->get_cname());
-    xbt_assert(not gw_src->is_netzone(), "add_route(): src(%s) is a netzone, gw_src(%s) cannot be a netzone",
-               src->get_cname(), gw_src->get_cname());
+    if (not gw_src)
+      throw std::invalid_argument(xbt::string_printf(
+          "StarZone::add_route(): source %s is a netzone but gw_src isn't configured", src->get_cname()));
+    if (gw_src->is_netzone())
+      throw std::invalid_argument(
+          xbt::string_printf("StarZone::add_route(): src(%s) is a netzone, gw_src(%s) cannot be a netzone",
+                             src->get_cname(), gw_src->get_cname()));
   }
 
   if (dst && dst->is_netzone()) {
-    xbt_assert(gw_dst, "add_route(): destination %s is a netzone but gw_dst isn't configured", dst->get_cname());
-    xbt_assert(not gw_dst->is_netzone(), "add_route(): dst(%s) is a netzone, gw_dst(%s) cannot be a netzone",
-               dst->get_cname(), gw_dst->get_cname());
+    if (not gw_dst)
+      throw std::invalid_argument(xbt::string_printf(
+          "StarZone::add_route(): destination %s is a netzone but gw_dst isn't configured", dst->get_cname()));
+    if (gw_dst->is_netzone())
+      throw std::invalid_argument(
+          xbt::string_printf("StarZone::add_route(): dst(%s) is a netzone, gw_dst(%s) cannot be a netzone",
+                             dst->get_cname(), gw_dst->get_cname()));
   }
 }
 

diff --git a/src/kernel/routing/StarZone_test.cpp b/src/kernel/routing/StarZone_test.cpp
index 46126ba..849489c 100644 (file)
--- a/src/kernel/routing/StarZone_test.cpp
+++ b/src/kernel/routing/StarZone_test.cpp
@@ -32,23 +32,30 @@ TEST_CASE("kernel::routing::StarZone: Creating Zone", "[creation]")
   REQUIRE(simgrid::s4u::create_star_zone("test"));
 }
 
-// One day we may be able to test contracts and asserts with catch2
-// https://github.com/catchorg/Catch2/issues/853
-TEST_CASE("kernel::routing::StarZone: Adding routes (hosts): assert", "[.][assert]")
+TEST_CASE("kernel::routing::StarZone: Adding routes (hosts): exception", "")
 {
   EngineWrapper e("test");
   auto* zone      = new simgrid::kernel::routing::StarZone("test");
   auto* netpoint1 = new simgrid::kernel::routing::NetPoint("netpoint1", simgrid::kernel::routing::NetPoint::Type::Host);
   auto* netpoint2 = new simgrid::kernel::routing::NetPoint("netpoint2", simgrid::kernel::routing::NetPoint::Type::Host);
 
-  SECTION("src and dst: nullptr") { zone->add_route(nullptr, nullptr, nullptr, nullptr, {}, false); }
+  SECTION("src and dst: nullptr")
+  {
+    REQUIRE_THROWS_AS(zone->add_route(nullptr, nullptr, nullptr, nullptr, {}, false), std::invalid_argument);
+  }
 
-  SECTION("src: nullptr and symmetrical: true") { zone->add_route(nullptr, netpoint2, nullptr, nullptr, {}, true); }
+  SECTION("src: nullptr and symmetrical: true")
+  {
+    REQUIRE_THROWS_AS(zone->add_route(nullptr, netpoint2, nullptr, nullptr, {}, true), std::invalid_argument);
+  }
 
-  SECTION("src and dst: not nullptr") { zone->add_route(netpoint1, netpoint2, nullptr, nullptr, {}, false); }
+  SECTION("src and dst: not nullptr")
+  {
+    REQUIRE_THROWS_AS(zone->add_route(netpoint1, netpoint2, nullptr, nullptr, {}, false), std::invalid_argument);
+  }
 }
 
-TEST_CASE("kernel::routing::StarZone: Adding routes (netzones): assert", "[.][assert]")
+TEST_CASE("kernel::routing::StarZone: Adding routes (netzones): exception", "")
 {
   EngineWrapper e("test");
   auto* zone = new simgrid::kernel::routing::StarZone("test");
@@ -57,21 +64,29 @@ TEST_CASE("kernel::routing::StarZone: Adding routes (netzones): assert", "[.][as
   auto* netpoint2 =
       new simgrid::kernel::routing::NetPoint("netpoint2", simgrid::kernel::routing::NetPoint::Type::NetZone);
 
-  SECTION("src: is a netzone and gw_src: nullptr") { zone->add_route(netpoint1, nullptr, nullptr, nullptr, {}, false); }
+  SECTION("src: is a netzone and gw_src: nullptr")
+  {
+    REQUIRE_THROWS_AS(zone->add_route(netpoint1, nullptr, nullptr, nullptr, {}, false), std::invalid_argument);
+  }
 
   SECTION("src: is a netzone and gw_src: is a netzone")
   {
-    zone->add_route(netpoint1, nullptr, netpoint2, nullptr, {}, false);
+    REQUIRE_THROWS_AS(zone->add_route(netpoint1, nullptr, netpoint2, nullptr, {}, false), std::invalid_argument);
   }
 
-  SECTION("dst: is a netzone and gw_dst: nullptr") { zone->add_route(nullptr, netpoint2, nullptr, nullptr, {}, false); }
+  SECTION("dst: is a netzone and gw_dst: nullptr")
+  {
+    REQUIRE_THROWS_AS(zone->add_route(nullptr, netpoint2, nullptr, nullptr, {}, false), std::invalid_argument);
+  }
 
   SECTION("dst: is a netzone and gw_dst: is a netzone")
   {
-    zone->add_route(nullptr, netpoint2, nullptr, netpoint1, {}, false);
+    REQUIRE_THROWS_AS(zone->add_route(nullptr, netpoint2, nullptr, netpoint1, {}, false), std::invalid_argument);
   }
 }
 
+// One day we may be able to test contracts and asserts with catch2
+// https://github.com/catchorg/Catch2/issues/853
 TEST_CASE("kernel::routing::StarZone: Get routes: assert", "[.][assert]")
 {
   /* workaround to initialize things, they must be done in this particular order */