From 5d7e57e846a27172cc3bc5d0fcf5b3e55551e289 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Thu, 15 Feb 2024 23:19:15 -0500 Subject: [PATCH] tests/lib: C++ify run-in and condition trigger code Change the design of the "run in" test util. Currently, the util exposes separate functions taking a function pointer (for all contexts where it's possible to run code in), Change it to a single function accepting a reference to an object of type `RunIn`, a class with virtual methods corresponding to each context. Clients are expected to define classes derived from `RunIn` and override only the methods they need. One advantage of this is that it allows a client to override multiple methods in order to test a scenario that requires executing code in multiple contexts. And it allows it to keep some data in the derived class to use across the callbacks. Adjust `tests/lib/conds/utils.{cpp,hpp}`, which is based on `RunIn`. Replace the open struct `cond_trigger` with: - a base class `CondTrigger`, with a virtual pure `operator()`. - an implementation `SimpleCondTrigger`, which executes a simple callback in `operator()`. - an implementation `RunInCondTrigger`, which calls `runIn` in `operator()`, with a user-provided `RunIn` object. Update `conds-triggers.cpp`, which is based on `utils.{cpp,hpp}`. Implement the condition triggers using the C++ bindings. Do some more miscellaneous C++ification in `utils.cpp` and `conds-triggers.cpp`. Change-Id: I483dd1a49bc7fb1e1fc1b19e0fc01b97d4ea627f Signed-off-by: Simon Marchi Reviewed-on: https://review.lttng.org/c/babeltrace/+/11795 Tested-by: jenkins Reviewed-by: Philippe Proulx --- tests/lib/conds/conds-triggers.cpp | 101 +++++++++++---- tests/lib/conds/utils.cpp | 68 +++++----- tests/lib/conds/utils.hpp | 196 ++++++++++++++++++++++------- tests/lib/test-fields-bin.cpp | 13 +- tests/lib/utils/run-in.cpp | 78 ++++-------- tests/lib/utils/run-in.hpp | 55 ++++---- 6 files changed, 315 insertions(+), 196 deletions(-) diff --git a/tests/lib/conds/conds-triggers.cpp b/tests/lib/conds/conds-triggers.cpp index 343c2be6..c3ce48fa 100644 --- a/tests/lib/conds/conds-triggers.cpp +++ b/tests/lib/conds/conds-triggers.cpp @@ -4,52 +4,105 @@ * Copyright (C) 2020 Philippe Proulx */ +#include + #include +#include "cpp-common/bt2/graph.hpp" + #include "utils.hpp" namespace { -void triggerGraphMipVersion() noexcept +/* + * Creates a simple condition trigger, calling `func`. + */ +template +CondTrigger *makeSimpleTrigger(FuncT&& func, const CondTrigger::Type type, + const std::string& condId, + const bt2s::optional& nameSuffix = bt2s::nullopt) { - bt_graph_create(292); + return new SimpleCondTrigger {std::forward(func), type, condId, nameSuffix}; } -bt2::IntegerFieldClass::Shared getUIntFc(const bt2::SelfComponent self) noexcept -{ - return self.createTraceClass()->createUnsignedIntegerFieldClass(); -} +using OnCompInitFunc = std::function; -void triggerFcIntSetFieldValueRangeN0(const bt2::SelfComponent self) noexcept +/* + * A "run in" class that delegates the execution to stored callables. + * + * Use the makeRunIn*Trigger() helpers below. + */ +class RunInDelegator final : public RunIn { - getUIntFc(self)->fieldValueRange(0); -} +public: + static RunInDelegator makeOnCompInit(OnCompInitFunc func) + { + return RunInDelegator {std::move(func)}; + } -void triggerFcIntSetFieldValueRangeNGt64(const bt2::SelfComponent self) noexcept + void onCompInit(const bt2::SelfComponent self) override + { + if (_mOnCompInitFunc) { + _mOnCompInitFunc(self); + } + } + +private: + explicit RunInDelegator(OnCompInitFunc onCompInitFunc) : + _mOnCompInitFunc {std::move(onCompInitFunc)} + { + } + + OnCompInitFunc _mOnCompInitFunc; +}; + +/* + * Creates a condition trigger, calling `func` in a component + * initialization context. + */ +CondTrigger *makeRunInCompInitTrigger(OnCompInitFunc func, const CondTrigger::Type type, + const std::string& condId, + const bt2s::optional& nameSuffix = bt2s::nullopt) { - getUIntFc(self)->fieldValueRange(65); + return new RunInCondTrigger {RunInDelegator::makeOnCompInit(std::move(func)), + type, condId, nameSuffix}; } -void triggerFcIntSetFieldValueRangeNull(bt2::SelfComponent) noexcept +bt2::IntegerFieldClass::Shared createUIntFc(const bt2::SelfComponent self) { - bt_field_class_integer_set_field_value_range(NULL, 23); + return self.createTraceClass()->createUnsignedIntegerFieldClass(); } -const cond_trigger triggers[] = { - COND_TRIGGER_PRE_BASIC("pre:graph-create:valid-mip-version", NULL, triggerGraphMipVersion), - COND_TRIGGER_PRE_RUN_IN_COMP_CLS_INIT("pre:field-class-integer-set-field-value-range:valid-n", - "0", triggerFcIntSetFieldValueRangeN0), - COND_TRIGGER_PRE_RUN_IN_COMP_CLS_INIT("pre:field-class-integer-set-field-value-range:valid-n", - "gt-64", triggerFcIntSetFieldValueRangeNGt64), - COND_TRIGGER_PRE_RUN_IN_COMP_CLS_INIT( - "pre:field-class-integer-set-field-value-range:not-null:field-class", NULL, - triggerFcIntSetFieldValueRangeNull), +/* Our condition triggers */ +CondTrigger * const triggers[] = { + makeSimpleTrigger( + [] { + bt2::Graph::create(292); + }, + CondTrigger::Type::PRE, "graph-create:valid-mip-version"), + + makeRunInCompInitTrigger( + [](const bt2::SelfComponent self) { + createUIntFc(self)->fieldValueRange(0); + }, + CondTrigger::Type::PRE, "field-class-integer-set-field-value-range:valid-n", "0"), + + makeRunInCompInitTrigger( + [](const bt2::SelfComponent self) { + createUIntFc(self)->fieldValueRange(65); + }, + CondTrigger::Type::PRE, "field-class-integer-set-field-value-range:valid-n", "gt-64"), + + makeSimpleTrigger( + [] { + bt_field_class_integer_set_field_value_range(nullptr, 23); + }, + CondTrigger::Type::PRE, "field-class-integer-set-field-value-range:not-null:field-class"), }; } /* namespace */ -int main(int argc, const char *argv[]) +int main(const int argc, const char ** const argv) { condMain(argc, argv, triggers); - return 0; } diff --git a/tests/lib/conds/utils.cpp b/tests/lib/conds/utils.cpp index 4df4340c..28f0ba40 100644 --- a/tests/lib/conds/utils.cpp +++ b/tests/lib/conds/utils.cpp @@ -13,65 +13,55 @@ #include "cpp-common/vendor/fmt/core.h" #include "cpp-common/vendor/nlohmann/json.hpp" -#include "../utils/run-in.hpp" #include "utils.hpp" -namespace { - -void runTrigger(const cond_trigger& trigger) noexcept +CondTrigger::CondTrigger(const Type type, const std::string& condId, + const bt2s::optional& nameSuffix) noexcept : + _mType {type}, + _mCondId {fmt::format("{}:{}", type == Type::PRE ? "pre" : "post", condId)}, + _mName { + fmt::format("{}{}{}", condId, nameSuffix ? "-" : "", nameSuffix ? nameSuffix->data() : "")} { - switch (trigger.func_type) { - case COND_TRIGGER_FUNC_TYPE_BASIC: - trigger.func.basic(); - break; - case COND_TRIGGER_FUNC_TYPE_RUN_IN_COMP_CLS_INIT: - runInCompClsInit(trigger.func.run_in_comp_cls_init); - break; - default: - abort(); - } } -void listTriggers(const bt2s::span triggers) noexcept +SimpleCondTrigger::SimpleCondTrigger(std::function func, const Type type, + const std::string& condId, + const bt2s::optional& nameSuffix) : + CondTrigger {type, condId, nameSuffix}, + _mFunc {std::move(func)} { - auto triggerArray = nlohmann::json::array(); - - for (auto& trigger : triggers) { - auto triggerObj = nlohmann::json::object(); - - /* Condition ID */ - triggerObj["cond-id"] = trigger.cond_id; +} - /* Name starts with condition ID */ - std::string name = trigger.cond_id; +namespace { - if (trigger.suffix) { - name += '-'; - name += trigger.suffix; - } +void listCondTriggers(const CondTriggers condTriggers) noexcept +{ + auto condTriggerArray = nlohmann::json::array(); - triggerObj["name"] = std::move(name); - triggerArray.push_back(std::move(triggerObj)); + for (const auto condTrigger : condTriggers) { + condTriggerArray.push_back(nlohmann::json { + {"cond-id", condTrigger->condId()}, + {"name", condTrigger->name()}, + }); } - fmt::println("{}", triggerArray.dump()); + fmt::println("{}", condTriggerArray.dump()); } } /* namespace */ -void condMain(const int argc, const char ** const argv, - const bt2s::span triggers) noexcept +void condMain(const int argc, const char ** const argv, const CondTriggers condTriggers) noexcept { BT_ASSERT(argc >= 2); if (strcmp(argv[1], "list") == 0) { - listTriggers(triggers); + listCondTriggers(condTriggers); } else if (strcmp(argv[1], "run") == 0) { - int index; - BT_ASSERT(argc >= 3); - index = atoi(argv[2]); - BT_ASSERT(index >= 0 && index < triggers.size()); - runTrigger(triggers[index]); + + const auto index = atoi(argv[2]); + + BT_ASSERT(index >= 0 && index < condTriggers.size()); + (*condTriggers[index])(); } } diff --git a/tests/lib/conds/utils.hpp b/tests/lib/conds/utils.hpp index 53676d8c..0fd4fa0a 100644 --- a/tests/lib/conds/utils.hpp +++ b/tests/lib/conds/utils.hpp @@ -7,71 +7,173 @@ #ifndef TESTS_LIB_CONDS_UTILS_H #define TESTS_LIB_CONDS_UTILS_H +#include +#include +#include + #include -#include "cpp-common/bt2/self-component-port.hpp" +#include "cpp-common/bt2s/optional.hpp" #include "cpp-common/bt2s/span.hpp" -enum cond_trigger_func_type -{ - COND_TRIGGER_FUNC_TYPE_BASIC, - COND_TRIGGER_FUNC_TYPE_RUN_IN_COMP_CLS_INIT, -}; +#include "../utils/run-in.hpp" -enum cond_trigger_type +/* + * Abstract condition trigger class. + * + * A derived class must provide operator()() which triggers a condition + * of which the specific type (precondition or postcondition) and ID are + * provided at construction time. + */ +class CondTrigger { - COND_TRIGGER_TYPE_PRE, - COND_TRIGGER_TYPE_POST, -}; +public: + /* + * Condition type. + */ + enum class Type + { + PRE, + POST, + }; -typedef void (*cond_trigger_basic_func)(void); -typedef void (*cond_trigger_run_in_comp_cls_init_func)(bt2::SelfComponent); +protected: + /* + * Builds a condition trigger having the type `type`, the condition + * ID `condId` (_without_ any `pre:` or `post:` prefix), and the + * optional name suffix `nameSuffix`. + * + * The concatenation of `condId` and, if it's set, `-` and + * `*nameSuffix`, forms the name of the condition trigger. Get the + * name of the created condition trigger with name(). + */ + explicit CondTrigger(Type type, const std::string& condId, + const bt2s::optional& nameSuffix) noexcept; -struct cond_trigger -{ - enum cond_trigger_type type; - enum cond_trigger_func_type func_type; - const char *cond_id; - const char *suffix; - union +public: + virtual ~CondTrigger() = default; + virtual void operator()() noexcept = 0; + + Type type() const noexcept + { + return _mType; + } + + /* + * Condition ID, including any `pre:` or `post:` prefix. + */ + const std::string& condId() const noexcept { - cond_trigger_basic_func basic; - cond_trigger_run_in_comp_cls_init_func run_in_comp_cls_init; - } func; + return _mCondId; + } + + const std::string& name() const noexcept + { + return _mName; + } + +private: + Type _mType; + std::string _mCondId; + std::string _mName; }; -#define COND_TRIGGER_PRE_BASIC(_cond_id, _suffix, _func) \ - { \ - .type = COND_TRIGGER_TYPE_PRE, .func_type = COND_TRIGGER_FUNC_TYPE_BASIC, \ - .cond_id = _cond_id, .suffix = _suffix, .func = { \ - .basic = _func, \ - } \ +/* + * Simple condition trigger. + * + * Implements a condition trigger where a function provided at + * construction time triggers a condition. + */ +class SimpleCondTrigger : public CondTrigger +{ +public: + explicit SimpleCondTrigger(std::function func, Type type, const std::string& condId, + const bt2s::optional& nameSuffix = bt2s::nullopt); + + void operator()() noexcept override + { + _mFunc(); } -#define COND_TRIGGER_POST_BASIC(_cond_id, _suffix, _func) \ - { \ - .type = COND_TRIGGER_TYPE_POST, .func_type = COND_TRIGGER_FUNC_TYPE_BASIC, \ - .cond_id = _cond_id, .suffix = _suffix, .func = { \ - .basic = _func, \ - } \ +private: + std::function _mFunc; +}; + +/* + * Run-in condition trigger. + * + * Implements a condition trigger of which the triggering function + * happens in a graph or component class query context using the + * runIn() API. + */ +template +class RunInCondTrigger : public CondTrigger +{ +public: + explicit RunInCondTrigger(RunInT runIn, const Type type, const std::string& condId, + const bt2s::optional& nameSuffix = bt2s::nullopt) : + CondTrigger {type, condId, nameSuffix}, + _mRunIn {std::move(runIn)} + { } -#define COND_TRIGGER_PRE_RUN_IN_COMP_CLS_INIT(_cond_id, _suffix, _func) \ - { \ - .type = COND_TRIGGER_TYPE_PRE, .func_type = COND_TRIGGER_FUNC_TYPE_RUN_IN_COMP_CLS_INIT, \ - .cond_id = _cond_id, .suffix = _suffix, .func = { \ - .run_in_comp_cls_init = _func, \ - } \ + explicit RunInCondTrigger(const Type type, const std::string& condId, + const bt2s::optional& nameSuffix = bt2s::nullopt) : + RunInCondTrigger {RunInT {}, type, condId, nameSuffix} + { } -#define COND_TRIGGER_POST_RUN_IN_COMP_CLS_INIT(_cond_id, _suffix, _func) \ - { \ - .type = COND_TRIGGER_TYPE_POST, .func_type = COND_TRIGGER_FUNC_TYPE_RUN_IN_COMP_CLS_INIT, \ - .cond_id = _cond_id, .suffix = _suffix, .func = { \ - .run_in_comp_cls_init = _func, \ - } \ + void operator()() noexcept override + { + runIn(_mRunIn); } -void condMain(int argc, const char **argv, bt2s::span triggers) noexcept; +private: + RunInT _mRunIn; +}; + +/* + * List of condition triggers. + */ +using CondTriggers = bt2s::span; + +/* + * The entry point of a condition trigger program. + * + * Call this from your own main() with your list of condition triggers + * `triggers`. + * + * Each condition trigger of `triggers` must have a unique name, as + * returned by CondTrigger::name(). + * + * This function uses `argc` and `argv` to respond to one of the + * following commands: + * + * `list`: + * Prints a list of condition triggers as a JSON array of objects. + * + * Each JSON object has: + * + * `cond-id`: + * The condition ID of the trigger, as returned by + * CondTrigger:condId(). + * + * `name`: + * The condition ID name, as returned by CondTrigger::name(). + * + * `run`: + * Runs the triggering function of the condition trigger at the + * index specified by the next command-line argument. + * + * For example, + * + * $ my-cond-trigger-program run 45 + * + * would run the function of the condition trigger `triggers[45]`. + * + * The program is expected to abort through a libbabeltrace2 + * condition failure. + */ +void condMain(int argc, const char **argv, CondTriggers triggers) noexcept; #endif /* TESTS_LIB_CONDS_UTILS_H */ diff --git a/tests/lib/test-fields-bin.cpp b/tests/lib/test-fields-bin.cpp index 79f50f31..3d2d9ade 100644 --- a/tests/lib/test-fields-bin.cpp +++ b/tests/lib/test-fields-bin.cpp @@ -14,9 +14,11 @@ namespace { constexpr int NR_TESTS = 2; -void testStringClear() noexcept +class TestStringClear final : public RunIn { - runInMsgIterClsInit([](const bt2::SelfMessageIterator self) { +public: + void onMsgIterInit(const bt2::SelfMessageIterator self) override + { /* Boilerplate to get a string field */ const auto traceCls = self.component().createTraceClass(); const auto streamCls = traceCls->createStreamClass(); @@ -39,8 +41,8 @@ void testStringClear() noexcept field.clear(); ok(field.value() == "", "string field is empty"); ok(field.length() == 0, "string field length is 0"); - }); -} + } +}; } /* namespace */ @@ -48,7 +50,8 @@ int main() { plan_tests(NR_TESTS); - testStringClear(); + TestStringClear testStringClear; + runIn(testStringClear); return exit_status(); } diff --git a/tests/lib/utils/run-in.cpp b/tests/lib/utils/run-in.cpp index a3391a93..91b23396 100644 --- a/tests/lib/utils/run-in.cpp +++ b/tests/lib/utils/run-in.cpp @@ -4,8 +4,6 @@ * Copyright (C) 2020-2023 EfficiOS, inc. */ -#include - #include "common/assert.h" #include "cpp-common/bt2/component-class-dev.hpp" #include "cpp-common/bt2/component-class.hpp" @@ -17,14 +15,19 @@ #include "run-in.hpp" -namespace { +void RunIn::onQuery(bt2::SelfComponentClass) +{ +} -struct RunInData final +void RunIn::onCompInit(bt2::SelfComponent) { - RunInCompClsQueryFunc compClsCtxFunc; - RunInCompClsInitFunc compCtxFunc; - RunInMsgIterClsInitFunc msgIterCtxFunc; -}; +} + +void RunIn::onMsgIterInit(bt2::SelfMessageIterator) +{ +} + +namespace { class RunInSource; @@ -36,11 +39,7 @@ public: const bt2::SelfComponentOutputPort port) : bt2::UserMessageIterator {self, "RUN-IN-SRC-MSG-ITER"} { - const auto& data = port.data(); - - if (data.msgIterCtxFunc) { - data.msgIterCtxFunc(self); - } + port.data().onMsgIterInit(self); } void _next(bt2::ConstMessageArray&) @@ -49,56 +48,46 @@ public: }; class RunInSource final : - public bt2::UserSourceComponent + public bt2::UserSourceComponent { public: static constexpr auto name = "run-in-src"; explicit RunInSource(const bt2::SelfSourceComponent self, bt2::ConstMapValue, - const RunInData * const runInData) : - bt2::UserSourceComponent {self, "RUN-IN-SRC"}, - _mRunInData {runInData} + RunIn * const runIn) : + bt2::UserSourceComponent {self, + "RUN-IN-SRC"}, + _mRunIn {runIn} { - this->_addOutputPort("out", *runInData); - - if (_mRunInData->compCtxFunc) { - _mRunInData->compCtxFunc(self); - } + this->_addOutputPort("out", *runIn); + _mRunIn->onCompInit(self); } static bt2::Value::Shared _query(const bt2::SelfComponentClass self, bt2::PrivateQueryExecutor, - bt2c::CStringView, bt2::ConstValue, - const RunInData * const data) + bt2c::CStringView, bt2::ConstValue, RunIn *data) { - if (data->compClsCtxFunc) { - data->compClsCtxFunc(self); - } - + data->onQuery(self); return bt2::NullValue {}.shared(); } private: - const RunInData *_mRunInData; + RunIn *_mRunIn; }; } /* namespace */ -void runIn(RunInCompClsQueryFunc compClsCtxFunc, RunInCompClsInitFunc compCtxFunc, - RunInMsgIterClsInitFunc msgIterCtxFunc) +void runIn(RunIn& runIn) { - RunInData data {std::move(compClsCtxFunc), std::move(compCtxFunc), std::move(msgIterCtxFunc)}; const auto srcCompCls = bt2::SourceComponentClass::create(); - /* Execute a query (executes `compClsCtxFunc`) */ - bt2::QueryExecutor::create(*srcCompCls, "object-name", data)->query(); + /* Execute a query */ + bt2::QueryExecutor::create(*srcCompCls, "object-name", runIn)->query(); /* Create graph */ const auto graph = bt2::Graph::create(0); /* Add custom source component (executes `compCtxFunc`) */ - const auto srcComp = graph->addComponent(*srcCompCls, "the-source", data); + const auto srcComp = graph->addComponent(*srcCompCls, "the-source", runIn); /* Add dummy sink component */ const auto sinkComp = bt2c::call([&] { @@ -125,18 +114,3 @@ void runIn(RunInCompClsQueryFunc compClsCtxFunc, RunInCompClsInitFunc compCtxFun /* Run graph (executes `msgIterCtxFunc`) */ graph->run(); } - -void runInCompClsQuery(RunInCompClsQueryFunc func) -{ - runIn(std::move(func), nullptr, nullptr); -} - -void runInCompClsInit(RunInCompClsInitFunc func) -{ - runIn(nullptr, std::move(func), nullptr); -} - -void runInMsgIterClsInit(RunInMsgIterClsInitFunc func) -{ - runIn(nullptr, nullptr, std::move(func)); -} diff --git a/tests/lib/utils/run-in.hpp b/tests/lib/utils/run-in.hpp index 01774f90..d08e931a 100644 --- a/tests/lib/utils/run-in.hpp +++ b/tests/lib/utils/run-in.hpp @@ -7,46 +7,43 @@ #ifndef TESTS_LIB_UTILS_H #define TESTS_LIB_UTILS_H -#include - #include #include "cpp-common/bt2/self-component-class.hpp" #include "cpp-common/bt2/self-component-port.hpp" #include "cpp-common/bt2/self-message-iterator.hpp" -using RunInCompClsQueryFunc = std::function; -using RunInCompClsInitFunc = std::function; -using RunInMsgIterClsInitFunc = std::function; - /* - * Runs: - * - * • `compClsCtxFunc` in the context of a component class method, - * if not `nullptr`. + * Base class from which to inherit to call runIn(). * - * • `compCtxFunc` in the context of a component method, if not - * `nullptr`. - * - * • `msgIterCtxFunc` in the context of a message iterator method, if - * not `nullptr`. - */ -void runIn(RunInCompClsQueryFunc compClsCtxFunc, RunInCompClsInitFunc compCtxFunc, - RunInMsgIterClsInitFunc msgIterCtxFunc); - -/* - * Runs `func` in the context of a component class method. - */ -void runInCompClsQuery(RunInCompClsQueryFunc func); - -/* - * Runs `func` in the context of a component method. + * Override any of the on*() methods to get your statements executed in + * a specific context. */ -void runInCompClsInit(RunInCompClsInitFunc func); +class RunIn +{ +public: + virtual ~RunIn() = default; + + /* + * Called when querying the component class `self`. + */ + virtual void onQuery(bt2::SelfComponentClass self); + + /* + * Called when initializing the component `self`. + */ + virtual void onCompInit(bt2::SelfComponent self); + + /* + * Called when initializing the message iterator `self`. + */ + virtual void onMsgIterInit(bt2::SelfMessageIterator self); +}; /* - * Runs `func` in the context of a message iterator method. + * Runs a simple graph (one source and one sink component), calling the + * `on*()` methods of `runIn` along the way. */ -void runInMsgIterClsInit(RunInMsgIterClsInitFunc func); +void runIn(RunIn& runIn); #endif /* TESTS_LIB_UTILS_H */ -- 2.34.1