iterator.c, muxer: tweak clock compatibility validation error messages

In general, make the prose part of the messages be a bit more in line
with the enumerator names, which usually ends up a bit more specific
than the current messages.

In muxer's msg-iter.cpp:

 - Split the ExpectingOriginUnixGotNone,
   ExpectingOriginUnknownWithUuidGotNone and
   ExpectingOriginUnknownWithoutUuidGotNone, to provide slightly more
   precise error messages about what kind of clock class is expected.

 - Always include the stream class information, whenever there is a
   stream class (message iterator inactivity messages don't have an
   associated stream class).  Factor this out in a `formatStreamCls`
   lambda.

 - Factor out the formatting of the clock class info, and print an
   additional `clock-class-origin-is-unix-epoch` and `clock-class-uuid`
   fields.  This factorisation will be helpful when adding support for
   MIP 1, since the fields to format will change.  We will need to
   update fewer places.

In iterator.c:

 - Print the actual clock class info in a few more places.

 - Change phrases like "has non Unix epoch origin" for "has unknown
   origin", both in human-readable messages and conditions IDs.

 - Tweak the "Expecting a clock class, got none." messages to be a bit
   more specific, differentiating the three cases.

 - Print the expected UUID whenever we expect a clock class with a
   specific UUID.

Instead of cherry-picking the information we print for each case, we
could also print the entire reference clock class information (making it
clear that we don't necessarily expect this particular clock class
instance, perhaps with a `reference-` prefix).  It would probably make
the implementation simpler, but it would print a bit more unnecessary
information.  The user would have to infer from the error message what
is the relevant information in there.  I don't have a strong preference
for either approach.

Change-Id: I0ce2ab5b146a79baa5aa0afbd9d5de4f79f283a1
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Reviewed-on: https://review.lttng.org/c/babeltrace/+/12784
Reviewed-by: Philippe Proulx <eeppeliteloop@gmail.com>
Tested-by: jenkins <jenkins@lttng.org>

diff --git a/src/lib/graph/iterator.c b/src/lib/graph/iterator.c
index d7119ee0d409c070f3b2966d64d8e5100ed0be6d..1724af330ac98a673fb0f62401c965c35ec53cab 100644 (file)
--- a/src/lib/graph/iterator.c
+++ b/src/lib/graph/iterator.c
@@ -696,41 +696,55 @@ void assert_post_dev_clock_classes_are_compatible_one(
                case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_NO_CLOCK_CLASS_GOT_ONE:
                        BT_ASSERT_POST_DEV(NEXT_METHOD_NAME,
                                "stream-class-has-no-clock-class", false,
-                               "Expecting no clock class, got one.");
+                               "Expecting no clock class, got one: %![cc-]+K",
+                               actual_clock_cls);
+
                case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_ORIGIN_UNIX_GOT_NO_CLOCK_CLASS:
                        BT_ASSERT_POST_DEV(NEXT_METHOD_NAME,
                                "stream-class-has-clock-class-with-unix-epoch-origin", false,
-                               "Expecting a clock class, got none.");
+                               "Expecting a clock class with Unix epoch origin, got none.");
+
                case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_ORIGIN_UNIX_GOT_UNKNOWN_ORIGIN:
                        BT_ASSERT_POST_DEV(NEXT_METHOD_NAME,
                                "clock-class-has-unix-epoch-origin", false,
-                               "Expecting a clock class with Unix epoch origin: %![cc-]+K",
+                               "Expecting a clock class with Unix epoch origin, got one with "
+                               "unknown origin: %![cc-]+K",
                                actual_clock_cls);
+
                case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_ORIGIN_UNKNOWN_WITH_UUID_GOT_NO_CLOCK_CLASS:
                        BT_ASSERT_POST_DEV(NEXT_METHOD_NAME,
                                "stream-class-has-clock-class-with-uuid", false,
-                               "Expecting a clock class, got none.");
+                               "Expecting a clock class with unknown origin and a specific UUID, "
+                               "got none: expected-uuid=%!u",
+                               bt_clock_class_get_uuid(ref_clock_cls));
+
                case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_ORIGIN_UNKNOWN_WITH_UUID_GOT_UNIX_ORIGIN:
                        BT_ASSERT_POST_DEV(NEXT_METHOD_NAME,
-                               "clock-class-has-non-unix-epoch-origin", false,
-                               "Expecting a clock class without Unix epoch origin: %![cc-]+K",
-                               actual_clock_cls);
+                               "clock-class-has-unknown-origin", false,
+                               "Expecting a clock class with unknown origin and a specific UUID, "
+                               "got one with Unix epoch origin: %![cc-]+K, expected-uuid=%!u",
+                               actual_clock_cls, bt_clock_class_get_uuid(ref_clock_cls));
+
                case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_ORIGIN_UNKNOWN_WITH_UUID_GOT_WITHOUT_UUID:
                        BT_ASSERT_POST_DEV(NEXT_METHOD_NAME,
                                "clock-class-has-uuid", false,
-                               "Expecting a clock class with UUID:  %![cc-]+K",
-                               actual_clock_cls);
+                               "Expecting a clock class with unknown origin and a specific UUID, "
+                               "got one without a UUID: %![cc-]+K, expected-uuid=%!u",
+                               actual_clock_cls, bt_clock_class_get_uuid(ref_clock_cls));
+
                case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_ORIGIN_UNKNOWN_WITH_UUID_GOT_OTHER_UUID:
                        BT_ASSERT_POST_DEV(NEXT_METHOD_NAME,
                                "clock-class-has-expected-uuid", false,
-                               "Expecting a clock class with UUID, got one with a different UUID: "
-                               "%![cc-]+K, expected-uuid=%!u",
+                               "Expecting a clock class with unknown origin and a specific UUID, "
+                               "got one with a different UUID: %![cc-]+K, expected-uuid=%!u",
                                actual_clock_cls, bt_clock_class_get_uuid(ref_clock_cls));
 
                case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_ORIGIN_UNKNOWN_WITHOUT_UUID_GOT_NO_CLOCK_CLASS:
                        BT_ASSERT_POST_DEV(NEXT_METHOD_NAME,
                                "stream-class-has-clock-class", false,
-                               "Expecting a clock class, got none.");
+                               "Expecting a clock class, got none: %![expected-cc-]+K",
+                               ref_clock_cls);
+
                case BT_CLOCK_CORRELATION_VALIDATOR_ERROR_TYPE_EXPECTING_ORIGIN_UNKNOWN_WITHOUT_UUID_GOT_OTHER_CLOCK_CLASS:
                        BT_ASSERT_POST_DEV(NEXT_METHOD_NAME,
                                "clock-class-is-expected", false,

diff --git a/src/plugins/utils/muxer/msg-iter.cpp b/src/plugins/utils/muxer/msg-iter.cpp
index cc7198987f1f735829b6a271ee97e8a219711b75..3cdc1a54640d880f4f63f112564d22cc945808bb 100644 (file)
--- a/src/plugins/utils/muxer/msg-iter.cpp
+++ b/src/plugins/utils/muxer/msg-iter.cpp
@@ -252,62 +252,98 @@ void MsgIter::_validateMsgClkCls(const bt2::ConstMessage msg)
 
         const auto actualClkCls = error.actualClockCls();
         const auto refClkCls = error.refClockCls();
+        const auto formatClkClsOrigin = [](const bt2::ConstClockClass clockCls,
+                                           const char * const prefix) {
+            return fmt::format("{}clock-class-origin-is-unix-epoch={}", prefix,
+                               clockCls.origin().isUnixEpoch());
+        };
+        const auto formatClkClsUuid = [](const bt2::ConstClockClass clockCls,
+                                         const char * const prefix) {
+            if (const auto uuid = clockCls.uuid()) {
+                return fmt::format("{}clock-class-uuid={}", prefix, *uuid);
+            } else {
+                return fmt::format("{}clock-class-uuid=(none)", prefix);
+            }
+        };
+        const auto formatExpClkClsUuid = [&] {
+            return formatClkClsUuid(*refClkCls, "expected-");
+        };
+        const auto formatClkCls = [&](const bt2::ConstClockClass clockCls,
+                                      const char * const prefix) {
+            return fmt::format("{}clock-class-addr={}, {}clock-class-name={}, {}, {}", prefix,
+                               fmt::ptr(clockCls.libObjPtr()), prefix, clockCls.name(),
+                               formatClkClsOrigin(clockCls, prefix),
+                               formatClkClsUuid(clockCls, prefix));
+        };
+        const auto formatActClkCls = [&] {
+            return formatClkCls(*actualClkCls, "");
+        };
+        const auto formatExpClkCls = [&] {
+            return formatClkCls(*refClkCls, "expected-");
+        };
+        const auto formatStreamCls = [&](const bool withTrailingComma) {
+            if (const auto streamCls = error.streamCls()) {
+                return fmt::format(
+                    "stream-class-addr={}, stream-class-name=\"{}\", stream-class-id={}{}",
+                    fmt::ptr(streamCls->libObjPtr()), streamCls->name(), streamCls->id(),
+                    withTrailingComma ? ", " : "");
+            } else {
+                return std::string {};
+            }
+        };
 
         switch (error.type()) {
         case Type::ExpectingNoClockClassGotOne:
-            BT_CPPLOGE_APPEND_CAUSE_AND_THROW(
-                bt2::Error,
-                "Expecting no clock class, but got one: clock-class-addr={}, clock-class-name={}",
-                fmt::ptr(actualClkCls->libObjPtr()), actualClkCls->name());
+            BT_CPPLOGE_APPEND_CAUSE_AND_THROW(bt2::Error, "Expecting no clock class, got one: {}{}",
+                                              formatStreamCls(true), formatActClkCls());
 
         case Type::ExpectingOriginUnixGotNoClockClass:
-        case Type::ExpectingOriginUnknownWithUuidGotNoClockClass:
-        case Type::ExpectingOriginUnknownWithoutUuidGotNoClockClass:
             BT_CPPLOGE_APPEND_CAUSE_AND_THROW(
-                bt2::Error,
-                "Expecting a clock class, but got none: stream-class-addr={}, "
-                "stream-class-name=\"{}\", stream-class-id={}",
-                fmt::ptr(error.streamCls()->libObjPtr()), error.streamCls()->name(),
-                error.streamCls()->id());
+                bt2::Error, "Expecting a clock class with Unix epoch origin, got none: {}",
+                formatStreamCls(false));
 
         case Type::ExpectingOriginUnixGotUnknownOrigin:
             BT_CPPLOGE_APPEND_CAUSE_AND_THROW(
                 bt2::Error,
-                "Expecting a clock class having a Unix epoch origin, but got one not having a Unix "
-                "epoch origin: clock-class-addr={}, clock-class-name={}",
-                fmt::ptr(actualClkCls->libObjPtr()), actualClkCls->name());
+                "Expecting a clock class with Unix epoch origin, got one with unknown "
+                "origin: {}{}",
+                formatStreamCls(true), formatActClkCls());
+
+        case Type::ExpectingOriginUnknownWithUuidGotNoClockClass:
+            BT_CPPLOGE_APPEND_CAUSE_AND_THROW(
+                bt2::Error,
+                "Expecting a clock class with unknown origin and a specific UUID, got none: {}",
+                formatStreamCls(true), formatExpClkClsUuid());
 
         case Type::ExpectingOriginUnknownWithUuidGotUnixOrigin:
             BT_CPPLOGE_APPEND_CAUSE_AND_THROW(
                 bt2::Error,
-                "Expecting a clock class not having a Unix epoch origin, "
-                "but got one having a Unix epoch origin: "
-                "clock-class-addr={}, clock-class-name={}",
-                fmt::ptr(actualClkCls->libObjPtr()), actualClkCls->name());
+                "Expecting a clock class with unknown origin and a specific UUID, got one "
+                "with Unix epoch origin: {}{}, {}",
+                formatStreamCls(true), formatActClkCls(), formatExpClkClsUuid());
 
         case Type::ExpectingOriginUnknownWithUuidGotWithoutUuid:
             BT_CPPLOGE_APPEND_CAUSE_AND_THROW(
                 bt2::Error,
-                "Expecting a clock class with a UUID, but got one without a UUID: "
-                "clock-class-addr={}, clock-class-name={}",
-                fmt::ptr(actualClkCls->libObjPtr()), actualClkCls->name());
+                "Expecting a clock class with unknown origin and a specific UUID, got one "
+                "without a UUID: {}{}, {}",
+                formatStreamCls(true), formatActClkCls(), formatExpClkClsUuid());
 
         case Type::ExpectingOriginUnknownWithUuidGotOtherUuid:
             BT_CPPLOGE_APPEND_CAUSE_AND_THROW(
                 bt2::Error,
-                "Expecting a clock class with a specific UUID, but got one with a different UUID: "
-                "clock-class-addr={}, clock-class-name={}, expected-uuid=\"{}\", uuid=\"{}\"",
-                fmt::ptr(actualClkCls->libObjPtr()), actualClkCls->name(), *refClkCls->uuid(),
-                *actualClkCls->uuid());
+                "Expecting a clock class with unknown origin and a specific UUID, got one with "
+                "a different UUID: {}{}, {}",
+                formatStreamCls(true), formatActClkCls(), formatExpClkClsUuid());
+
+        case Type::ExpectingOriginUnknownWithoutUuidGotNoClockClass:
+            BT_CPPLOGE_APPEND_CAUSE_AND_THROW(bt2::Error, "Expecting a clock class, got none: {}{}",
+                                              formatStreamCls(true), formatExpClkCls());
 
         case Type::ExpectingOriginUnknownWithoutUuidGotOtherClockClass:
-            BT_CPPLOGE_APPEND_CAUSE_AND_THROW(
-                bt2::Error,
-                "Unexpected clock class: "
-                "expected-clock-class-addr={}, expected-clock-class-name={}, "
-                "actual-clock-class-addr={}, actual-clock-class-name={}",
-                fmt::ptr(refClkCls->libObjPtr()), refClkCls->name(),
-                fmt::ptr(actualClkCls->libObjPtr()), actualClkCls->name());
+            BT_CPPLOGE_APPEND_CAUSE_AND_THROW(bt2::Error, "Unexpected clock class: {}{}, {}",
+                                              formatStreamCls(true), formatActClkCls(),
+                                              formatExpClkCls());
         }
     }
 }

diff --git a/tests/lib/conds/clk-cls-compat-postconds-triggers.cpp b/tests/lib/conds/clk-cls-compat-postconds-triggers.cpp
index 0c7675b4cfa7fabbde50a38f1700b12c4151a467..344298717f8e8d06d4a6b19743f23650458c8f3b 100644 (file)
--- a/tests/lib/conds/clk-cls-compat-postconds-triggers.cpp
+++ b/tests/lib/conds/clk-cls-compat-postconds-triggers.cpp
@@ -190,7 +190,7 @@ void addClkClsCompatTriggers(CondTriggers& triggers)
         [](const bt2::SelfComponent self) {
             return self.createClockClass();
         },
-        "message-iterator-class-next-method:clock-class-has-non-unix-epoch-origin");
+        "message-iterator-class-next-method:clock-class-has-unknown-origin");
 
     addValidCases(
         [](const bt2::SelfComponent self) {

diff --git a/tests/plugins/flt.utils.muxer/test-clock-compatibility.cpp b/tests/plugins/flt.utils.muxer/test-clock-compatibility.cpp
index 29b847bba5359737be9591849194228d25091d0b..02a72be187ef09db1db5ba89f2cb85b5b3ed4c29 100644 (file)
--- a/tests/plugins/flt.utils.muxer/test-clock-compatibility.cpp
+++ b/tests/plugins/flt.utils.muxer/test-clock-compatibility.cpp
@@ -333,13 +333,13 @@ const ErrorTestCase errorTestCases[] = {
      [](const bt2::SelfComponent self) {
          return self.createClockClass();
      },
-     "no clock class followed by clock class", "Expecting no clock class, but got one"},
+     "no clock class followed by clock class", "Expecting no clock class, got one"},
 
     {[](const bt2::SelfComponent self) {
          return self.createClockClass();
      },
      noClockClass, "clock class with Unix epoch origin followed by no clock class",
-     "Expecting a clock class, but got none"},
+     "Expecting a clock class with Unix epoch origin, got none"},
 
     {[](const bt2::SelfComponent self) {
          return self.createClockClass();
@@ -350,8 +350,8 @@ const ErrorTestCase errorTestCases[] = {
          clockCls->originIsUnixEpoch(false);
          return clockCls;
      },
-     "clock class with Unix epoch origin followed by clock class with other origin",
-     "Expecting a clock class having a Unix epoch origin, but got one not having a Unix epoch origin"},
+     "clock class with Unix epoch origin followed by clock class with unknown origin",
+     "Expecting a clock class with Unix epoch origin, got one with unknown origin"},
 
     {[](const bt2::SelfComponent self) {
          const auto clockCls = self.createClockClass();
@@ -359,8 +359,8 @@ const ErrorTestCase errorTestCases[] = {
          clockCls->originIsUnixEpoch(false).uuid(uuidA);
          return clockCls;
      },
-     noClockClass, "clock class with other origin and a UUID followed by no clock class",
-     "Expecting a clock class, but got none"},
+     noClockClass, "clock class with unknown origin and a UUID followed by no clock class",
+     "Expecting a clock class with unknown origin and a specific UUID, got none"},
 
     {[](const bt2::SelfComponent self) {
          const auto clockCls = self.createClockClass();
@@ -371,8 +371,8 @@ const ErrorTestCase errorTestCases[] = {
      [](const bt2::SelfComponent self) {
          return self.createClockClass();
      },
-     "clock class with other origin and a UUID followed by clock class with Unix epoch origin",
-     "Expecting a clock class not having a Unix epoch origin, but got one having a Unix epoch origin"},
+     "clock class with unknown origin and a UUID followed by clock class with Unix epoch origin",
+     "Expecting a clock class with unknown origin and a specific UUID, got one with Unix epoch origin"},
 
     {[](const bt2::SelfComponent self) {
          const auto clockCls = self.createClockClass();
@@ -386,8 +386,8 @@ const ErrorTestCase errorTestCases[] = {
          clockCls->originIsUnixEpoch(false);
          return clockCls;
      },
-     "clock class with other origin and a UUID followed by clock class with other origin and no UUID",
-     "Expecting a clock class with a UUID, but got one without a UUID"},
+     "clock class with unknown origin and a UUID followed by clock class with unknown origin and no UUID",
+     "Expecting a clock class with unknown origin and a specific UUID, got one without a UUID"},
 
     {[](const bt2::SelfComponent self) {
          const auto clockCls = self.createClockClass();
@@ -401,8 +401,8 @@ const ErrorTestCase errorTestCases[] = {
          clockCls->originIsUnixEpoch(false).uuid(uuidB);
          return clockCls;
      },
-     "clock class with other origin and a UUID followed by clock class with other origin and another UUID",
-     "Expecting a clock class with a specific UUID, but got one with a different UUID"},
+     "clock class with unknown origin and a UUID followed by clock class with unknown origin and another UUID",
+     "Expecting a clock class with unknown origin and a specific UUID, got one with a different UUID"},
 
     {[](const bt2::SelfComponent self) {
          const auto clockCls = self.createClockClass();
@@ -410,8 +410,8 @@ const ErrorTestCase errorTestCases[] = {
          clockCls->originIsUnixEpoch(false);
          return clockCls;
      },
-     noClockClass, "clock class with other origin and no UUID followed by no clock class",
-     "Expecting a clock class, but got none"},
+     noClockClass, "clock class with unknown origin and no UUID followed by no clock class",
+     "Expecting a clock class, got none"},
 
     {[](const bt2::SelfComponent self) {
          const auto clockCls = self.createClockClass();
@@ -426,7 +426,7 @@ const ErrorTestCase errorTestCases[] = {
          clockCls->originIsUnixEpoch(false);
          return clockCls;
      },
-     "clock class with other origin and no UUID followed by different clock class",
+     "clock class with unknown origin and no UUID followed by different clock class",
      "Unexpected clock class"},
 };