Re: [igt-dev] [PATCH i-g-t 3/9] lib: Make it possible to abort the whole execution from inside of a test

[Petri Latvala <petri.latvala@intel.com>]

On Wed, Feb 12, 2020 at 03:21:17PM +0200, Arkadiusz Hiler wrote:
> igt_abort_on_f() is introduced which does very little cleanup and causes
> a hard exit() of the test binary with a unique exit code
> (IGT_EXIT_ABORT).
> 
> The exit code informs the monitoring process that there is a critical
> issue with the testing environment which may have an impact on the
> results if testing continues.
> 
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>

Reviewed-by: Petri Latvala <petri.latvala@intel.com>


> ---
>  lib/igt_core.c        |  46 ++++++++-
>  lib/igt_core.h        |  29 ++++++
>  lib/tests/igt_abort.c | 227 ++++++++++++++++++++++++++++++++++++++++++
>  lib/tests/meson.build |   1 +
>  4 files changed, 300 insertions(+), 3 deletions(-)
>  create mode 100644 lib/tests/igt_abort.c
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 51041793..8bd1e642 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -587,6 +587,7 @@ static void ftrace_dump_on_oops(bool enable)
>  }
>  
>  bool igt_exit_called;
> +bool igt_is_aborting;
>  static void common_exit_handler(int sig)
>  {
>  	if (!igt_only_list_subtests()) {
> @@ -595,7 +596,7 @@ static void common_exit_handler(int sig)
>  
>  	/* When not killed by a signal check that igt_exit() has been properly
>  	 * called. */
> -	assert(sig != 0 || igt_exit_called);
> +	assert(sig != 0 || igt_exit_called || igt_is_aborting);
>  }
>  
>  static void print_line_wrapping(const char *indent, const char *text)
> @@ -1944,6 +1945,46 @@ void __igt_fail_assert(const char *domain, const char *file, const int line,
>  	igt_fail(IGT_EXIT_FAILURE);
>  }
>  
> +static void kill_children(void)
> +{
> +	for (int c = 0; c < num_test_children; c++)
> +		kill(test_children[c], SIGKILL);
> +}
> +
> +void __igt_abort(const char *domain, const char *file, const int line,
> +		 const char *func, const char *expression,
> +		 const char *f, ...)
> +{
> +	va_list args;
> +	int err = errno;
> +
> +	igt_is_aborting = true;
> +
> +	igt_log(domain, IGT_LOG_CRITICAL,
> +		"Test abort in function %s, file %s:%i:\n", func, file,
> +		line);
> +	igt_log(domain, IGT_LOG_CRITICAL, "abort condition: %s\n", expression);
> +	if (err)
> +		igt_log(domain, IGT_LOG_CRITICAL, "Last errno: %i, %s\n", err,
> +			strerror(err));
> +
> +	if (f) {
> +		va_start(args, f);
> +		igt_vlog(domain, IGT_LOG_CRITICAL, f, args);
> +		va_end(args);
> +	}
> +
> +	/* just try our best, we are aborting the execution anyway */
> +	kill_children();
> +
> +	print_backtrace();
> +
> +	if (running_under_gdb())
> +		abort();
> +
> +	exit(IGT_EXIT_ABORT);
> +}
> +
>  /**
>   * igt_exit:
>   *
> @@ -1993,8 +2034,7 @@ void igt_exit(void)
>  			 command_str, igt_exitcode);
>  	igt_debug("Exiting with status code %d\n", igt_exitcode);
>  
> -	for (int c = 0; c < num_test_children; c++)
> -		kill(test_children[c], SIGKILL);
> +	kill_children();
>  	assert(!num_test_children);
>  
>  	assert(waitpid(-1, &tmp, WNOHANG) == -1 && errno == ECHILD);
> diff --git a/lib/igt_core.h b/lib/igt_core.h
> index c17a7ba8..a79b41af 100644
> --- a/lib/igt_core.h
> +++ b/lib/igt_core.h
> @@ -123,6 +123,14 @@ struct _GKeyFile *igt_load_igtrc(void);
>   */
>  #define IGT_EXIT_FAILURE 98
>  
> +/**
> + * IGT_EXIT_ABORT
> + *
> + * Exit status indicating a severe test/enviroment failure, any continued
> + * testing past this point can yeild unexpected reasults and is not recommended
> + */
> +#define IGT_EXIT_ABORT 112
> +
>  bool __igt_fixture(void);
>  void __igt_fixture_complete(void);
>  void __igt_fixture_end(void) __attribute__((noreturn));
> @@ -499,6 +507,11 @@ void __igt_fail_assert(const char *domain, const char *file,
>  		       const int line, const char *func, const char *assertion,
>  		       const char *format, ...)
>  	__attribute__((noreturn));
> +__attribute__((format(printf, 6, 7)))
> +void __igt_abort(const char *domain, const char *file, const int line,
> +		 const char *func, const char *expression,
> +		 const char *f, ...)
> +	__attribute__((noreturn));
>  void igt_exit(void) __attribute__((noreturn));
>  void igt_fatal_error(void) __attribute__((noreturn));
>  
> @@ -1027,6 +1040,22 @@ void igt_describe_f(const char *fmt, ...);
>  	else igt_debug("Test requirement passed: !(%s)\n", #expr); \
>  } while (0)
>  
> +
> +/**
> + * igt_abort_on_f:
> + * @expr: condition to test
> + * @...: format string and optional arguments
> + *
> + * Aborts current execution if a condition is met.
> + *
> + * Should be used only when there is a serious issue with the environment and
> + * any further testing may be affected by it.
> + */
> +#define igt_abort_on_f(expr, f...) \
> +	do { if ((expr)) \
> +		__igt_abort(IGT_LOG_DOMAIN, __FILE__, __LINE__, __func__, #expr , f); \
> +	} while (0)
> +
>  /* fork support code */
>  bool __igt_fork(void);
>  
> diff --git a/lib/tests/igt_abort.c b/lib/tests/igt_abort.c
> new file mode 100644
> index 00000000..53b7d4fd
> --- /dev/null
> +++ b/lib/tests/igt_abort.c
> @@ -0,0 +1,227 @@
> +/*
> + * Copyright © 2020 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.
> + *
> + */
> +
> +#include "igt_core.h"
> +#include "drmtest.h"
> +
> +#include "igt_tests_common.h"
> +
> +char test[] = "test";
> +char *fake_argv[] = { test };
> +int fake_argc = ARRAY_SIZE(fake_argv);
> +
> +static void fake_simple_test(void)
> +{
> +	igt_simple_init(fake_argc, fake_argv);
> +
> +	igt_abort_on_f(true, "I'm out!\n");
> +
> +	exit(0); /* unreachable */
> +}
> +
> +static void fake_fixture_test(void)
> +{
> +	igt_subtest_init(fake_argc, fake_argv);
> +
> +	igt_fixture {
> +		igt_abort_on_f(true, "I'm out!\n");
> +	}
> +
> +	exit(0); /* unreachable */
> +}
> +
> +static void fake_outside_fixture_test(void)
> +{
> +	igt_subtest_init(fake_argc, fake_argv);
> +
> +	igt_abort_on_f(true, "I'm out!\n");
> +
> +	exit(0); /* unreachable */
> +}
> +
> +static void fake_subtest_test(void)
> +{
> +	igt_subtest_init(fake_argc, fake_argv);
> +
> +	igt_subtest("A")
> +		;
> +
> +	igt_subtest("B")
> +		igt_abort_on_f(true, "I'm out!\n");
> +
> +	igt_subtest("C")
> +		exit(0); /* unreachable */
> +
> +	exit(0); /* unreachable */
> +}
> +
> +static void fake_dynamic_test(void)
> +{
> +	igt_subtest_init(fake_argc, fake_argv);
> +
> +	igt_subtest_with_dynamic("A") {
> +		igt_dynamic("AA")
> +			;
> +		igt_dynamic("AB")
> +			igt_abort_on_f(true, "I'm out!\n");
> +
> +		igt_dynamic("AC")
> +			exit(0); /* unreachable */
> +
> +	}
> +
> +	igt_subtest("B")
> +		exit(0); /* unreachable */
> +
> +	exit(0); /* unreachable */
> +}
> +
> +static void fake_outside_dynamic_test(void)
> +{
> +	igt_subtest_init(fake_argc, fake_argv);
> +
> +	igt_subtest_with_dynamic("A") {
> +		igt_dynamic("AA")
> +			;
> +
> +		igt_abort_on_f(true, "I'm out!\n");
> +
> +		igt_dynamic("AB")
> +			exit(0); /* unreachable */
> +
> +		igt_dynamic("AC")
> +			exit(0); /* unreachable */
> +
> +	}
> +
> +	igt_subtest("B")
> +		exit(0); /* unreachable */
> +
> +	exit(0); /* unreachable */
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	int status;
> +	pid_t pid;
> +
> +	/* make sure that we log the message and can abort from a simple test*/ {
> +		static char err[4096];
> +		int errfd;
> +
> +		pid = do_fork_bg_with_pipes(fake_simple_test, NULL, &errfd);
> +
> +		read_whole_pipe(errfd, err, sizeof(err));
> +
> +		internal_assert(strstr(err, "CRITICAL: Test abort"));
> +		internal_assert(strstr(err, "I'm out!"));
> +
> +		internal_assert(safe_wait(pid, &status) != -1);
> +		internal_assert_wexited(status, IGT_EXIT_ABORT);
> +	}
> +
> +	/* make sure that we can abort from a fixture */ {
> +		pid = do_fork_bg_with_pipes(fake_fixture_test, NULL, NULL);
> +		internal_assert(safe_wait(pid, &status) != -1);
> +		internal_assert_wexited(status, IGT_EXIT_ABORT);
> +	}
> +
> +	/* make sure that we can abort from outside fixture/subtest */ {
> +		pid = do_fork_bg_with_pipes(fake_outside_fixture_test, NULL, NULL);
> +		internal_assert(safe_wait(pid, &status) != -1);
> +		internal_assert_wexited(status, IGT_EXIT_ABORT);
> +	}
> +
> +	/* make sure we abort during B and don't see B's end/C start */ {
> +		static char out[4096];
> +		int outfd;
> +
> +		pid = do_fork_bg_with_pipes(fake_subtest_test, &outfd, NULL);
> +
> +		read_whole_pipe(outfd, out, sizeof(out));
> +
> +		internal_assert(safe_wait(pid, &status) != -1);
> +		internal_assert_wexited(status, IGT_EXIT_ABORT);
> +
> +		internal_assert(strstr(out, "Starting subtest: A"));
> +		internal_assert(strstr(out, "Subtest A:"));
> +
> +		internal_assert(strstr(out, "Starting subtest: B"));
> +		internal_assert(!strstr(out, "Subtest B:"));
> +
> +		internal_assert(!strstr(out, "Starting subtest: C"));
> +
> +		close(outfd);
> +	}
> +
> +	/* make sure we abort during AB and don't see AC/B */ {
> +		static char out[4096];
> +		int outfd;
> +
> +		pid = do_fork_bg_with_pipes(fake_dynamic_test, &outfd, NULL);
> +
> +		read_whole_pipe(outfd, out, sizeof(out));
> +
> +		internal_assert(safe_wait(pid, &status) != -1);
> +		internal_assert_wexited(status, IGT_EXIT_ABORT);
> +
> +		internal_assert(strstr(out, "Starting subtest: A"));
> +		internal_assert(strstr(out, "Starting dynamic subtest: AA"));
> +		internal_assert(strstr(out, "Dynamic subtest AA:"));
> +
> +		internal_assert(strstr(out, "Starting dynamic subtest: AB"));
> +		internal_assert(!strstr(out, "Dynamic subtest AB:"));
> +
> +		internal_assert(!strstr(out, "Starting subtest: B"));
> +
> +		close(outfd);
> +
> +	}
> +
> +	/* make sure we abort between AA and AB */ {
> +		static char out[4096];
> +		int outfd;
> +
> +		pid = do_fork_bg_with_pipes(fake_outside_dynamic_test, &outfd, NULL);
> +
> +		read_whole_pipe(outfd, out, sizeof(out));
> +
> +		internal_assert(safe_wait(pid, &status) != -1);
> +		internal_assert_wexited(status, IGT_EXIT_ABORT);
> +
> +		internal_assert(strstr(out, "Starting subtest: A"));
> +		internal_assert(strstr(out, "Starting dynamic subtest: AA"));
> +		internal_assert(strstr(out, "Dynamic subtest AA:"));
> +
> +		internal_assert(!strstr(out, "Starting dynamic subtest: AB"));
> +		internal_assert(!strstr(out, "Dynamic subtest AB:"));
> +
> +		internal_assert(!strstr(out, "Starting subtest: B"));
> +
> +		close(outfd);
> +
> +	}
> +
> +	return 0;
> +}
> diff --git a/lib/tests/meson.build b/lib/tests/meson.build
> index e5066665..e75a723a 100644
> --- a/lib/tests/meson.build
> +++ b/lib/tests/meson.build
> @@ -1,5 +1,6 @@
>  lib_tests = [
>  	'igt_assert',
> +	'igt_abort',
>  	'igt_can_fail',
>  	'igt_can_fail_simple',
>  	'igt_conflicting_args',
> -- 
> 2.24.1
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev