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

[Daniel Vetter <daniel@ffwll.ch>]

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