[Intel-gfx] [PATCH i-g-t 1/2] Add support for subtest-specific documentation

Belgaumkar, Vinay vinay.belgaumkar at intel.com
Wed Aug 9 20:18:55 UTC 2017



On 8/9/2017 4:40 AM, Petri Latvala wrote:
> The current documentation for tests is limited to a single string per
> test binary. This patch adds support for documenting individual
> subtests.
>
> The syntax for subtest documentation is:
>
>     igt_document_subtest("Frob knobs to see if one of the "
>                          "crossbeams will go out of skew on the "
>                          "treadle.\n");
>     igt_subtest("knob-frobbing-askew")
>       test_frob();
>
> or with a format string:
>
>    for_example_loop(e) {
>      igt_document_subtest_f("Frob %s to see if one of the "
>                             "crossbeams will go out of skew on the "
>                             "treadle.\n", e->readable_name);
>      igt_subtest_f("%s-frob-askew", e->name)
>        test_frob(e);
>    }
>
> The documentation cannot be extracted from just comments, because
> associating them with the correct subtest name will then require doing
> pattern matching in the documentation generator, for subtests where
> the name is generated at runtime using igt_subtest_f.
>
> v2: Rebase, change function name in commit message to match code
>
> Signed-off-by: Petri Latvala <petri.latvala at intel.com>
> Acked-by: Leo Li <sunpeng.li at amd.com>
> ---
>   lib/igt_aux.c  |   8 ++--
>   lib/igt_core.c | 147 +++++++++++++++++++++++++++++++++++++++++++++------------
>   lib/igt_core.h |   6 ++-
>   3 files changed, 126 insertions(+), 35 deletions(-)
>
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index f428f15..d56f41f 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -311,7 +311,7 @@ static void sig_handler(int i)
>    */
>   void igt_fork_signal_helper(void)
>   {
> -	if (igt_only_list_subtests())
> +	if (igt_only_collect_data())
>   		return;
>   
>   	/* We pick SIGCONT as it is a "safe" signal - if we send SIGCONT to
> @@ -344,7 +344,7 @@ void igt_fork_signal_helper(void)
>    */
>   void igt_stop_signal_helper(void)
>   {
> -	if (igt_only_list_subtests())
> +	if (igt_only_collect_data())
>   		return;
>   
>   	igt_stop_helper(&signal_helper);
> @@ -375,7 +375,7 @@ static void __attribute__((noreturn)) shrink_helper_process(int fd, pid_t pid)
>    */
>   void igt_fork_shrink_helper(int drm_fd)
>   {
> -	assert(!igt_only_list_subtests());
> +	assert(!igt_only_collect_data());
>   	igt_require(igt_drop_caches_has(drm_fd, DROP_SHRINK_ALL));
>   	igt_fork_helper(&shrink_helper)
>   		shrink_helper_process(drm_fd, getppid());
> @@ -473,7 +473,7 @@ void igt_stop_hang_detector(void)
>   #else
>   void igt_fork_hang_detector(int fd)
>   {
> -	if (igt_only_list_subtests())
> +	if (igt_only_collect_data())
>   		return;
>   }
>   
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index c0488e9..f1cb0e9 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -99,7 +99,7 @@
>    *
>    * To allow this i-g-t provides #igt_fixture code blocks for setup code outside
>    * of subtests and automatically skips the subtest code blocks themselves. For
> - * special cases igt_only_list_subtests() is also provided. For setup code only
> + * special cases igt_only_collect_data() is also provided. For setup code only
>    * shared by a group of subtest encapsulate the #igt_fixture block and all the
>    * subtestest in a #igt_subtest_group block.
>    *
> @@ -253,9 +253,9 @@ static unsigned int exit_handler_count;
>   const char *igt_interactive_debug;
>   
>   /* subtests helpers */
> -static bool list_subtests = false;
> -static char *run_single_subtest = NULL;
> -static bool run_single_subtest_found = false;
> +static char *single_subtest = NULL;
> +static bool single_subtest_found = false;
> +static char *current_subtest_documentation = NULL;
>   static const char *in_subtest = NULL;
>   static struct timespec subtest_time;
>   static clockid_t igt_clock = (clockid_t)-1;
> @@ -265,6 +265,13 @@ static bool in_atexit_handler = false;
>   static enum {
>   	CONT = 0, SKIP, FAIL
>   } skip_subtests_henceforth = CONT;
> +static enum {
> +	EXECUTE_ALL,
> +	EXECUTE_SINGLE,
> +	LIST_SUBTESTS,
> +	DOCUMENT,
> +	DOCUMENT_SINGLE
> +} runmode = EXECUTE_ALL;
>   
>   bool __igt_plain_output = false;
>   
> @@ -277,6 +284,8 @@ bool test_child;
>   enum {
>    OPT_LIST_SUBTESTS,
>    OPT_RUN_SUBTEST,
> + OPT_DOC_SUBTESTS,
> + OPT_DOC_SINGLE_SUBTEST,
>    OPT_DESCRIPTION,
>    OPT_DEBUG,
>    OPT_INTERACTIVE_DEBUG,
> @@ -469,7 +478,7 @@ bool __igt_fixture(void)
>   {
>   	assert(!in_fixture);
>   
> -	if (igt_only_list_subtests())
> +	if (igt_only_collect_data())
>   		return false;
>   
>   	if (skip_subtests_henceforth)
> @@ -563,7 +572,7 @@ static void low_mem_killer_disable(bool disable)
>   bool igt_exit_called;
>   static void common_exit_handler(int sig)
>   {
> -	if (!igt_only_list_subtests()) {
> +	if (!igt_only_collect_data()) {
>   		low_mem_killer_disable(false);
>   		kick_fbcon(true);
>   	}
> @@ -583,7 +592,7 @@ static void print_version(void)
>   {
>   	struct utsname uts;
>   
> -	if (list_subtests)
> +	if (igt_only_collect_data())
>   		return;
>   
>   	uname(&uts);
> @@ -599,6 +608,8 @@ static void print_usage(const char *help_str, bool output_on_stderr)
>   
>   	fprintf(f, "Usage: %s [OPTIONS]\n", command_str);
>   	fprintf(f, "  --list-subtests\n"
> +		   "  --document-all-subtests\n"
> +		   "  --document-subtest <pattern>\n"
>   		   "  --run-subtest <pattern>\n"
>   		   "  --debug[=log-domain]\n"
>   		   "  --interactive-debug[=domain]\n"
> @@ -710,6 +721,8 @@ static int common_init(int *argc, char **argv,
>   	static struct option long_options[] = {
>   		{"list-subtests", 0, 0, OPT_LIST_SUBTESTS},
>   		{"run-subtest", 1, 0, OPT_RUN_SUBTEST},
> +		{"document-all-subtests", 0, 0, OPT_DOC_SUBTESTS},
> +		{"document-subtest", 1, 0, OPT_DOC_SINGLE_SUBTEST},

Can we add these options to the README file as well?

>   		{"help-description", 0, 0, OPT_DESCRIPTION},
>   		{"debug", optional_argument, 0, OPT_DEBUG},
>   		{"interactive-debug", optional_argument, 0, OPT_INTERACTIVE_DEBUG},
> @@ -800,12 +813,24 @@ static int common_init(int *argc, char **argv,
>   				igt_log_domain_filter = strdup(optarg);
>   			break;
>   		case OPT_LIST_SUBTESTS:
> -			if (!run_single_subtest)
> -				list_subtests = true;
> +			if (runmode == EXECUTE_ALL)
> +				runmode = LIST_SUBTESTS;
>   			break;
>   		case OPT_RUN_SUBTEST:
> -			if (!list_subtests)
> -				run_single_subtest = strdup(optarg);
> +			if (runmode == EXECUTE_ALL) {
> +				runmode = EXECUTE_SINGLE;
> +				single_subtest = strdup(optarg);
> +			}
> +			break;
> +		case OPT_DOC_SUBTESTS:
> +			if (runmode == EXECUTE_ALL)
> +				runmode = DOCUMENT;
> +			break;
> +		case OPT_DOC_SINGLE_SUBTEST:
> +			if (runmode == EXECUTE_ALL) {
> +				runmode = DOCUMENT_SINGLE;
> +				single_subtest = strdup(optarg);
> +			}
>   			break;
>   		case OPT_DESCRIPTION:
>   			print_test_description();
> @@ -837,11 +862,11 @@ out:
>   	/* exit immediately if this test has no subtests and a subtest or the
>   	 * list of subtests has been requested */
>   	if (!test_with_subtests) {
> -		if (run_single_subtest) {
> -			igt_warn("Unknown subtest: %s\n", run_single_subtest);
> +		if (runmode == EXECUTE_SINGLE || runmode == DOCUMENT_SINGLE) {
> +			igt_warn("Unknown subtest: %s\n", single_subtest);
>   			exit(IGT_EXIT_INVALID);
>   		}
> -		if (list_subtests)
> +		if (runmode == LIST_SUBTESTS || runmode == DOCUMENT)
>   			exit(IGT_EXIT_INVALID);
>   	}
>   
> @@ -849,7 +874,7 @@ out:
>   		/* exit with no error for -h/--help */
>   		exit(ret == -1 ? 0 : IGT_EXIT_INVALID);
>   
> -	if (!list_subtests) {
> +	if (!igt_only_collect_data()) {
>   		kick_fbcon(false);
>   		kmsg(KERN_INFO "[IGT] %s: executing\n", command_str);
>   		print_version();
> @@ -957,16 +982,37 @@ bool __igt_run_subtest(const char *subtest_name)
>   			igt_exit();
>   		}
>   
> -	if (list_subtests) {
> +	if (runmode == LIST_SUBTESTS) {
>   		printf("%s\n", subtest_name);
>   		return false;
>   	}
>   
> -	if (run_single_subtest) {
> -		if (uwildmat(subtest_name, run_single_subtest) == 0)
> +	if (runmode == DOCUMENT) {
> +		if (current_subtest_documentation) {
> +			printf("%s:\n\n", subtest_name);
> +			printf("%s", current_subtest_documentation);
> +			free(current_subtest_documentation);
> +			current_subtest_documentation = NULL;
> +		}
> +		return false;
> +	}
> +
> +	if (runmode == EXECUTE_SINGLE || runmode == DOCUMENT_SINGLE) {
> +		if (uwildmat(subtest_name, single_subtest) == 0)
>   			return false;
> -		else
> -			run_single_subtest_found = true;
> +		else {
> +			single_subtest_found = true;
> +
> +			if (runmode == DOCUMENT_SINGLE) {
> +				if (current_subtest_documentation) {
> +					printf("%s", current_subtest_documentation);
> +					free(current_subtest_documentation);
> +					current_subtest_documentation = NULL;
> +				}
> +
> +				return false;
> +			}
> +		}
>   	}
>   
>   	if (skip_subtests_henceforth) {
> @@ -983,10 +1029,51 @@ bool __igt_run_subtest(const char *subtest_name)
>   	_igt_log_buffer_reset();
>   
>   	gettime(&subtest_time);
> +
>   	return (in_subtest = subtest_name);
>   }
>   
>   /**
> + * igt_document_subtest:
> + * @documentation: documentation for the next subtest
> + *
> + * This function sets the documentation string for the next occurring subtest.
> + */
> +void igt_document_subtest(const char *documentation)
> +{
> +	if (runmode == DOCUMENT || runmode == DOCUMENT_SINGLE) {
> +		free(current_subtest_documentation);
> +		current_subtest_documentation = strdup(documentation);
> +	}
> +}
> +
> +/**
> + * igt_document_subtest_f:
> + * @documentation: Documentation for the next subtest
> + * @...: format string and optional arguments
> + *
> + * This function sets the documentation string for the next occurring subtest.
> + *
> + * Like igt_document_subtest(), but also accepts a printf format
> + * string instead of a static string.
> + */
> +__attribute__((format(printf, 1, 2)))
> +void igt_document_subtest_f(const char *documentation, ...)
> +{
> +	int err;
> +	va_list args;
> +
> +	if (runmode == DOCUMENT || runmode == DOCUMENT_SINGLE) {
> +		free(current_subtest_documentation);
> +		va_start(args, documentation);
> +		err = vasprintf(&current_subtest_documentation, documentation, args);

Missing va_end?

> +		if (err < 0)
> +			current_subtest_documentation = NULL;
> +	}
> +}
> +
> +
> +/**
>    * igt_subtest_name:
>    *
>    * Returns: The name of the currently executed subtest or NULL if called from
> @@ -998,14 +1085,14 @@ const char *igt_subtest_name(void)
>   }
>   
>   /**
> - * igt_only_list_subtests:
> + * igt_only_collect_data:
>    *
> - * Returns: Returns true if only subtest should be listed and any setup code
> + * Returns: Returns true if the running mode is only collecting data and any setup code
>    * must be skipped, false otherwise.
>    */
> -bool igt_only_list_subtests(void)
> +bool igt_only_collect_data(void)
>   {
> -	return list_subtests;
> +	return runmode != EXECUTE_ALL && runmode != EXECUTE_SINGLE;
>   }
>   
>   void __igt_subtest_group_save(int *save)
> @@ -1059,7 +1146,7 @@ void igt_skip(const char *f, ...)
>   
>   	assert(!test_child);
>   
> -	if (!igt_only_list_subtests()) {
> +	if (!igt_only_collect_data()) {
>   		va_start(args, f);
>   		vprintf(f, args);
>   		va_end(args);
> @@ -1443,12 +1530,12 @@ void igt_exit(void)
>   		g_key_file_free(igt_key_file);
>   #endif
>   
> -	if (run_single_subtest && !run_single_subtest_found) {
> -		igt_warn("Unknown subtest: %s\n", run_single_subtest);
> +	if (single_subtest && !single_subtest_found) {
> +		igt_warn("Unknown subtest: %s\n", single_subtest);
>   		exit(IGT_EXIT_INVALID);
>   	}
>   
> -	if (igt_only_list_subtests())
> +	if (igt_only_collect_data())
>   		exit(IGT_EXIT_SUCCESS);
>   
>   	/* Calling this without calling one of the above is a failure */
> @@ -2012,7 +2099,7 @@ bool igt_run_in_simulation(void)
>    */
>   void igt_skip_on_simulation(void)
>   {
> -	if (igt_only_list_subtests())
> +	if (igt_only_collect_data())
>   		return;
>   
>   	if (!in_fixture && !in_subtest) {
> @@ -2087,7 +2174,7 @@ void igt_vlog(const char *domain, enum igt_log_level level, const char *format,
>   	program_name = command_str;
>   #endif
>   
> -	if (list_subtests && level <= IGT_LOG_WARN)
> +	if (igt_only_collect_data() && level <= IGT_LOG_WARN)
>   		return;
>   
>   	if (vasprintf(&line, format, args) == -1)
> diff --git a/lib/igt_core.h b/lib/igt_core.h
> index 1619a9d..275e467 100644
> --- a/lib/igt_core.h
> +++ b/lib/igt_core.h
> @@ -197,8 +197,12 @@ bool __igt_run_subtest(const char *subtest_name);
>   #define igt_subtest_f(f...) \
>   	__igt_subtest_f(igt_tokencat(__tmpchar, __LINE__), f)
>   
> +void igt_document_subtest(const char *documentation);
> +__attribute__((format(printf, 1, 2)))
> +void igt_document_subtest_f(const char *documentation, ...);
> +
>   const char *igt_subtest_name(void);
> -bool igt_only_list_subtests(void);
> +bool igt_only_collect_data(void);
>   
>   void __igt_subtest_group_save(int *);
>   void __igt_subtest_group_restore(int);

I have also sent you a separate email with the gtkdoc generated after 
using the new method.



More information about the Intel-gfx mailing list