From 39cdf856b788d0ec84a86e18ea8d037011c75762 Mon Sep 17 00:00:00 2001
From: Agata Momot <agata.momot@intel.com>
Date: Fri, 30 Aug 2024 14:32:51 +0000
Subject: [PATCH] fix: replace UT_ASSERTs with GTEST asserts

canQuery*(...) are changed to void,
since EXPECT_* and ADD_FAILURE() interefere with GTEST_SKIP()
when used in a non-void helper function -
tests are run in spite of labelling as skipped by the fixture.
The success, failure or the need to skip the tests
are determined in the function body using GTEST asserts,
which set the appropriate flags for the current test.
Then the flags are checked in the fixture.

Ref. #569
---
 test/common/base.hpp                          |  2 ++
 test/memspaces/memspace_fixtures.hpp          | 21 ++++++++++++++-----
 test/memspaces/memspace_highest_bandwidth.cpp | 16 +++++++++-----
 test/memspaces/memspace_lowest_latency.cpp    | 16 +++++++++-----
 4 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/test/common/base.hpp b/test/common/base.hpp
index 8f2d5f6be..ea8f04a37 100644
--- a/test/common/base.hpp
+++ b/test/common/base.hpp
@@ -14,6 +14,8 @@
 
 namespace umf_test {
 
+#define IS_SKIPPED_OR_FAILED() (HasFatalFailure() || IsSkipped())
+
 #define NOEXCEPT_COND(cond, val, expected_val)                                                                   \
     try {                                                                                                        \
         cond(val, expected_val);                                                                                 \
diff --git a/test/memspaces/memspace_fixtures.hpp b/test/memspaces/memspace_fixtures.hpp
index d2886ee7e..9bcfc5fbe 100644
--- a/test/memspaces/memspace_fixtures.hpp
+++ b/test/memspaces/memspace_fixtures.hpp
@@ -51,7 +51,7 @@ struct numaNodesTest : ::umf_test::test {
     unsigned long maxNodeId = 0;
 };
 
-using isQuerySupportedFunc = bool (*)(size_t);
+using isQuerySupportedFunc = void (*)(size_t);
 using memspaceGetFunc = umf_const_memspace_handle_t (*)();
 using memspaceGetParams = std::tuple<isQuerySupportedFunc, memspaceGetFunc>;
 
@@ -65,9 +65,10 @@ struct memspaceGetTest : ::numaNodesTest,
         }
 
         auto [isQuerySupported, memspaceGet] = this->GetParam();
+        isQuerySupported(nodeIds.front());
 
-        if (!isQuerySupported(nodeIds.front())) {
-            GTEST_SKIP();
+        if (IS_SKIPPED_OR_FAILED()) {
+            return;
         }
 
         hMemspace = memspaceGet();
@@ -81,8 +82,18 @@ struct memspaceProviderTest : ::memspaceGetTest {
     void SetUp() override {
         ::memspaceGetTest::SetUp();
 
-        if (::memspaceGetTest::IsSkipped()) {
-            GTEST_SKIP();
+        if (numa_available() == -1 || numa_all_nodes_ptr == nullptr) {
+            GTEST_SKIP() << "No available NUMA support; skipped";
+        }
+
+        auto [isQuerySupported, memspaceGet] = ::memspaceGetTest::GetParam();
+        isQuerySupported(nodeIds.front());
+
+        // The test has been marked as skipped in isQuerySupported,
+        // repeating GTEST_SKIP in fixture would only duplicate
+        // the output message
+        if (IS_SKIPPED_OR_FAILED()) {
+            return;
         }
 
         umf_result_t ret =
diff --git a/test/memspaces/memspace_highest_bandwidth.cpp b/test/memspaces/memspace_highest_bandwidth.cpp
index a5bffb41d..5c30696a8 100644
--- a/test/memspaces/memspace_highest_bandwidth.cpp
+++ b/test/memspaces/memspace_highest_bandwidth.cpp
@@ -9,16 +9,17 @@
 #include "memspace_internal.h"
 #include "test_helpers.h"
 
-static bool canQueryBandwidth(size_t nodeId) {
+static void canQueryBandwidth(size_t nodeId) {
     hwloc_topology_t topology = nullptr;
     int ret = hwloc_topology_init(&topology);
-    UT_ASSERTeq(ret, 0);
+    ASSERT_EQ(ret, 0);
+
     ret = hwloc_topology_load(topology);
-    UT_ASSERTeq(ret, 0);
+    ASSERT_EQ(ret, 0);
 
     hwloc_obj_t numaNode =
         hwloc_get_obj_by_type(topology, HWLOC_OBJ_NUMANODE, nodeId);
-    UT_ASSERTne(numaNode, nullptr);
+    ASSERT_NE(numaNode, nullptr);
 
     // Setup initiator structure.
     struct hwloc_location initiator;
@@ -30,7 +31,12 @@ static bool canQueryBandwidth(size_t nodeId) {
                                   numaNode, &initiator, 0, &value);
 
     hwloc_topology_destroy(topology);
-    return (ret == 0);
+
+    if (ret != 0) {
+        GTEST_SKIP()
+            << "Error: hwloc_memattr_get_value return value is equal to " << ret
+            << ", should be " << 0;
+    }
 }
 
 INSTANTIATE_TEST_SUITE_P(memspaceLowestLatencyTest, memspaceGetTest,
diff --git a/test/memspaces/memspace_lowest_latency.cpp b/test/memspaces/memspace_lowest_latency.cpp
index cf921612c..fc35f465a 100644
--- a/test/memspaces/memspace_lowest_latency.cpp
+++ b/test/memspaces/memspace_lowest_latency.cpp
@@ -9,16 +9,17 @@
 #include "memspace_internal.h"
 #include "test_helpers.h"
 
-static bool canQueryLatency(size_t nodeId) {
+static void canQueryLatency(size_t nodeId) {
     hwloc_topology_t topology = nullptr;
     int ret = hwloc_topology_init(&topology);
-    UT_ASSERTeq(ret, 0);
+    ASSERT_EQ(ret, 0);
+
     ret = hwloc_topology_load(topology);
-    UT_ASSERTeq(ret, 0);
+    ASSERT_EQ(ret, 0);
 
     hwloc_obj_t numaNode =
         hwloc_get_obj_by_type(topology, HWLOC_OBJ_NUMANODE, nodeId);
-    UT_ASSERTne(numaNode, nullptr);
+    ASSERT_NE(numaNode, nullptr);
 
     // Setup initiator structure.
     struct hwloc_location initiator;
@@ -30,7 +31,12 @@ static bool canQueryLatency(size_t nodeId) {
                                   &initiator, 0, &value);
 
     hwloc_topology_destroy(topology);
-    return (ret == 0);
+
+    if (ret != 0) {
+        GTEST_SKIP()
+            << "Error: hwloc_memattr_get_value return value is equal to " << ret
+            << ", should be " << 0;
+    }
 }
 
 INSTANTIATE_TEST_SUITE_P(memspaceLowestLatencyTest, memspaceGetTest,