[PATCH i-g-t 1/3] igt_hook: Add feature
Lucas De Marchi
lucas.demarchi at intel.com
Tue May 21 19:40:56 UTC 2024
On Thu, May 09, 2024 at 12:24:29PM GMT, Gustavo Sousa wrote:
>For development purposes, sometimes it is useful to have a way of
>running custom scripts at certain points of test executions. A
>real-world example I bumped into recently is to collect information from
>sysfs before and after running each entry of a testlist.
>
>While it is possible for the user to handcraft a script that calls each
>test with the correct actions before and after execution, we can provide
>a better experience by adding built-in support for running hooks during
>test execution.
>
>That would be even better when adding the same kind of support for
>igt_runner (which is done in an upcoming change), since the user can
>also nicely resume with igt_resume with the hook already setup in case a
>crash happens during execution of the test list.
>
>As such provide implement support for hooks, integrate it into
>igt_core and expose the functionality via --hook CLI option on test
>executables.
>
>Signed-off-by: Gustavo Sousa <gustavo.sousa at intel.com>
>---
> .../igt-gpu-tools/igt-gpu-tools-docs.xml | 1 +
> lib/igt_core.c | 116 +++-
> lib/igt_hook.c | 499 ++++++++++++++++++
> lib/igt_hook.h | 86 +++
> lib/meson.build | 1 +
> lib/tests/igt_hook.c | 187 +++++++
> lib/tests/igt_hook_integration.c | 297 +++++++++++
> lib/tests/meson.build | 2 +
> 8 files changed, 1180 insertions(+), 9 deletions(-)
> create mode 100644 lib/igt_hook.c
> create mode 100644 lib/igt_hook.h
> create mode 100644 lib/tests/igt_hook.c
> create mode 100644 lib/tests/igt_hook_integration.c
>
>diff --git a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
>index 9085eb924e85..11458c68124b 100644
>--- a/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
>+++ b/docs/reference/igt-gpu-tools/igt-gpu-tools-docs.xml
>@@ -32,6 +32,7 @@
> <xi:include href="xml/igt_fb.xml"/>
> <xi:include href="xml/igt_frame.xml"/>
> <xi:include href="xml/igt_gt.xml"/>
>+ <xi:include href="xml/igt_hook.xml"/>
> <xi:include href="xml/igt_io.xml"/>
> <xi:include href="xml/igt_kmod.xml"/>
> <xi:include href="xml/igt_kms.xml"/>
>diff --git a/lib/igt_core.c b/lib/igt_core.c
>index 3ff3e0392316..291d891cf884 100644
>--- a/lib/igt_core.c
>+++ b/lib/igt_core.c
>@@ -70,6 +70,7 @@
>
> #include "igt_core.h"
> #include "igt_aux.h"
>+#include "igt_hook.h"
> #include "igt_sysfs.h"
> #include "igt_sysrq.h"
> #include "igt_rc.h"
>@@ -241,6 +242,9 @@
> * - '*,!basic*' match any subtest not starting basic
> * - 'basic*,!basic-render*' match any subtest starting basic but not starting basic-render
> *
>+ * It is possible to run a shell script at certain points of test execution with
>+ * "--hook". See the usage description with "--help-hook" for details.
>+ *
> * # Configuration
> *
> * Some of IGT's behavior can be configured through a configuration file.
>@@ -273,6 +277,8 @@ static unsigned int exit_handler_count;
> const char *igt_interactive_debug;
> bool igt_skip_crc_compare;
>
>+static struct igt_hook *igt_hook = NULL;
>+
> /* subtests helpers */
> static bool show_testlist = false;
> static bool list_subtests = false;
>@@ -338,6 +344,8 @@ enum {
> OPT_INTERACTIVE_DEBUG,
> OPT_SKIP_CRC,
> OPT_TRACE_OOPS,
>+ OPT_HOOK,
>+ OPT_HELP_HOOK,
> OPT_DEVICE,
> OPT_VERSION,
> OPT_HELP = 'h'
>@@ -810,6 +818,8 @@ static void common_exit_handler(int sig)
> bind_fbcon(true);
> }
>
>+ igt_hook_free(igt_hook);
>+
> /* When not killed by a signal check that igt_exit() has been properly
> * called. */
> assert(sig != 0 || igt_exit_called || igt_is_aborting);
>@@ -907,6 +917,8 @@ static void print_usage(const char *help_str, bool output_on_stderr)
> " --interactive-debug[=domain]\n"
> " --skip-crc-compare\n"
> " --trace-on-oops\n"
>+ " --hook [<events>:]<cmd>\n"
>+ " --help-hook\n"
> " --help-description\n"
> " --describe\n"
> " --device filters\n"
>@@ -1090,6 +1102,8 @@ static int common_init(int *argc, char **argv,
> {"interactive-debug", optional_argument, NULL, OPT_INTERACTIVE_DEBUG},
> {"skip-crc-compare", no_argument, NULL, OPT_SKIP_CRC},
> {"trace-on-oops", no_argument, NULL, OPT_TRACE_OOPS},
>+ {"hook", required_argument, NULL, OPT_HOOK},
>+ {"help-hook", no_argument, NULL, OPT_HELP_HOOK},
> {"device", required_argument, NULL, OPT_DEVICE},
> {"version", no_argument, NULL, OPT_VERSION},
> {"help", no_argument, NULL, OPT_HELP},
>@@ -1225,6 +1239,24 @@ static int common_init(int *argc, char **argv,
> case OPT_TRACE_OOPS:
> show_ftrace = true;
> break;
>+ case OPT_HOOK:
>+ assert(optarg);
>+ if (igt_hook) {
>+ igt_warn("Overriding previous hook descriptor\n");
>+ igt_hook_free(igt_hook);
>+ }
>+ igt_hook = igt_hook_init(optarg, &ret);
this is a pair with igt_hook_free(). I think it's more common to
call it either _new() or _create() rather than _init().
>+ if (!igt_hook) {
>+ igt_critical("Failed to initialize hook data: %s\n",
>+ igt_hook_error_str(ret));
>+ ret = ret > 0 ? -2 : -3;
>+ goto out;
>+ }
>+ break;
>+ case OPT_HELP_HOOK:
>+ igt_hook_print_help(stdout, "--hook");
>+ ret = -1;
>+ goto out;
> case OPT_DEVICE:
> assert(optarg);
> /* if set by env IGT_DEVICE we need to free it */
>@@ -1274,9 +1306,24 @@ out:
> exit(IGT_EXIT_INVALID);
> }
>
>- if (ret < 0)
>- /* exit with no error for -h/--help */
>- exit(ret == -1 ? 0 : IGT_EXIT_INVALID);
>+ if (ret < 0) {
>+ free(igt_hook);
>+ igt_hook = NULL;
>+
>+ switch (ret) {
>+ case -1: /* exit with no error for -h/--help */
>+ exit(0);
>+ break;
>+ case -2:
>+ exit(IGT_EXIT_INVALID);
>+ break;
>+ case -3:
>+ exit(IGT_EXIT_ABORT);
>+ break;
>+ default:
>+ assert(0);
>+ }
>+ }
>
> if (!igt_only_list_subtests()) {
> bind_fbcon(false);
>@@ -1284,6 +1331,15 @@ out:
> print_version();
> igt_srandom();
>
>+ if (igt_hook) {
>+ struct igt_hook_evt hook_evt = {
>+ .evt_type = IGT_HOOK_PRE_TEST,
>+ .target_name = command_str,
>+ };
>+
>+ igt_hook_push_evt(igt_hook, &hook_evt);
>+ }
>+
> sync();
> oom_adjust_for_doom();
> ftrace_dump_on_oops(show_ftrace);
>@@ -1487,6 +1543,16 @@ bool __igt_run_subtest(const char *subtest_name, const char *file, const int lin
> igt_thread_clear_fail_state();
>
> igt_gettime(&subtest_time);
>+
>+ if (igt_hook) {
>+ struct igt_hook_evt hook_evt = {
>+ .evt_type = IGT_HOOK_PRE_SUBTEST,
>+ .target_name = subtest_name,
>+ };
>+
>+ igt_hook_push_evt(igt_hook, &hook_evt);
>+ }
>+
> return (in_subtest = subtest_name);
> }
>
>@@ -1517,6 +1583,16 @@ bool __igt_run_dynamic_subtest(const char *dynamic_subtest_name)
> _igt_dynamic_tests_executed++;
>
> igt_gettime(&dynamic_subtest_time);
>+
>+ if (igt_hook) {
>+ struct igt_hook_evt hook_evt = {
>+ .evt_type = IGT_HOOK_PRE_DYN_SUBTEST,
>+ .target_name = dynamic_subtest_name,
>+ };
>+
>+ igt_hook_push_evt(igt_hook, &hook_evt);
>+ }
>+
> return (in_dynamic_subtest = dynamic_subtest_name);
> }
>
>@@ -1602,6 +1678,17 @@ __noreturn static void exit_subtest(const char *result)
> struct timespec *thentime = in_dynamic_subtest ? &dynamic_subtest_time : &subtest_time;
> jmp_buf *jmptarget = in_dynamic_subtest ? &igt_dynamic_jmpbuf : &igt_subtest_jmpbuf;
>
>+ if (igt_hook) {
>+ struct igt_hook_evt hook_evt = {
>+ .evt_type = (in_dynamic_subtest
>+ ? IGT_HOOK_POST_DYN_SUBTEST
>+ : IGT_HOOK_POST_SUBTEST),
>+ .result = result,
>+ };
>+
>+ igt_hook_push_evt(igt_hook, &hook_evt);
>+ }
>+
> if (!igt_thread_is_main()) {
> igt_thread_fail();
> pthread_exit(NULL);
>@@ -2274,6 +2361,7 @@ void __igt_abort(const char *domain, const char *file, const int line,
> void igt_exit(void)
> {
> int tmp;
>+ const char *result;
>
> if (!test_with_subtests)
> igt_thread_assert_no_failures();
>@@ -2318,12 +2406,7 @@ void igt_exit(void)
>
> assert(waitpid(-1, &tmp, WNOHANG) == -1 && errno == ECHILD);
>
>- if (!test_with_subtests) {
>- struct timespec now;
>- const char *result;
>-
>- igt_gettime(&now);
>-
>+ if (!test_with_subtests || igt_hook) {
> switch (igt_exitcode) {
> case IGT_EXIT_SUCCESS:
> result = "SUCCESS";
>@@ -2334,6 +2417,12 @@ void igt_exit(void)
> default:
> result = "FAIL";
> }
>+ }
>+
>+ if (!test_with_subtests) {
>+ struct timespec now;
>+
>+ igt_gettime(&now);
>
> if (test_multi_fork_child) /* parent will do the yelling */
> _log_line_fprintf(stdout, "dyn_child pid:%d (%.3fs) ends with err=%d\n",
>@@ -2344,6 +2433,15 @@ void igt_exit(void)
> result, igt_time_elapsed(&subtest_time, &now));
> }
>
>+ if (igt_hook) {
>+ struct igt_hook_evt hook_evt = {
>+ .evt_type = IGT_HOOK_POST_TEST,
>+ .result = result,
>+ };
>+
>+ igt_hook_push_evt(igt_hook, &hook_evt);
>+ }
>+
> exit(igt_exitcode);
> }
>
>diff --git a/lib/igt_hook.c b/lib/igt_hook.c
>new file mode 100644
>index 000000000000..8a39e19e3e5f
>--- /dev/null
>+++ b/lib/igt_hook.c
>@@ -0,0 +1,499 @@
>+/*
>+ * Copyright © 2024 Intel Corporation
>+ *
>+ * Permission is hereby granted, free of charge, to any person obtaining a
>+ * copy of this software and associated documentation files (the "Software"),
>+ * to deal in the Software without restriction, including without limitation
>+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>+ * and/or sell copies of the Software, and to permit persons to whom the
>+ * Software is furnished to do so, subject to the following conditions:
>+ *
>+ * The above copyright notice and this permission notice (including the next
>+ * paragraph) shall be included in all copies or substantial portions of the
>+ * Software.
>+ *
>+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>+ * IN THE SOFTWARE.
I think this should follow kernel and some of our igt and just use SPDX.
>+ */
>+#include <assert.h>
>+#include <errno.h>
>+#include <limits.h>
>+#include <stdbool.h>
>+#include <stddef.h>
>+#include <stdint.h>
>+#include <stdio.h>
>+#include <stdlib.h>
>+#include <string.h>
>+
>+#include "igt_hook.h"
>+
>+/**
>+ * SECTION:igt_hook
>+ * @short_description: Support for running a hook script on test execution
>+ * @title: Hook support
>+ *
>+ * IGT provides support for running a hook script when executing tests. This
>+ * support is provided to users via CLI option `--hook` available in test
>+ * binaries. Users should use `--help-hook` for detailed usaged description of
>+ * the feature.
>+ *
>+ * The sole user of the exposed API is `igt_core`, which calls @igt_hook_init()
>+ * when initializing a test case, then calls @igt_hook_push_evt() for each event
>+ * that occurs during that test's execution and finally calls @igt_hook_free()
>+ * to clean up at the end.
>+ */
>+
>+#define TEST_NAME_INITIAL_SIZE 16
>+
>+typedef uint16_t evt_mask_t;
>+
>+struct igt_hook {
>+ evt_mask_t evt_mask;
>+ char *cmd;
>+ char *test_name;
>+ size_t test_name_size;
>+ char *subtest_name;
>+ size_t subtest_name_size;
>+ char *dyn_subtest_name;
>+ size_t dyn_subtest_name_size;
>+ char *test_fullname;
>+};
>+
>+enum igt_hook_error {
>+ IGT_HOOK_EVT_EMPTY_NAME = 1,
>+ IGT_HOOK_EVT_NO_MATCH,
>+};
>+
>+static_assert(IGT_HOOK_NUM_EVENTS <= sizeof(evt_mask_t) * CHAR_BIT,
>+ "Number of event types does not fit event type mask");
>+
>+static const char *igt_hook_evt_type_to_name(enum igt_hook_evt_type evt_type)
>+{
>+ switch (evt_type) {
>+ case IGT_HOOK_PRE_TEST:
>+ return "pre-test";
>+ case IGT_HOOK_PRE_SUBTEST:
>+ return "pre-subtest";
>+ case IGT_HOOK_PRE_DYN_SUBTEST:
>+ return "pre-dyn-subtest";
>+ case IGT_HOOK_POST_DYN_SUBTEST:
>+ return "post-dyn-subtest";
>+ case IGT_HOOK_POST_SUBTEST:
>+ return "post-subtest";
>+ case IGT_HOOK_POST_TEST:
>+ return "post-test";
>+ case IGT_HOOK_NUM_EVENTS:
>+ break;
>+ /* No "default:" case, to force a warning from -Wswitch in case we miss
>+ * any new event type. */
yeah, I like -Wswitch for this purpose, but I don't think we need a
comment. Instead return something here.
>+ }
>+ return "?";
and return nothing here
>+}
>+
>+static int igt_hook_parse_hook_str(const char *hook_str, evt_mask_t *evt_mask, const char **cmd)
>+{
>+ const char *s;
>+
>+ if (!strchr(hook_str, ':')) {
>+ *evt_mask = ~0;
>+ *cmd = hook_str;
>+ return 0;
>+ }
>+
>+ s = hook_str;
>+ *evt_mask = 0;
>+
>+ while (1) {
>+ const char *evt_name;
>+ bool has_match;
>+ bool is_star;
>+ enum igt_hook_evt_type evt_type;
>+
>+ evt_name = s;
>+
>+ while (*s != ':' && *s != ',')
>+ s++;
>+
>+ if (evt_name == s)
>+ return IGT_HOOK_EVT_EMPTY_NAME;
>+
>+ has_match = false;
>+ is_star = *evt_name == '*' && evt_name + 1 == s;
>+
>+ for (evt_type = IGT_HOOK_PRE_TEST; evt_type < IGT_HOOK_NUM_EVENTS; evt_type++) {
>+ if (!is_star) {
>+ const char *this_event_name = igt_hook_evt_type_to_name(evt_type);
>+ size_t len = s - evt_name;
>+
>+ if (len != strlen(this_event_name))
>+ continue;
>+
>+ if (strncmp(evt_name, this_event_name, len))
>+ continue;
>+ }
>+
>+ *evt_mask |= 1 << evt_type;
pre-test,pre-test,pre-test:echo foo
^ shouldn't this cause a warning?
but see below for alternative interface
>+ has_match = true;
>+
>+ if (!is_star)
>+ break;
>+ }
>+
>+ if (!has_match)
>+ return IGT_HOOK_EVT_NO_MATCH;
>+
>+ if (*s++ == ':')
>+ break;
>+ }
>+
>+ *cmd = s;
>+
>+ return 0;
>+}
>+
>+static size_t igt_hook_calc_test_fullname_size(struct igt_hook *igt_hook) {
>+ /* The maximum size of test_fullname will be the maximum length of
>+ * "igt@<test_name>@<subtest_name>@<dyn_subtest_name>" plus 1 for the
>+ * null byte. */
>+ return (igt_hook->test_name_size +
>+ igt_hook->subtest_name_size +
>+ igt_hook->dyn_subtest_name_size) + 4;
>+}
>+
>+static void igt_hook_update_test_fullname(struct igt_hook *igt_hook)
>+{
>+ int i;
>+ char *s;
>+ const char *values[3] = {
>+ igt_hook->test_name,
>+ igt_hook->subtest_name,
>+ igt_hook->dyn_subtest_name,
>+ };
>+
>+ if (igt_hook->test_name[0] == '\0') {
>+ igt_hook->test_fullname[0] = '\0';
>+ return;
>+ }
>+
>+ s = stpcpy(igt_hook->test_fullname, "igt");
>+ for (i = 0; i < 3 && values[i][0] != '\0'; i++) {
^ ARRAY_SIZE()
>+ *s++ = '@';
>+ s = stpcpy(s, values[i]);
>+ }
>+}
>+
>+/**
>+ * igt_hook_init:
>+ * @hook_str: Hook descriptor string.
>+ * @error: Pointer to error number.
>+ *
>+ * Allocate and initialize an #igt_hook structure.
>+ *
>+ * This function parses the hook descriptor @hook_str and initializes the struct
>+ * to be returned.
>+ *
>+ * The hook descriptor comes from the argument to `--hook` of the test
>+ * executable being run.
>+ *
>+ * If not #NULL, @error is used to store a non-zero error number if an error
>+ * happens. A human-readable string for that error number can be obtained with
>+ * @igt_hook_error_str().
>+ *
>+ * Returns: The pointer to the #igt_hook structure on success or #NULL on error.
>+ */
>+struct igt_hook *igt_hook_init(const char *hook_str, int *error)
>+{
>+ int err;
>+ evt_mask_t evt_mask;
>+ const char *cmd;
>+ struct igt_hook *igt_hook = NULL;
>+
>+
>+ err = igt_hook_parse_hook_str(hook_str, &evt_mask, &cmd);
>+ if (err)
>+ goto out;
>+
>+ igt_hook = calloc(1, sizeof(*igt_hook));
>+ igt_hook->evt_mask = evt_mask;
>+
>+ igt_hook->cmd = strdup(cmd);
>+ if (!igt_hook->cmd) {
>+ err = -errno;
>+ goto out;
>+ }
>+
>+ igt_hook->test_name = malloc(TEST_NAME_INITIAL_SIZE);
>+ igt_hook->test_name_size = TEST_NAME_INITIAL_SIZE;
>+ igt_hook->subtest_name = malloc(TEST_NAME_INITIAL_SIZE);
>+ igt_hook->subtest_name_size = TEST_NAME_INITIAL_SIZE;
>+ igt_hook->dyn_subtest_name = malloc(TEST_NAME_INITIAL_SIZE);
>+ igt_hook->dyn_subtest_name_size = TEST_NAME_INITIAL_SIZE;
up to you, but in this kind of things I tend to prefer the strings in
the outer struct so we have fewer separate malloc()'s and thus fewer
chances of leaking them
just calculate the size ahead and then update igt_hook->test_name,
igt_hook->subtest_name and igt_hook->dyn_subtest_name to point to the right part.
>+ igt_hook->test_fullname = malloc(igt_hook_calc_test_fullname_size(igt_hook));
>+
>+ igt_hook->test_name[0] = '\0';
>+ igt_hook->subtest_name[0] = '\0';
>+ igt_hook->dyn_subtest_name[0] = '\0';
>+ igt_hook->test_fullname[0] = '\0';
with the benefit that then these are covered by the calloc.
up to you.
>+
>+out:
>+ if (err) {
>+ if (error)
>+ *error = err;
>+
>+ igt_hook_free(igt_hook);
>+
>+ return NULL;
>+ }
>+
>+ return igt_hook;
>+}
>+
>+/**
>+ * igt_hook_free:
>+ * @igt_hook: The igt_hook struct.
>+ *
>+ * De-initialize an igt_hook struct returned by @igt_hook_init().
>+ *
>+ * This is a no-op if @igt_hook is #NULL.
>+ */
>+void igt_hook_free(struct igt_hook *igt_hook)
>+{
>+ if (!igt_hook)
>+ return;
>+
>+ free(igt_hook->cmd);
>+ free(igt_hook->test_name);
>+ free(igt_hook->subtest_name);
>+ free(igt_hook->dyn_subtest_name);
>+ free(igt_hook);
>+}
>+
>+static void igt_hook_update_test_name_pre_call(struct igt_hook *igt_hook, struct igt_hook_evt *evt)
>+{
>+ char **name_ptr;
>+ size_t *size_ptr;
>+ size_t len;
>+
>+ switch (evt->evt_type) {
>+ case IGT_HOOK_PRE_TEST:
>+ name_ptr = &igt_hook->test_name;
>+ size_ptr = &igt_hook->test_name_size;
>+ break;
>+ case IGT_HOOK_PRE_SUBTEST:
>+ name_ptr = &igt_hook->subtest_name;
>+ size_ptr = &igt_hook->subtest_name_size;
>+ break;
>+ case IGT_HOOK_PRE_DYN_SUBTEST:
>+ name_ptr = &igt_hook->dyn_subtest_name;
>+ size_ptr = &igt_hook->dyn_subtest_name_size;
>+ break;
>+ default:
>+ return;
>+ }
>+
>+ len = strlen(evt->target_name);
>+ if (len + 1 > *size_ptr) {
>+ size_t fullname_size;
>+
>+ *size_ptr *= 2;
>+ *name_ptr = realloc(*name_ptr, *size_ptr);
>+
>+ fullname_size = igt_hook_calc_test_fullname_size(igt_hook);
>+ igt_hook->test_fullname = realloc(igt_hook->test_fullname, fullname_size);
>+ }
>+
>+ strcpy(*name_ptr, evt->target_name);
>+ igt_hook_update_test_fullname(igt_hook);
>+}
>+
>+static void igt_hook_update_test_name_post_call(struct igt_hook *igt_hook, struct igt_hook_evt *evt)
>+{
>+ switch (evt->evt_type) {
>+ case IGT_HOOK_POST_TEST:
>+ igt_hook->test_name[0] = '\0';
>+ break;
>+ case IGT_HOOK_POST_SUBTEST:
>+ igt_hook->subtest_name[0] = '\0';
>+ break;
>+ case IGT_HOOK_POST_DYN_SUBTEST:
>+ igt_hook->dyn_subtest_name[0] = '\0';
>+ break;
>+ default:
>+ return;
>+ }
>+
>+ igt_hook_update_test_fullname(igt_hook);
>+}
>+
>+static void igt_hook_update_env_vars(struct igt_hook *igt_hook, struct igt_hook_evt *evt)
>+{
>+ setenv("IGT_HOOK_EVENT", igt_hook_evt_type_to_name(evt->evt_type), 1);
>+ setenv("IGT_HOOK_TEST_FULLNAME", igt_hook->test_fullname, 1);
>+ setenv("IGT_HOOK_TEST", igt_hook->test_name, 1);
>+ setenv("IGT_HOOK_SUBTEST", igt_hook->subtest_name, 1);
>+ setenv("IGT_HOOK_DYN_SUBTEST", igt_hook->dyn_subtest_name, 1);
>+ setenv("IGT_HOOK_RESULT", evt->result ?: "", 1);
>+}
>+
>+/**
>+ * igt_hook_push_evt:
>+ * @igt_hook: The igt_hook structure.
>+ * @evt: The event to be pushed.
>+ *
>+ * Push a new igt_hook event.
>+ *
>+ * This function must be used to register a new igt_hook event. Calling it will
>+ * cause execution of the hook script if the event type matches the filters
>+ * provided during initialization of @igt_hook.
>+ */
>+void igt_hook_push_evt(struct igt_hook *igt_hook, struct igt_hook_evt *evt)
>+{
>+ typeof(igt_hook->evt_mask) evt_bit = (1 << evt->evt_type);
>+
>+ igt_hook_update_test_name_pre_call(igt_hook, evt);
>+
>+ if ((evt_bit & igt_hook->evt_mask)) {
>+ igt_hook_update_env_vars(igt_hook, evt);
>+ system(igt_hook->cmd);
this polutes the env for the current process just so the child process
has those env vars. We could either duplicate the environ on startup and
use execle(), or to be simpler, just make sure to zap our env just after
running the command. This avoids undesirable behavior because we didn't
clean up the environ.
>+ }
>+
>+ igt_hook_update_test_name_post_call(igt_hook, evt);
>+}
>+
>+/**
>+ * igt_hook_error_str:
>+ * @error: Non-zero error number.
>+ *
>+ * Return a human-readable string containing a description of an error number
>+ * generated by one of the `igt_hook_*` functions.
>+ *
>+ * The string will be the result of strerror() for errors from the C standard
>+ * library or a custom description specific to igt_hook.
>+ */
>+const char *igt_hook_error_str(int error)
>+{
>+ if (!error)
>+ return "No error";
>+
>+ if (error > 0) {
>+ enum igt_hook_error hook_error = error;
>+
>+ switch (hook_error) {
>+ case IGT_HOOK_EVT_EMPTY_NAME:
>+ return "Empty name in event descriptor";
>+ case IGT_HOOK_EVT_NO_MATCH:
>+ return "Event name in event descriptor does not match any event type";
>+ default:
>+ return "Unknown error";
>+ }
>+ } else {
>+ return strerror(-error);
>+ }
>+}
>+
>+/**
>+ * igt_hook_print_help:
>+ * @f: File pointer where to write the output.
>+ * @option_name: Name of the CLI option that accepts the hook descriptor.
>+ *
>+ * Print a detailed user help text on hook usage.
>+ */
>+void igt_hook_print_help(FILE *f, const char *option_name)
>+{
>+ fprintf(f, "\
>+The option %1$s receives as argument a \"hook descriptor\" and allows the\n\
>+execution of a shell command at different points during execution of tests. Each\n\
>+such a point is called a \"hook event\".\n\
>+\n\
>+Examples:\n\
>+\n\
>+ # Prints hook-specic env vars for every event.\n\
>+ %1$s 'printenv | grep ^IGT_HOOK_'\n\
>+\n\
>+ # Equivalent to the above. Useful if command contains ':'.\n\
>+ %1$s '*:printenv | grep ^IGT_HOOK_'\n\
>+\n\
>+ # Adds a line to out.txt containing the result of each test case.\n\
>+ %1$s 'post-test:echo $IGT_HOOK_TEST_FULLNAME $IGT_HOOK_RESULT >> out.txt'\n\
>+\n\
>+The accepted format for a hook descriptor is `[<events>:]<cmd>`, where:\n\
>+\n\
>+ - <events> is a comma-separated list of event descriptors, which defines the\n\
>+ set of events be tracked. If omitted, all events are tracked.\n\
>+\n\
I think the interface git-rebase is more natural for this kind of
thing. You have 2 concepts: event and hook with n:1 relationship
because of the interface chosen.
Why not simply use 1:1 and then allow the program to have multiple
hooks?
Just in git-rebase we do `git rebase -x "echo foo" -x "echo bar"` and
the commands are appended, we could have:
igt_runner -x 'pre-test:echo foo' -x 'post-test:echo bar'. Then you only
warn when you user incorrectly used the same hook type. I think it
would be more natural to eventually embed the support on a test list
(with the exception we'd not warn anymore), with something like:
hook at pre-test@echo foo
hook at post-test@echo bar
igt at ...
igt at ...
igt at ...
hook at pre-test@ ....
igt at ...
>+ - <cmd> is a shell command to be executed on the occurrence each tracked\n\
>+ event. If the command contains ':', then passing <events> is required,\n\
>+ otherwise part of the command would be treated as an event descriptor.\n\
>+\n\
>+", option_name);
>+
>+ fprintf(f, "\
>+An \"event descriptor\" is either the name of an event or the string '*'. The\n\
>+latter matches all event names. The list of possible event names is provided\n\
>+below:\n\
>+\n\
>+");
>+
>+ for (enum igt_hook_evt_type et = 0; et < IGT_HOOK_NUM_EVENTS; et++) {
>+ const char *desc;
>+
>+ switch (et) {
>+ case IGT_HOOK_PRE_TEST:
>+ desc = "Occurs before a test case starts.";
>+ break;
>+ case IGT_HOOK_PRE_SUBTEST:
>+ desc = "Occurs before the execution of a subtest.";
>+ break;
>+ case IGT_HOOK_PRE_DYN_SUBTEST:
>+ desc = "Occurs before the execution of a dynamic subtest.";
>+ break;
>+ case IGT_HOOK_POST_DYN_SUBTEST:
>+ desc = "Occurs after the execution of a dynamic subtest.";
>+ break;
>+ case IGT_HOOK_POST_SUBTEST:
>+ desc = "Occurs after the execution of a subtest.";
>+ break;
>+ case IGT_HOOK_POST_TEST:
>+ desc = "Occurs after a test case has finished.";
>+ break;
>+ default:
>+ desc = "MISSING DESCRIPTION";
>+ }
>+
>+ fprintf(f, " %s\n %s\n\n", igt_hook_evt_type_to_name(et), desc);
>+ }
>+
>+ fprintf(f, "\
>+For each event matched by <events>, <cmd> is executed as a shell command. The\n\
>+exit status of the command is ignored. The following environment variables are\n\
>+available to the command:\n\
>+\n\
>+ IGT_HOOK_EVENT\n\
>+ Name of the current event.\n\
>+\n\
>+ IGT_HOOK_TEST_FULLNAME\n\
>+ Full name of the test in the format `igt@<test>[@<subtest>[@<dyn_subtest>]]`.\n\
>+\n\
>+ IGT_HOOK_TEST \n\
>+ Name of the current test.\n\
>+\n\
>+ IGT_HOOK_SUBTEST\n\
>+ Name of the current subtest. Will be the empty string if not running a\n\
>+ subtest.\n\
>+\n\
>+ IGT_HOOK_DYN_SUBTEST\n\
>+ Name of the current dynamic subtest. Will be the empty string if not running a\n\
>+ dynamic subtest.\n\
>+\n\
>+ IGT_HOOK_RESULT\n\
>+ String representing the result of the test/subtest/dynamic subtest. Possible\n\
>+ values are: SUCCESS, SKIP or FAIL. This is only applicable on \"post-*\"\n\
>+ events and will be the empty string for other types of events.\n\
>+\n\
>+");
>+}
>diff --git a/lib/igt_hook.h b/lib/igt_hook.h
>new file mode 100644
>index 000000000000..6fef7254a317
>--- /dev/null
>+++ b/lib/igt_hook.h
>@@ -0,0 +1,86 @@
>+/*
>+ * Copyright © 2024 Intel Corporation
>+ *
>+ * Permission is hereby granted, free of charge, to any person obtaining a
>+ * copy of this software and associated documentation files (the "Software"),
>+ * to deal in the Software without restriction, including without limitation
>+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>+ * and/or sell copies of the Software, and to permit persons to whom the
>+ * Software is furnished to do so, subject to the following conditions:
>+ *
>+ * The above copyright notice and this permission notice (including the next
>+ * paragraph) shall be included in all copies or substantial portions of the
>+ * Software.
>+ *
>+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>+ * IN THE SOFTWARE.
>+ */
ditto on SPDX
Lucas De Marchi
More information about the igt-dev
mailing list