[Openmp-commits] [openmp] [OpenMP][omptest] Enforce test case order for 'standalone' build (PR #154977)

[Michael Halkenhäuser via Openmp-commits]

https://github.com/mhalk created https://github.com/llvm/llvm-project/pull/154977

Note: this only applies to 'standalone' builds, i.e. when:
LIBOMPTEST_BUILD_STANDALONE evaluates to 'ON'.

Use std::vector<std::pair<std::string, TestSuite>> instead of a std::map.

Background:
In some cases it could happen that the test execution order would change vs. the order of appearance.
This can lead to suite failures when e.g. testing for device initialization because it is performed by the first executed test case.
By storing the test suites and cases in order of appearance this issue is avoided. (So far GoogleTest has behaved in the same way.)

>From 2a69a66f62982279b8278f0c67b771c9dbd9a5fa Mon Sep 17 00:00:00 2001
From: Michael Halkenhaeuser <MichaelGerald.Halkenhauser at amd.com>
Date: Fri, 22 Aug 2025 11:09:32 -0500
Subject: [PATCH] [OpenMP][omptest] Enforce test case order for 'standalone'
 build

Note: this only applies to 'standalone' builds, i.e. when:
LIBOMPTEST_BUILD_STANDALONE evaluates to 'ON'.

Use std::vector<std::pair<std::string, TestSuite>> instead of a std::map.

Background:
In some cases it could happen that the test execution order would change vs.
the order of appearance. This can lead to suite failures when e.g. testing for
device initialization because it is performed by the first executed test case.
By storing the test suites and cases in order of appearance this issue is avoided.
(So far GoogleTest has behaved in the same way.)
---
 .../omptest/include/OmptTesterStandalone.h    | 13 +++++++-----
 openmp/tools/omptest/src/OmptTester.cpp       |  2 +-
 .../omptest/src/OmptTesterStandalone.cpp      | 21 ++++++++++++++-----
 3 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/openmp/tools/omptest/include/OmptTesterStandalone.h b/openmp/tools/omptest/include/OmptTesterStandalone.h
index 6a0995538e2ca..aca72f7db375f 100644
--- a/openmp/tools/omptest/include/OmptTesterStandalone.h
+++ b/openmp/tools/omptest/include/OmptTesterStandalone.h
@@ -19,7 +19,7 @@
 #include "OmptAsserter.h"
 #include "OmptTesterGlobals.h"
 
-#include <map>
+#include <utility>
 #include <vector>
 
 // Forward declarations.
@@ -60,14 +60,15 @@ struct TestCase {
 /// A pretty crude test suite abstraction
 struct TestSuite {
   using TestCaseVec = std::vector<std::unique_ptr<TestCase>>;
-  std::string Name;
   TestSuite() = default;
+  TestSuite(const std::string &TSName) : Name(TSName) {}
   TestSuite(const TestSuite &O) = delete;
   TestSuite(TestSuite &&O);
   void setup();
   void teardown();
   TestCaseVec::iterator begin();
   TestCaseVec::iterator end();
+  std::string Name;
   TestCaseVec TestCases;
 };
 /// Static class used to register all test cases and provide them to the driver
@@ -75,14 +76,16 @@ class TestRegistrar {
 public:
   static TestRegistrar &get();
   static std::vector<TestSuite> getTestSuites();
-  static void addCaseToSuite(TestCase *TC, std::string TSName);
+  static void addCaseToSuite(TestCase *TC, const std::string &TSName);
 
 private:
   TestRegistrar() = default;
   TestRegistrar(const TestRegistrar &o) = delete;
   TestRegistrar operator=(const TestRegistrar &o) = delete;
-  // Keep tests in order 'of appearance' (top -> bottom), avoid unordered_map
-  static std::map<std::string, TestSuite> Tests;
+  // Keep tests in order 'of appearance', i.e. top -> bottom.
+  // This effectively mimicks the (observed) behavior of GoogleTest.
+  // Avoid maps as they do not have corresponding order guarantees.
+  static std::vector<std::pair<std::string, TestSuite>> Tests;
 };
 /// Hack to register test cases
 struct Registerer {
diff --git a/openmp/tools/omptest/src/OmptTester.cpp b/openmp/tools/omptest/src/OmptTester.cpp
index 32eec51f44fbf..afa96ac1d4b23 100644
--- a/openmp/tools/omptest/src/OmptTester.cpp
+++ b/openmp/tools/omptest/src/OmptTester.cpp
@@ -42,7 +42,7 @@ static OmptEventReporter *EventReporter;
 #define OMPT_BUFFER_REQUEST_SIZE 256
 
 #ifdef OPENMP_LIBOMPTEST_BUILD_STANDALONE
-std::map<std::string, TestSuite> TestRegistrar::Tests;
+std::vector<std::pair<std::string, TestSuite>> TestRegistrar::Tests;
 #endif
 
 static std::atomic<ompt_id_t> NextOpId{0x8000000000000001};
diff --git a/openmp/tools/omptest/src/OmptTesterStandalone.cpp b/openmp/tools/omptest/src/OmptTesterStandalone.cpp
index 660e385d28499..6f13939c92941 100644
--- a/openmp/tools/omptest/src/OmptTesterStandalone.cpp
+++ b/openmp/tools/omptest/src/OmptTesterStandalone.cpp
@@ -15,6 +15,7 @@
 #include "OmptTesterStandalone.h"
 #include "OmptCallbackHandler.h"
 
+#include <algorithm>
 #include <cassert>
 #include <iomanip>
 #include <iostream>
@@ -86,11 +87,21 @@ std::vector<TestSuite> TestRegistrar::getTestSuites() {
   return TSs;
 }
 
-void TestRegistrar::addCaseToSuite(TestCase *TC, std::string TSName) {
-  auto &TS = Tests[TSName];
-  if (TS.Name.empty())
-    TS.Name = TSName;
-  TS.TestCases.emplace_back(TC);
+void TestRegistrar::addCaseToSuite(TestCase *TC, const std::string &TSName) {
+  // Search the test suites for a matching name
+  auto It = std::find_if(Tests.begin(), Tests.end(),
+                         [&](const auto &P) { return P.first == TSName; });
+
+  if (It != Tests.end()) {
+    // Test suite exists: add the test case
+    It->second.TestCases.emplace_back(TC);
+  } else {
+    // Test suite does not exist: construct it and add the test case
+    TestSuite TS(TSName);
+    TS.TestCases.emplace_back(TC);
+    // Move and emplace the suite
+    Tests.emplace_back(TSName, std::move(TS));
+  }
 }
 
 Registerer::Registerer(TestCase *TC, const std::string SuiteName) {