From: Alex Ainscow <aainscow@uk.ibm.com>
Date: Tue, 25 Mar 2025 15:21:07 +0000 (+0000)
Subject: test/osd: Adapt strategy for testing asserts in interval_sets
X-Git-Tag: v21.0.0~256^2~607^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=d7005063be287fd90892eb143251d5d81039913c;p=ceph.git

test/osd: Adapt strategy for testing asserts in interval_sets

Previously the strategy for testing interval_sets was to use a gtest
feature called "EXPECT_DEATH" This is, however, slow and creates
core dumps. Instead, we use a mechanism which replaces the
ceph_assert macro with a strict_assert, which behaves the same in
product code, but in the unit test can be tested using the faster
EXPECT_THROW() mechanism in gtest.

Signed-off-by: Alex Ainscow <aainscow@uk.ibm.com>
---

diff --git a/src/include/interval_set.h b/src/include/interval_set.h
index e93ba7601e28..2adcfb950cd9 100644
--- a/src/include/interval_set.h
+++ b/src/include/interval_set.h
@@ -25,6 +25,14 @@
 
 #include "encoding.h"
 
+/* strict_mode_assert is a standard ceph_assert() in product code. Some tests
+ * (specifically test_interval_set.cc) can override this macro with an
+ * exception.
+ */
+#ifndef strict_mode_assert
+#define strict_mode_assert(expr) ceph_assert(expr)
+#endif
+
 /*
  * *** NOTE ***
  *
@@ -362,7 +370,7 @@ class interval_set {
 
       auto start = std::max<T>(ps->first, pl->first);
       auto en = std::min<T>(ps->first + ps->second, offset);
-      ceph_assert(en > start);
+      strict_mode_assert(en > start);
       mi = m.emplace_hint(mi, start, en - start);
       _size += mi->second;
       if (ps->first + ps->second <= offset) {
@@ -556,7 +564,7 @@ class interval_set {
 
         if (p->first + p->second != start) {
           //cout << "p is " << p->first << "~" << p->second << ", start is " << start << ", len is " << len << endl;
-          ceph_abort();
+          strict_mode_assert(false);
         }
 
         p->second += len;               // append to end
@@ -572,7 +580,7 @@ class interval_set {
 	    *plen = p->second;
           m.erase(n);
         } else {
-          ceph_assert(n == m.end() || start + len < n->first);
+          strict_mode_assert(n == m.end() || start + len < n->first);
 	  if (plen)
 	    *plen = p->second;
 	}
@@ -586,7 +594,7 @@ class interval_set {
           m.erase(p);
           m[start] = len + psecond;  // append to front
         } else {
-          ceph_assert(p->first > start+len);
+          strict_mode_assert(p->first > start+len);
 	  if (pstart)
 	    *pstart = start;
 	  if (plen)
@@ -677,11 +685,11 @@ class interval_set {
 
     _size -= len;
 
-    ceph_assert(p != m.end());
-    ceph_assert(p->first <= start);
+    strict_mode_assert(p != m.end());
+    strict_mode_assert(p->first <= start);
 
     T before = start - p->first;
-    ceph_assert(p->second >= before+len);
+    strict_mode_assert(p->second >= before+len);
     T after = p->second - before - len;
     if (before) {
       if (claim && claim(p->first, before)) {
@@ -999,4 +1007,5 @@ public:
 template<typename T, template<typename, typename, typename ...> class C, bool strict>
 struct fmt::is_range<interval_set<T, C, strict>, char> : std::false_type {};
 
+#undef strict_mode_assert
 #endif
diff --git a/src/test/common/test_interval_set.cc b/src/test/common/test_interval_set.cc
index f415d6127a8b..860e2eabcbe2 100644
--- a/src/test/common/test_interval_set.cc
+++ b/src/test/common/test_interval_set.cc
@@ -17,6 +17,16 @@
 #include <gtest/gtest.h>
 #include <boost/container/flat_map.hpp>
 
+/* This will override the strict_assert macro in interval set, but leave
+ * all product-asserts as ceph_assert. These asserts are used when strict mode
+ * is on to police some important restrictions.
+ */
+#define strict_mode_assert(expr)                           \
+  do {                                                     \
+    ((expr))                                               \
+    ? _CEPH_ASSERT_VOID_CAST (0) : throw std::exception(); \
+  } while (false)
+
 #include "include/interval_set.h"
 #include "include/btree_map.h"
 
@@ -26,10 +36,7 @@ using namespace ceph;
  * if (interval set has strict=true) expect that ceph will panic.
  * else expect that ceph will not panic and execute the second clause.
  */
-// FIXME: The CI pipeline cannot cope with the core dumps that ASSERT_DEATH
-//        creates, so we need to find a better approach. Disabling to unblock.
-//#define ASSERT_STRICT_DEATH(s, e) if constexpr (ISet::test_strict) ASSERT_DEATH(s, ""); else { s; e; }
-#define ASSERT_STRICT_DEATH(s, e) if constexpr (ISet::test_strict) GTEST_SKIP(); else { s; e; }
+#define ASSERT_STRICT_DEATH(s, e) if constexpr (ISet::test_strict) EXPECT_THROW(s, std::exception); else { s; e; }
 
 typedef uint64_t IntervalValueType;