Skip to content

fix: topology failure only validated by assert - #183

Open
andrewwhitecdw wants to merge 1 commit into
NVIDIA:mainfrom
andrewwhitecdw:codequality/reservation-manager-configurator-topology-failure-only-validated-by
Open

fix: topology failure only validated by assert#183
andrewwhitecdw wants to merge 1 commit into
NVIDIA:mainfrom
andrewwhitecdw:codequality/reservation-manager-configurator-topology-failure-only-validated-by

Conversation

@andrewwhitecdw

Copy link
Copy Markdown

This PR addresses the following issue in src/memory/reservation_manager_configurator.cpp: topology failure only validated by assert.

Changes

  • src/memory/reservation_manager_configurator.cpp: topology failure only validated by assert.

Details

--- a/src/memory/reservation_manager_configurator.cpp
+++ b/src/memory/reservation_manager_configurator.cpp
@@ -1,8 +1,9 @@
-std::vector<memory_space_config> reservation_manager_configurator::build() const
-{
-  topology_discovery discovery;
-  [[maybe_unused]] bool status = discovery.discover();
-  assert(status);
-  auto& topology = discovery.get_topology();
-  return build(topology);
-}
+std::vector<memory_space_config> reservation_manager_configurator::build() const
+{
+  topology_discovery discovery;
+  if (!discovery.discover()) {
+    throw std::runtime_error("Failed to discover system topology");
+  }
+  auto& topology = discovery.get_topology();
+  return build(topology);
+}

Tests

  • tests/memory/test_reservation_manager_configurator.cpp
--- /dev/null
+++ b/tests/memory/test_reservation_manager_configurator.cpp
@@ -0,0 +1,25 @@
+/*
+ * SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+#include <cucascade/memory/reservation_manager_configurator.hpp>
+#include <cucascade/memory/topology_discovery.hpp>
+
+#include <gtest/gtest.h>
+
+#include <stdexcept>
+
+namespace cucascade::memory {
+
+TEST(reservation_manager_configurator, build_from_empty_topology_throws)
+{
+  reservation_manager_configurator configurator;
+  configurator.set_number_of_gpus(1);
+
+  system_topology_info empty_topology{};
+
+  EXPECT_THROW(configurator.build(empty_topology), std::runtime_error);
+}
+
+}  // namespace cucascade::memory

Signed-off-by: andrewwhitecdw <andrewwhitecdw@users.noreply.github.com>
@copy-pr-bot

copy-pr-bot Bot commented Aug 13, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant