Skip to content

Commit ec0f6ac

Browse files
committed
[OpenMP][OMPT] Refactorigns to ompTest and improvements
The patch introduces a few minor improvements and name changes to show the more general nature of certain classes. A test no longer exists/aborts execution when an assert fails, but reports an error and continues execution. Change-Id: I43b1af14b061a2e26981b94e2e3021266daf73a0
1 parent c91ab74 commit ec0f6ac

File tree

5 files changed

+55
-35
lines changed

5 files changed

+55
-35
lines changed

openmp/libomptarget/test/ompTest/include/OmptAssertEvent.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@
99
#include <string>
1010

1111
namespace omptest{
12+
13+
enum class AssertState { pass, fail };
14+
1215
struct OmptAssertEvent {
1316
static OmptAssertEvent ThreadBegin(const std::string &Name) {
1417
auto EName = getName(Name);

openmp/libomptarget/test/ompTest/include/OmptAsserter.h

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,15 @@
99

1010
#include <iostream>
1111

12+
/// General base class for the subscriber/notification pattern in
13+
/// OmptCallbachHandler. Derived classes need to implement the notify method.
14+
struct OmptListener {
15+
/// Called for each registered OMPT event of the OmptCallbackHandler
16+
virtual void notify(omptest::OmptAssertEvent &&AE) = 0;
17+
};
18+
1219
/// Base class for asserting on OMPT events
13-
class OmptAsserter {
20+
class OmptAsserter : public OmptListener {
1421
public:
1522
virtual void insert(omptest::OmptAssertEvent &&AE) {
1623
assert(false && "Base class 'insert' has undefined semantics.");
@@ -39,24 +46,29 @@ class OmptAsserter {
3946
/// Class that can assert in a sequenced fashion, i.e., events hace to occur in
4047
/// the order they were registered
4148
struct OmptSequencedAsserter : public OmptAsserter {
49+
OmptSequencedAsserter()
50+
: NextEvent(0), Events(), State(omptest::AssertState::pass) {}
51+
52+
/// Add the event to the in-sequence set of events that the asserter should
53+
/// check for.
4254
void insert(omptest::OmptAssertEvent &&AE) override {
4355
Events.emplace_back(std::move(AE));
4456
}
4557

46-
/// Implements the asserter's logic
58+
/// Implements the asserter's actual logic
4759
virtual void notifyImpl(omptest::OmptAssertEvent &&AE) override {
4860
// Ignore notifications while inactive
4961
if (!isActive())
5062
return;
5163

52-
std::cout << "OmptSequencedAsserter::notifyImpl called w/ " << Events.size()
53-
<< " Events to check.\nNext Check item: " << NextEvent
54-
<< std::endl;
5564
if (NextEvent >= Events.size()) {
56-
std::cerr << "[Error] Too many events to check. Only asserted single "
57-
"event.\nOffending event: "
58-
<< AE.getEventName() << std::endl;
59-
exit(-1); // TODO: Make this reasonable assert error
65+
std::cerr << "[Error] [OmptSequencedAsserter] Too many events to check. "
66+
"Only asserted "
67+
<< Events.size()
68+
<< " event.\nOffending event: " << AE.getEventName()
69+
<< std::endl;
70+
State = omptest::AssertState::fail;
71+
return;
6072
}
6173

6274
auto &E = Events[NextEvent++];
@@ -66,11 +78,21 @@ struct OmptSequencedAsserter : public OmptAsserter {
6678
// TODO: Implement assert error
6779
std::cout << "[ASSERT ERROR] The events are not equal.\n"
6880
<< AE.getEventName() << " == " << E.getEventName() << std::endl;
69-
exit(-2);
81+
State = omptest::AssertState::fail;
82+
}
83+
84+
omptest::AssertState getState() {
85+
// This is called after the testcase executed.
86+
// Once, reached, no more events should be in the queue
87+
if (NextEvent < Events.size())
88+
State = omptest::AssertState::fail;
89+
90+
return State;
7091
}
7192

72-
int NextEvent{0};
93+
size_t NextEvent{0};
7394
std::vector<omptest::OmptAssertEvent> Events;
95+
omptest::AssertState State;
7496
};
7597

7698
/// Class that asserts with set semantics, i.e., unordered

openmp/libomptarget/test/ompTest/include/OmptCallbackHandler.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ struct OmptCallbackHandler {
1717
return Handler;
1818
}
1919

20-
void subscribe(OmptAsserter *Asserter) { Subscribers.push_back(Asserter); }
20+
void subscribe(OmptListener *Listener) { Subscribers.push_back(Listener); }
2121

2222
void clearSubscribers() { Subscribers.clear(); }
2323

@@ -135,6 +135,6 @@ struct OmptCallbackHandler {
135135
ompt_data_t *parallel_data, ompt_data_t *task_data,
136136
uint64_t count, const void *codeptr_ra) {}
137137

138-
std::vector<OmptAsserter *> Subscribers;
138+
std::vector<OmptListener *> Subscribers;
139139
};
140140
#endif

openmp/libomptarget/test/ompTest/include/OmptTester.h

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -29,32 +29,30 @@ ompt_start_tool_result_t *ompt_start_tool(unsigned int omp_version,
2929
#endif
3030

3131
struct Error {
32-
operator bool() { return fail; }
33-
bool fail;
32+
operator bool() { return Fail; }
33+
bool Fail;
3434
};
3535

36-
enum class AssertState { pass, fail };
37-
3836
/// A pretty crude test case abstraction
3937
struct TestCase {
40-
TestCase(const std::string name) : Name(name) {}
38+
TestCase(const std::string &name) : Name(name) {}
4139
virtual ~TestCase() {}
4240
std::string Name;
43-
AssertState assertState;
41+
omptest::AssertState AS;
4442
Error exec() {
4543
OmptCallbackHandler::get().subscribe(&SequenceAsserter);
4644
OmptCallbackHandler::get().subscribe(&EventAsserter);
4745

48-
Error e;
49-
e.fail = false;
46+
Error E;
47+
E.Fail = false;
5048
execImpl();
5149

5250
// We remove subscribers to not be notified of events after our test case
5351
// finished.
5452
OmptCallbackHandler::get().clearSubscribers();
55-
if (assertState == AssertState::fail)
56-
e.fail = true;
57-
return e;
53+
if (SequenceAsserter.getState() == omptest::AssertState::fail)
54+
E.Fail = true;
55+
return E;
5856
};
5957

6058
virtual void execImpl() { assert(false && "Allocating base class"); }
@@ -65,19 +63,16 @@ struct TestCase {
6563
OmptEventAsserter EventAsserter;
6664
};
6765

68-
void failAssertImpl(TestCase &tc) { tc.assertState = AssertState::fail; }
69-
void passAssertImpl(TestCase &tc) { tc.assertState = AssertState::pass; }
70-
7166
/// A pretty crude test suite abstraction
7267
struct TestSuite {
7368
using C = std::vector<std::unique_ptr<TestCase>>;
7469

7570
std::string Name;
7671
TestSuite() = default;
77-
TestSuite(const TestSuite &o) = delete;
78-
TestSuite(TestSuite &&o) {
79-
Name = o.Name;
80-
TestCases.swap(o.TestCases);
72+
TestSuite(const TestSuite &O) = delete;
73+
TestSuite(TestSuite &&O) {
74+
Name = O.Name;
75+
TestCases.swap(O.TestCases);
8176
}
8277

8378
void setup() {}
@@ -139,8 +134,8 @@ struct Runner {
139134
TS.setup();
140135
for (auto &TC : TS) {
141136
std::cout << "\nExecuting " << TC->Name << std::endl;
142-
if (Error err = TC->exec()) {
143-
reportError();
137+
if (Error Err = TC->exec()) {
138+
reportError(Err);
144139
abortOrKeepGoing();
145140
}
146141
}
@@ -149,7 +144,7 @@ struct Runner {
149144
return 0;
150145
}
151146

152-
void reportError() {}
147+
void reportError(const Error &Err) {}
153148
void abortOrKeepGoing() {}
154149

155150
std::vector<TestSuite> TestSuites;

openmp/libomptarget/test/ompTest/src/main.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,4 +76,4 @@ int main(int argc, char **argv) {
7676

7777
std::cout << "Ending" << std::endl;
7878
return 0;
79-
}
79+
}

0 commit comments

Comments
 (0)