Re: [PATCH i-g-t] Add support for subtest-specific documentation

["Belgaumkar, Vinay" <vinay.belgaumkar@xxxxxxxxx>]

Bump.


On 7/11/2017 3:08 PM, Belgaumkar, Vinay wrote:
> On 6/22/2017 4:30 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_subtest_documentation("Frob knobs to see if one of the "
> >                                "crossbeams will go out of skew on the "
> >      			     "treadle.");
> >      igt_subtest("knob-frobbing-askew")
> >        test_frob();
> >
> > or with a format string:
> >
> >     for_example_loop(e) {
> >       igt_subtest_documentation_f("Frob %s to see if one of the "
> >                                   "crossbeams will go out of skew on the "
> >         				"treadle.", 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.
> >
> > This patch does not hook the subtest documentation to the doc
> > building. It adds command line parameters for --document-all-subtests
> > and --document-subtest <namepattern>, but using them blindly without
> > parallelizing will cause the docs build time to rise too much.
> We should also update the README file to reflect the new command line
> options.  Will the subtest description now show up in the gtkdoc when we
> use the new igt_subtest_documentation functions?
> > Signed-off-by: Petri Latvala <petri.latvala@xxxxxxxxx>
> > ---
> >    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 eb563f7..4796b01 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 9a5ed40..0324382 100644
> > --- a/lib/igt_core.c
> > +++ b/lib/igt_core.c
> > @@ -98,7 +98,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.
> >     *
> > @@ -231,9 +231,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;
> > @@ -243,6 +243,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;
> >    
> > @@ -255,6 +262,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,
> > @@ -441,7 +450,7 @@ bool __igt_fixture(void)
> >    {
> >    	assert(!in_fixture);
> >    
> > -	if (igt_only_list_subtests())
> > +	if (igt_only_collect_data())
> >    		return false;
> >    
> >    	if (skip_subtests_henceforth)
> > @@ -535,7 +544,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);
> >    	}
> > @@ -555,7 +564,7 @@ static void print_version(void)
> >    {
> >    	struct utsname uts;
> >    
> > -	if (list_subtests)
> > +	if (igt_only_collect_data())
> >    		return;
> >    
> >    	uname(&uts);
> > @@ -571,6 +580,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"
> > @@ -604,6 +615,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},
> >    		{"help-description", 0, 0, OPT_DESCRIPTION},
> >    		{"debug", optional_argument, 0, OPT_DEBUG},
> >    		{"interactive-debug", optional_argument, 0, OPT_INTERACTIVE_DEBUG},
> > @@ -711,12 +724,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();
> > @@ -744,11 +769,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);
> >    	}
> >    
> > @@ -756,7 +781,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();
> > @@ -864,16 +889,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) {
> > @@ -890,10 +936,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);
> > +		if (err < 0)
> > +			current_subtest_documentation = NULL;
> Missing va_end?
> > +	}
> > +}
> > +
> > +
> > +/**
> >     * igt_subtest_name:
> >     *
> >     * Returns: The name of the currently executed subtest or NULL if called from
> > @@ -905,14 +992,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)
> > @@ -966,7 +1053,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);
> > @@ -1345,12 +1432,12 @@ void igt_exit(void)
> >    {
> >    	igt_exit_called = true;
> >    
> > -	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 */
> > @@ -1914,7 +2001,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) {
> > @@ -1989,7 +2076,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 a2ed972..423e5e7 100644
> > --- a/lib/igt_core.h
> > +++ b/lib/igt_core.h
> > @@ -195,8 +195,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);
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx