Re: [PATCH 2/2] intel-gpu-tools: Re-use igt_subtest_init for simple tests

Daniel Vetter
2014-07-07T16:09:34Z

On Fri, Jun 27, 2014 at 03:15:37PM +0100, tim.gore@intel.com wrote:
> From: Tim Gore <tim.gore@intel.com>
>
> igt_subtest_init mainly does stuff that we also want for
> simple/single tests, such as looking for --list-subtests
> and --help options and calling common_init. So just call
> this from igt_simple_init and then set tests_with_subtests
> to false. NOTE that this means that check_igt_exit is now
> registered as an exit handler for single tests, so need to
> make sure that ALL tests exit via igt_exit.
>
> Signed-off-by: Tim Gore <tim.gore@intel.com>
> ---
> lib/igt_core.c | 32 ++++++++++++++++----------------
> lib/igt_core.h | 4 ++--
> tests/gem_gtt_hog.c | 2 +-
> tests/igt_simulation.c | 4 ++--
> 4 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 7ac7ebe..aaeaa3b 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -458,13 +458,15 @@ void igt_subtest_init(int argc, char **argv)
> * #igt_simple_main block instead of stitching the tests's main() function together
> * manually.
> */
> -void igt_simple_init(void)
> +void igt_simple_init(int argc, char **argv)
> {
> - print_version();
> -
> - oom_adjust_for_doom();
> -
> - common_init();
> + /* Use the same init function as is used with subtests - we want most of its functionality */
> + /* Note that this will install the igt_exit_handler so you need to exit via igt_exit(), */
> + /* Dont call exit() */
> + igt_subtest_init(argc, argv);

Not sure whether forcing the reuse of igt_subtest_init makes a lot of
sense since it requires a lot of follow-up changes all over. I'd just add
a bit of argparsing here with the required special cases, i.e.
- exit without doing anything for --list-subtests
- exit with 77 when anything is specified with --run-subtests

Also I prefer if the piglit changes come together with this patch so that
we can roll it all out together.
-Daniel

> + test_with_subtests = false;
> + if (list_subtests)
> + igt_exit();
> }
>
> /*
> @@ -565,7 +567,7 @@ void igt_skip(const char *f, ...)
> assert(in_fixture);
> __igt_fixture_end();
> } else {
> - exit(IGT_EXIT_SKIP);
> + igt_exit();
> }
> }
>
> @@ -655,7 +657,7 @@ void igt_fail(int exitcode)
> __igt_fixture_end();
> }
>
> - exit(exitcode);
> + igt_exit();
> }
> }
>
> @@ -713,18 +715,16 @@ void igt_exit(void)
> if (igt_only_list_subtests())
> exit(IGT_EXIT_SUCCESS);
>
> - if (!test_with_subtests)
> - exit(IGT_EXIT_SUCCESS);
> -
> - /* Calling this without calling one of the above is a failure */
> - assert(skipped_one || succeeded_one || failed_one);
> + if (test_with_subtests)
> + /* Calling this without calling one of the above is a failure */
> + assert(skipped_one || succeeded_one || failed_one);
>
> if (failed_one)
> exit(igt_exitcode);
> - else if (succeeded_one)
> - exit(IGT_EXIT_SUCCESS);
> - else
> + else if (skipped_one)
> exit(IGT_EXIT_SKIP);
> + else
> + exit(IGT_EXIT_SUCCESS);
> }
>
> /* fork support code */
> diff --git a/lib/igt_core.h b/lib/igt_core.h
> index 9853e6b..cabbc3b 100644
> --- a/lib/igt_core.h
> +++ b/lib/igt_core.h
> @@ -166,7 +166,7 @@ bool igt_only_list_subtests(void);
> *
> * Init for simple tests without subtests
> */
> -void igt_simple_init(void);
> +void igt_simple_init(int argc, char **argv);
>
> /**
> * igt_simple_main:
> @@ -178,7 +178,7 @@ void igt_simple_init(void);
> #define igt_simple_main \
> static void igt_tokencat(__real_main, __LINE__)(int argc, char **argv); \
> int main(int argc, char **argv) { \
> - igt_simple_init(); \
> + igt_simple_init(argc, argv); \
> igt_tokencat(__real_main, __LINE__)(argc, argv); \
> igt_exit(); \
> } \
> diff --git a/tests/gem_gtt_hog.c b/tests/gem_gtt_hog.c
> index 5d47540..f607ea0 100644
> --- a/tests/gem_gtt_hog.c
> +++ b/tests/gem_gtt_hog.c
> @@ -150,7 +150,7 @@ static void run(data_t *data, int child)
> munmap(ptr, size);
>
> igt_assert(x == canary);
> - exit(0);
> + _exit(0);
> }
>
> igt_simple_main
> diff --git a/tests/igt_simulation.c b/tests/igt_simulation.c
> index 15cbe64..3048c9e 100644
> --- a/tests/igt_simulation.c
> +++ b/tests/igt_simulation.c
> @@ -53,11 +53,11 @@ static int do_fork(void)
> assert(0);
> case 0:
> if (simple) {
> - igt_simple_init();
> + igt_simple_init(1, argv_run);
>
> igt_skip_on_simulation();
>
> - exit(0);
> + igt_exit();
> } else {
> if (list_subtests)
> igt_subtest_init(2, argv_list);
> --
> 1.9.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch