All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t 1/5] lib/igt_core: Add support for subtest descriptions
@ 2019-07-01 12:21 Arkadiusz Hiler
  2019-07-01 12:21 ` [igt-dev] [PATCH i-g-t 2/5] tests/kms_hdmi_inject: Provide igt_descriptions Arkadiusz Hiler
                   ` (9 more replies)
  0 siblings, 10 replies; 17+ messages in thread
From: Arkadiusz Hiler @ 2019-07-01 12:21 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala

This patch adds igt_description() which attaches a description to the
following igt_subtest or igt_subtest_group block.

Descriptions are accessible via './test --describe[=pattern]'

Subtest description is its own igt_describe as well as igt_describes
of all the parenting igt_subtest_groups, starting from the outermost
scope.

Examples of code and produced outputs are included in
lib/test/igt_describe.c and as a documentation comment on igt_describe()
macro.

Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Acked-by: Petri Latvala <petri.latvala@intel.com>
---
 lib/igt_core.c           | 179 ++++++++++++++++++++++++--
 lib/igt_core.h           | 137 ++++++++++++++++++--
 lib/tests/igt_describe.c | 266 +++++++++++++++++++++++++++++++++++++++
 lib/tests/meson.build    |   1 +
 4 files changed, 562 insertions(+), 21 deletions(-)
 create mode 100644 lib/tests/igt_describe.c

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 6b9f0425..8b1c7b2f 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -70,6 +70,7 @@
 #include "igt_sysfs.h"
 #include "igt_sysrq.h"
 #include "igt_rc.h"
+#include "igt_list.h"
 
 #define UNW_LOCAL_ONLY
 #include <libunwind.h>
@@ -259,6 +260,7 @@ const char *igt_interactive_debug;
 
 /* subtests helpers */
 static bool list_subtests = false;
+static bool describe_subtests = false;
 static char *run_single_subtest = NULL;
 static bool run_single_subtest_found = false;
 static const char *in_subtest = NULL;
@@ -271,6 +273,16 @@ static enum {
 	CONT = 0, SKIP, FAIL
 } skip_subtests_henceforth = CONT;
 
+static char __current_description[4096];
+
+struct description_node {
+	char desc[sizeof(__current_description)];
+	struct igt_list link;
+};
+
+static struct igt_list subgroup_descriptions;
+
+
 bool __igt_plain_output = false;
 
 /* fork support state */
@@ -285,6 +297,7 @@ enum {
 	 * conflict with core ones
 	 */
 	OPT_LIST_SUBTESTS = 500,
+	OPT_DESCRIBE_SUBTESTS,
 	OPT_RUN_SUBTEST,
 	OPT_DESCRIPTION,
 	OPT_DEBUG,
@@ -528,10 +541,59 @@ static void common_exit_handler(int sig)
 	assert(sig != 0 || igt_exit_called);
 }
 
+static void print_line_wrapping(const char *indent, const char *text)
+{
+	char *copy, *curr, *next_space;
+	int current_line_length = 0;
+	bool done = false;
+
+	const int total_line_length = 80;
+	const int line_length = total_line_length - strlen(indent);
+
+	copy = malloc(strlen(text) + 1);
+	memcpy(copy, text, strlen(text) + 1);
+
+	curr = copy;
+
+	printf("%s", indent);
+
+	while (!done) {
+		next_space = strchr(curr, ' ');
+
+		if (!next_space) { /* no more spaces, print everything that is left */
+			done = true;
+			next_space = strchr(curr, '\0');
+		}
+
+		*next_space = '\0';
+
+		if ((next_space - curr) + current_line_length > line_length && curr != copy) {
+			printf("\n%s", indent);
+			current_line_length = 0;
+		}
+
+		if (current_line_length == 0)
+			printf("%s", curr); /* first word in a line, don't space out */
+		else
+			printf(" %s", curr);
+
+		current_line_length += next_space - curr;
+		curr = next_space + 1;
+	}
+
+	printf("\n");
+
+	free(copy);
+}
+
+
 static void print_test_description(void)
 {
-	if (&__igt_test_description)
-		printf("%s\n", __igt_test_description);
+	if (&__igt_test_description) {
+		print_line_wrapping("", __igt_test_description);
+		if (describe_subtests)
+			printf("\n");
+	}
 }
 
 static void print_version(void)
@@ -558,6 +620,7 @@ static void print_usage(const char *help_str, bool output_on_stderr)
 		   "  --debug[=log-domain]\n"
 		   "  --interactive-debug[=domain]\n"
 		   "  --help-description\n"
+		   "  --describe\n"
 		   "  --help|-h\n");
 	if (help_str)
 		fprintf(f, "%s\n", help_str);
@@ -671,6 +734,7 @@ static int common_init(int *argc, char **argv,
 	int c, option_index = 0, i, x;
 	static struct option long_options[] = {
 		{"list-subtests",     no_argument,       NULL, OPT_LIST_SUBTESTS},
+		{"describe",          optional_argument, NULL, OPT_DESCRIBE_SUBTESTS},
 		{"run-subtest",       required_argument, NULL, OPT_RUN_SUBTEST},
 		{"help-description",  no_argument,       NULL, OPT_DESCRIPTION},
 		{"debug",             optional_argument, NULL, OPT_DEBUG},
@@ -687,6 +751,7 @@ static int common_init(int *argc, char **argv,
 	int ret = 0;
 
 	common_init_env();
+	igt_list_init(&subgroup_descriptions);
 
 	command_str = argv[0];
 	if (strrchr(command_str, '/'))
@@ -777,6 +842,13 @@ static int common_init(int *argc, char **argv,
 			if (!run_single_subtest)
 				list_subtests = true;
 			break;
+		case OPT_DESCRIBE_SUBTESTS:
+			if (optarg)
+				run_single_subtest = strdup(optarg);
+			list_subtests = true;
+			describe_subtests = true;
+			print_test_description();
+			break;
 		case OPT_RUN_SUBTEST:
 			assert(optarg);
 			if (!list_subtests)
@@ -934,12 +1006,41 @@ void igt_simple_init_parse_opts(int *argc, char **argv,
 		    extra_opt_handler, handler_data);
 }
 
+static void _clear_current_description(void) {
+	__current_description[0] = '\0';
+}
+
+#define __INDENT "  "
+static void __igt_print_description(const char *subtest_name, const char *file, const int line)
+{
+	struct description_node *desc;
+	bool has_doc = false;
+
+	printf("SUB %s %s:%d:\n", subtest_name, file, line);
+
+	igt_list_for_each(desc, &subgroup_descriptions, link) {
+		print_line_wrapping(__INDENT, desc->desc);
+		printf("\n");
+		has_doc = true;
+	}
+
+	if (__current_description[0] != '\0') {
+		print_line_wrapping(__INDENT, __current_description);
+		printf("\n");
+		has_doc = true;
+	}
+
+	if (!has_doc)
+		printf("%sNO DOCUMENTATION!\n\n", __INDENT);
+}
+#undef __INDENT
+
 /*
  * Note: Testcases which use these helpers MUST NOT output anything to stdout
  * outside of places protected by igt_run_subtest checks - the piglit
  * runner adds every line to the subtest list.
  */
-bool __igt_run_subtest(const char *subtest_name)
+bool __igt_run_subtest(const char *subtest_name, const char *file, const int line)
 {
 	int i;
 
@@ -954,18 +1055,25 @@ bool __igt_run_subtest(const char *subtest_name)
 			igt_exit();
 		}
 
-	if (list_subtests) {
-		printf("%s\n", subtest_name);
-		return false;
-	}
-
 	if (run_single_subtest) {
-		if (uwildmat(subtest_name, run_single_subtest) == 0)
+		if (uwildmat(subtest_name, run_single_subtest) == 0) {
+			_clear_current_description();
 			return false;
-		else
+		} else {
 			run_single_subtest_found = true;
+		}
 	}
 
+	if (describe_subtests) {
+		__igt_print_description(subtest_name, file, line);
+		_clear_current_description();
+		return false;
+	} else if (list_subtests) {
+		printf("%s\n", subtest_name);
+		return false;
+	}
+
+
 	if (skip_subtests_henceforth) {
 		printf("%sSubtest %s: %s%s\n",
 		       (!__igt_plain_output) ? "\x1b[1m" : "", subtest_name,
@@ -1014,15 +1122,32 @@ bool igt_only_list_subtests(void)
 	return list_subtests;
 }
 
-void __igt_subtest_group_save(int *save)
+
+
+void __igt_subtest_group_save(int *save, int *desc)
 {
 	assert(test_with_subtests);
 
+	if (__current_description[0] != '\0') {
+		struct description_node *new = calloc(1, sizeof(*new));
+		memcpy(new->desc, __current_description, sizeof(__current_description));
+		igt_list_add_tail(&new->link, &subgroup_descriptions);
+		_clear_current_description();
+		*desc = true;
+	}
+
 	*save = skip_subtests_henceforth;
 }
 
-void __igt_subtest_group_restore(int save)
+void __igt_subtest_group_restore(int save, int desc)
 {
+	if (desc) {
+		struct description_node *last =
+			igt_list_last_entry(&subgroup_descriptions, last, link);
+		igt_list_del(&last->link);
+		free(last);
+	}
+
 	skip_subtests_henceforth = save;
 }
 
@@ -1229,6 +1354,34 @@ bool igt_can_fail(void)
 	return !test_with_subtests || in_fixture || in_subtest;
 }
 
+/**
+ * igt_describe_f:
+ * @fmt: format string containing description
+ * @...: argument used by the format string
+ *
+ * Attach a description to the following #igt_subtest or #igt_subtest_group
+ * block.
+ *
+ * Check #igt_describe for more details.
+ *
+ */
+void igt_describe_f(const char *fmt, ...)
+{
+	int ret;
+	va_list args;
+
+	if (!describe_subtests)
+		return;
+
+	va_start(args, fmt);
+
+	ret = vsnprintf(__current_description, sizeof(__current_description), fmt, args);
+
+	va_end(args);
+
+	assert(ret < sizeof(__current_description));
+}
+
 static bool running_under_gdb(void)
 {
 	char pathname[30], buf[1024];
@@ -1540,7 +1693,7 @@ void igt_exit(void)
 		g_key_file_free(igt_key_file);
 
 	if (run_single_subtest && !run_single_subtest_found) {
-		igt_warn("Unknown subtest: %s\n", run_single_subtest);
+		igt_critical("Unknown subtest: %s\n", run_single_subtest);
 		exit(IGT_EXIT_INVALID);
 	}
 
diff --git a/lib/igt_core.h b/lib/igt_core.h
index 88a95ec2..1b37f142 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -174,7 +174,7 @@ int igt_subtest_init_parse_opts(int *argc, char **argv,
 #define igt_subtest_init(argc, argv) \
 	igt_subtest_init_parse_opts(&argc, argv, NULL, NULL, NULL, NULL, NULL);
 
-bool __igt_run_subtest(const char *subtest_name);
+bool __igt_run_subtest(const char *subtest_name, const char *file, const int line);
 #define __igt_tokencat2(x, y) x ## y
 
 /**
@@ -198,14 +198,14 @@ bool __igt_run_subtest(const char *subtest_name);
  *
  * This is a simpler version of igt_subtest_f()
  */
-#define igt_subtest(name) for (; __igt_run_subtest((name)) && \
+#define igt_subtest(name) for (; __igt_run_subtest((name), __FILE__, __LINE__) && \
 				   (sigsetjmp(igt_subtest_jmpbuf, 1) == 0); \
 				   igt_success())
 #define __igt_subtest_f(tmp, format...) \
 	for (char tmp [256]; \
 	     snprintf( tmp , sizeof( tmp ), \
 		      format), \
-	     __igt_run_subtest( tmp ) && \
+	     __igt_run_subtest(tmp, __FILE__, __LINE__) && \
 	     (sigsetjmp(igt_subtest_jmpbuf, 1) == 0); \
 	     igt_success())
 
@@ -227,8 +227,8 @@ bool __igt_run_subtest(const char *subtest_name);
 const char *igt_subtest_name(void);
 bool igt_only_list_subtests(void);
 
-void __igt_subtest_group_save(int *);
-void __igt_subtest_group_restore(int);
+void __igt_subtest_group_save(int *, int *);
+void __igt_subtest_group_restore(int, int);
 /**
  * igt_subtest_group:
  *
@@ -245,11 +245,14 @@ void __igt_subtest_group_restore(int);
  * group will fail or skip. Subtest groups can be arbitrarily nested.
  */
 #define igt_subtest_group for (int igt_tokencat(__tmpint,__LINE__) = 0, \
-			       igt_tokencat(__save,__LINE__) = 0; \
+			       igt_tokencat(__save,__LINE__) = 0, \
+			       igt_tokencat(__desc,__LINE__) = 0; \
 			       igt_tokencat(__tmpint,__LINE__) < 1 && \
-			       (__igt_subtest_group_save(& igt_tokencat(__save,__LINE__) ), true); \
+			       (__igt_subtest_group_save(& igt_tokencat(__save,__LINE__), \
+							 & igt_tokencat(__desc,__LINE__) ), true); \
 			       igt_tokencat(__tmpint,__LINE__) ++, \
-			       __igt_subtest_group_restore(igt_tokencat(__save,__LINE__) ))
+			       __igt_subtest_group_restore(igt_tokencat(__save,__LINE__), \
+							   igt_tokencat(__desc,__LINE__)))
 
 /**
  * igt_main_args:
@@ -385,6 +388,124 @@ static inline void igt_ignore_warn(bool value)
 {
 }
 
+__attribute__((format(printf, 1, 2)))
+void igt_describe_f(const char *fmt, ...);
+
+/**
+ * igt_describe:
+ * @dsc: string containing description
+ *
+ * Attach a description to the following #igt_subtest or #igt_subtest_group
+ * block.
+ *
+ * The description should complement the test/subtest name and provide more
+ * context on what is being tested. It should explain the idea of the test and
+ * do not mention implementation details, so that it never goes out of date.
+ *
+ * DO:
+ *  * focus on the userspace's perspective
+ *  * try to capture the reason for the test's existence
+ *  * be brief
+ *
+ * DON'T:
+ *  * try to translate the code into English
+ *  * explain all the checks the test does
+ *  * delve on the implementation
+ *
+ * Good examples:
+ *  * "make sure that legacy cursor updates do not stall atomic commits"
+ *  * "check that atomic updates of many planes are indeed atomic and take
+ *     effect immediately after the commit"
+ *  * "make sure that the meta-data exposed by the kernel to the userspace
+ *     is correct and matches the used EDID"
+ *
+ * Bad examples:
+ *  * "spawn 10 threads, each pinning cpu core with a busy loop..."
+ *  * "randomly generate holes in a primary plane then try to cover each hole
+ *    with a plane and make sure that CRC matches, do 25 gazillion rounds of
+ *    that..."
+ *
+ *
+ * Resulting #igt_subtest documentation is a concatenation of its own
+ * description and all the parenting #igt_subtest_group descriptions, starting
+ * from the outermost one. Example:
+ *
+ * |[<!-- language="C" -->
+ * #include "igt.h"
+ *
+ * IGT_TEST_DESCRIPTION("Global description of the whole binary");
+ * igt_main
+ * {
+ * 	igt_describe("Desc of the subgroup with A and B");
+ * 	igt_subtest_group {
+ * 		igt_describe("Desc of the subtest A");
+ * 		igt_subtest("subtest-a") {
+ * 			...
+ * 		}
+ *
+ * 		igt_describe("Desc of the subtest B");
+ * 		igt_subtest("subtest-b") {
+ * 			...
+ * 		}
+ * 	}
+ *
+ * 	igt_describe("Desc of the subtest C");
+ * 	igt_subtest("subtest-c") {
+ * 		...
+ * 	}
+ * }
+ * ]|
+ *
+ * It's will accessible via --describe command line switch:
+ *
+ * |[
+ * $ test --describe
+ * Global description of the whole binary
+ *
+ * SUB subtest-a test.c:5:
+ *   Desc of the subgroup with A and B
+ *
+ *   Desc of the subtest A
+ *
+ * SUB subtest-b test.c:10:
+ *   Desc of the subgroup with A and B
+ *
+ *   Desc of the subtest B
+ *
+ * SUB subtest-c test.c:15:
+ *   Desc of the subtest C
+ * ]|
+ *
+ * Every single #igt_subtest does not have to be preceded with a #igt_describe
+ * as long as it has good-enough explanation provided on the #igt_subtest_group
+ * level.
+ *
+ * Example:
+ *
+ * |[<!-- language="C" -->
+ * #include "igt.h"
+ *
+ * igt_main
+ * {
+ * 	igt_describe("check xyz with different tilings");
+ * 	igt_subtest_group {
+ * 		// no need for extra description, group is enough and tiling is
+ * 		// obvious from the test name
+ * 		igt_subtest("foo-tiling-x") {
+ * 			...
+ * 		}
+ *
+ * 		igt_subtest("foo-tiling-y") {
+ * 			...
+ * 		}
+ * 	}
+ * }
+ * ]|
+ *
+ */
+#define igt_describe(dsc) \
+	igt_describe_f("%s", dsc)
+
 /**
  * igt_assert:
  * @expr: condition to test
diff --git a/lib/tests/igt_describe.c b/lib/tests/igt_describe.c
new file mode 100644
index 00000000..a9f857bb
--- /dev/null
+++ b/lib/tests/igt_describe.c
@@ -0,0 +1,266 @@
+/*
+ * Copyright © 2019 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 <sys/wait.h>
+#include <string.h>
+#include <stdlib.h>
+
+#include "drmtest.h"
+#include "igt_tests_common.h"
+
+IGT_TEST_DESCRIPTION("the top level description");
+static void fake_main(int argc, char **argv) {
+	igt_subtest_init(argc, argv);
+
+	igt_describe("Basic A");
+	igt_subtest("A")
+		;
+
+	igt_fixture
+		printf("should not be executed!\n");
+
+	igt_describe("Group with B, C & D");
+	igt_subtest_group {
+		igt_describe("Basic B");
+		igt_subtest("B")
+			;
+
+		if (!igt_only_list_subtests())
+			printf("should not be executed!\n");
+
+		igt_describe("Group with C & D");
+		igt_subtest_group {
+			igt_describe("Basic C");
+			igt_subtest("C")
+				printf("should not be executed!\n");
+
+			// NO DOC
+			igt_subtest("D")
+				;
+		}
+	}
+
+	// NO DOC
+	igt_subtest_group {
+		// NO DOC
+		igt_subtest("E")
+			;
+	}
+
+	// NO DOC
+	igt_subtest("F")
+		;
+
+	igt_describe("this description should be so long that it wraps itself nicely in the terminal "
+		     "this description should be so long that it wraps itself nicely in the terminal "
+		     "this description should be so long that it wraps itself nicely in the terminal "
+		     "this description should be so long that it wraps itself nicely in the terminal "
+		     "this description should be so long that it wraps itself nicely in the terminal "
+		     "this description should be so long that it wraps itself nicely in the terminal");
+	igt_subtest("G")
+		;
+
+	igt_describe("verylongwordthatshoudlbeprintedeventhoughitspastthewrppinglimit"
+		     "verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit "
+		     "verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit"
+		     "verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit");
+	igt_subtest("F")
+		;
+
+	igt_exit();
+}
+
+static const char DESCRIBE_ALL_OUTPUT[] = \
+	"the top level description\n"
+	"\n"
+	"SUB A ../lib/tests/igt_describe.c:36:\n"
+	"  Basic A\n"
+	"\n"
+	"SUB B ../lib/tests/igt_describe.c:45:\n"
+	"  Group with B, C & D\n"
+	"\n"
+	"  Basic B\n"
+	"\n"
+	"SUB C ../lib/tests/igt_describe.c:54:\n"
+	"  Group with B, C & D\n"
+	"\n"
+	"  Group with C & D\n"
+	"\n"
+	"  Basic C\n"
+	"\n"
+	"SUB D ../lib/tests/igt_describe.c:58:\n"
+	"  Group with B, C & D\n"
+	"\n"
+	"  Group with C & D\n"
+	"\n"
+	"SUB E ../lib/tests/igt_describe.c:66:\n"
+	"  NO DOCUMENTATION!\n"
+	"\n"
+	"SUB F ../lib/tests/igt_describe.c:71:\n"
+	"  NO DOCUMENTATION!\n"
+	"\n"
+	"SUB G ../lib/tests/igt_describe.c:80:\n"
+	"  this description should be so long that it wraps itself nicely in the terminal this\n"
+	"  description should be so long that it wraps itself nicely in the terminal this description\n"
+	"  should be so long that it wraps itself nicely in the terminal this description should be so\n"
+	"  long that it wraps itself nicely in the terminal this description should be so long that it\n"
+	"  wraps itself nicely in the terminal this description should be so long that it wraps itself\n"
+	"  nicely in the terminal\n"
+	"\n"
+	"SUB F ../lib/tests/igt_describe.c:87:\n"
+	"  verylongwordthatshoudlbeprintedeventhoughitspastthewrppinglimitverylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit\n"
+	"  verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimitverylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit\n\n";
+
+static const char JUST_C_OUTPUT[] = \
+	"the top level description\n"
+	"\n"
+	"SUB C ../lib/tests/igt_describe.c:54:\n"
+	"  Group with B, C & D\n"
+	"\n"
+	"  Group with C & D\n"
+	"\n"
+	"  Basic C\n"
+	"\n";
+
+static void assert_pipe_empty(int fd)
+{
+	char buf[5];
+	internal_assert(0 == read(fd, buf, sizeof(buf)));
+}
+
+static void read_whole_pipe(int fd, char *buf, size_t buflen)
+{
+	ssize_t readlen;
+	off_t offset;
+
+	offset = 0;
+	while ((readlen = read(fd, buf+offset, buflen-offset))) {
+		internal_assert(readlen != -1);
+		offset += readlen;
+	}
+}
+
+static pid_t do_fork(int argc, char **argv, int *out, int *err)
+{
+	int outfd[2], errfd[2];
+	pid_t pid;
+
+	internal_assert(pipe(outfd) != -1);
+	internal_assert(pipe(errfd) != -1);
+
+	pid = fork();
+	internal_assert(pid != -1);
+
+	if (pid == 0) {
+		while ((dup2(outfd[1], STDOUT_FILENO) == -1) && (errno == EINTR)) {}
+		while ((dup2(errfd[1], STDERR_FILENO) == -1) && (errno == EINTR)) {}
+
+		fake_main(argc, argv);
+
+		exit(-1);
+	} else {
+		/* close the writing ends */
+		close(outfd[1]);
+		close(errfd[1]);
+
+		*out = outfd[0];
+		*err = errfd[0];
+
+		return pid;
+	}
+}
+
+static int _wait(pid_t pid, int *status) {
+	int ret;
+
+	do {
+		ret = waitpid(pid, status, 0);
+	} while (ret == -1 && errno == EINTR);
+
+	return ret;
+}
+
+int main(int argc, char **argv)
+{
+	char prog[] = "igt_describe";
+	int status;
+	int outfd, errfd;
+
+	/* describe all subtest */ {
+		static char out[4096];
+		char arg[] = "--describe";
+		char *fake_argv[] = {prog, arg};
+		int fake_argc = ARRAY_SIZE(fake_argv);
+
+		pid_t pid = do_fork(fake_argc, fake_argv, &outfd, &errfd);
+
+		read_whole_pipe(outfd, out, sizeof(out));
+		assert_pipe_empty(errfd);
+
+		internal_assert(_wait(pid, &status) != -1);
+		internal_assert(WEXITSTATUS(status) == IGT_EXIT_SUCCESS);
+		internal_assert(0 == strcmp(DESCRIBE_ALL_OUTPUT, out));
+
+		close(outfd);
+		close(errfd);
+	}
+
+	/* describe C using a pattern */ {
+		static char out[4096];
+		char arg[] = "--describe=C";
+		char *fake_argv[] = {prog, arg};
+		int fake_argc = ARRAY_SIZE(fake_argv);
+
+		pid_t pid = do_fork(fake_argc, fake_argv, &outfd, &errfd);
+
+		read_whole_pipe(outfd, out, sizeof(out));
+		assert_pipe_empty(errfd);
+
+		internal_assert(_wait(pid, &status) != -1);
+		internal_assert(WEXITSTATUS(status) == IGT_EXIT_SUCCESS);
+		internal_assert(0 == strcmp(JUST_C_OUTPUT, out));
+
+		close(outfd);
+		close(errfd);
+	}
+
+	/* fail describing with a bad pattern */ {
+		static char err[4096];
+		char arg[] = "--describe=Z";
+		char *fake_argv[] = {prog, arg};
+		int fake_argc = ARRAY_SIZE(fake_argv);
+
+		pid_t pid = do_fork(fake_argc, fake_argv, &outfd, &errfd);
+
+		read_whole_pipe(errfd, err, sizeof(err));
+
+		internal_assert(_wait(pid, &status) != -1);
+		internal_assert(WEXITSTATUS(status) == IGT_EXIT_INVALID);
+		internal_assert(strstr(err, "Unknown subtest: Z"));
+
+		close(outfd);
+		close(errfd);
+	}
+
+	return 0;
+}
diff --git a/lib/tests/meson.build b/lib/tests/meson.build
index b930ee6e..4d907e34 100644
--- a/lib/tests/meson.build
+++ b/lib/tests/meson.build
@@ -3,6 +3,7 @@ lib_tests = [
 	'igt_can_fail',
 	'igt_can_fail_simple',
 	'igt_conflicting_args',
+	'igt_describe',
 	'igt_edid',
 	'igt_exit_handler',
 	'igt_fork',
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [igt-dev] [PATCH i-g-t 2/5] tests/kms_hdmi_inject: Provide igt_descriptions
  2019-07-01 12:21 [igt-dev] [PATCH i-g-t 1/5] lib/igt_core: Add support for subtest descriptions Arkadiusz Hiler
@ 2019-07-01 12:21 ` Arkadiusz Hiler
  2019-07-01 12:21 ` [igt-dev] [PATCH i-g-t 3/5] tests/kms_plane_multiple: Describe the test Arkadiusz Hiler
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Arkadiusz Hiler @ 2019-07-01 12:21 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala

This test is quite simple which makes it perfect for the first real
world example of igt_describe usage.

v2: keep the original comment (Simon)

Cc: Simon Ser <simon.ser@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Acked-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Simon Ser <simon.ser@intel.com>
---
 tests/kms_hdmi_inject.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tests/kms_hdmi_inject.c b/tests/kms_hdmi_inject.c
index 8c0d1333..c518c5d8 100644
--- a/tests/kms_hdmi_inject.c
+++ b/tests/kms_hdmi_inject.c
@@ -33,7 +33,10 @@
 #define HDISPLAY_4K	3840
 #define VDISPLAY_4K	2160
 
-IGT_TEST_DESCRIPTION("Tests 4K and audio HDMI injection.");
+IGT_TEST_DESCRIPTION("Test that in-kernel EDID parsing is producing "
+		     "expected results by forcing a disconnected HDMI "
+		     "connector with a known EDID and checking that the "
+		     "metadata exposed to user space matches.");
 
 /**
  * This collection of tests performs EDID and status injection tests. Injection
@@ -210,9 +213,13 @@ igt_main
 		igt_require(connector);
 	}
 
+	igt_describe("Make sure that 4K modes exposed by DRM match the "
+                     "forced EDID and modesetting using it succeed.");
 	igt_subtest("inject-4k")
 		hdmi_inject_4k(drm_fd, connector);
 
+	igt_describe("Make sure that audio information exposed by ALSA "
+		     "match the forced EDID.");
 	igt_subtest("inject-audio")
 		hdmi_inject_audio(drm_fd, connector);
 
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [igt-dev] [PATCH i-g-t 3/5] tests/kms_plane_multiple: Describe the test
  2019-07-01 12:21 [igt-dev] [PATCH i-g-t 1/5] lib/igt_core: Add support for subtest descriptions Arkadiusz Hiler
  2019-07-01 12:21 ` [igt-dev] [PATCH i-g-t 2/5] tests/kms_hdmi_inject: Provide igt_descriptions Arkadiusz Hiler
@ 2019-07-01 12:21 ` Arkadiusz Hiler
  2019-07-01 12:21 ` [igt-dev] [PATCH i-g-t 4/5] CONTRIBUTING: Rework a bit and update Arkadiusz Hiler
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Arkadiusz Hiler @ 2019-07-01 12:21 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala

In this test all the subtest are doing the same exact thing but with
different tiling / on a different pipe. Tiling/pipe is in the test name
so no need to elaborate on them.

We can have just one igt_describe() on top of the group to describe all
the subtests in one go, no copy-and-paste necessary.

The description is short and explains the spirit of the test (verifying
atomicity of plane updates) without delving into the implementation
details (holes, colors, etc.)

Other changes:
 * The function used for grabbing reference CRC is now under a more
   descriptive name.
 * Comment explaining test implementation in more detail is moved to the
   top of the function actually implementing those steps.
 * Minor formatting touch ups.

Cc: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Acked-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Mika Kahola <mika.kahola@intel.com>
---
 tests/kms_plane_multiple.c | 39 ++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/tests/kms_plane_multiple.c b/tests/kms_plane_multiple.c
index 0d3ba4ff..81ed45dd 100644
--- a/tests/kms_plane_multiple.c
+++ b/tests/kms_plane_multiple.c
@@ -30,7 +30,7 @@
 #include <string.h>
 #include <time.h>
 
-IGT_TEST_DESCRIPTION("Test atomic mode setting with multiple planes ");
+IGT_TEST_DESCRIPTION("Test atomic mode setting with multiple planes.");
 
 #define SIZE_PLANE      256
 #define SIZE_CURSOR     128
@@ -96,7 +96,7 @@ static void test_fini(data_t *data, igt_output_t *output, int n_planes)
 }
 
 static void
-test_grab_crc(data_t *data, igt_output_t *output, enum pipe pipe,
+get_reference_crc(data_t *data, igt_output_t *output, enum pipe pipe,
 	      color_t *color, uint64_t tiling)
 {
 	drmModeModeInfo *mode;
@@ -125,17 +125,6 @@ test_grab_crc(data_t *data, igt_output_t *output, enum pipe pipe,
 	igt_pipe_crc_get_single(data->pipe_crc, &data->ref_crc);
 }
 
-/*
- * Multiple plane position test.
- *   - We start by grabbing a reference CRC of a full blue fb being scanned
- *     out on the primary plane
- *   - Then we scannout number of planes:
- *      * the primary plane uses a blue fb with a black rectangle hole
- *      * planes, on top of the primary plane, with a blue fb that is set-up
- *        to cover the black rectangles of the primary plane fb
- *     The resulting CRC should be identical to the reference CRC
- */
-
 static void
 create_fb_for_mode_position(data_t *data, igt_output_t *output, drmModeModeInfo *mode,
 			    color_t *color, int *rect_x, int *rect_y,
@@ -281,6 +270,17 @@ prepare_planes(data_t *data, enum pipe pipe_id, color_t *color,
 	free((void*)suffle);
 }
 
+/*
+ * Multiple plane position test.
+ *   - We start by grabbing a reference CRC of a full blue fb being scanned
+ *     out on the primary plane
+ *   - Then we scannout number of planes:
+ *      * the primary plane uses a blue fb with a black rectangle holes
+ *      * planes, on top of the primary plane, with a blue fb that is set-up
+ *        to cover the black rectangles of the primary plane
+ *     The resulting CRC should be identical to the reference CRC
+ */
+
 static void
 test_plane_position_with_output(data_t *data, enum pipe pipe,
 				igt_output_t *output, int n_planes,
@@ -306,11 +306,9 @@ test_plane_position_with_output(data_t *data, enum pipe pipe,
 
 	test_init(data, pipe, n_planes);
 
-	test_grab_crc(data, output, pipe, &blue, tiling);
+	get_reference_crc(data, output, pipe, &blue, tiling);
 
-	/*
-	 * Find out how many planes are allowed simultaneously
-	 */
+	/* Find out how many planes are allowed simultaneously */
 	do {
 		c++;
 		prepare_planes(data, pipe, &blue, tiling, c, output);
@@ -323,7 +321,7 @@ test_plane_position_with_output(data_t *data, enum pipe pipe,
 			igt_remove_fb(data->drm_fd, &data->fb[x]);
 	} while (!err && c < n_planes);
 
-	if(err)
+	if (err)
 		c--;
 
 	igt_info("Testing connector %s using pipe %s with %d planes %s with seed %d\n",
@@ -332,6 +330,7 @@ test_plane_position_with_output(data_t *data, enum pipe pipe,
 
 	i = 0;
 	while (i < iterations || loop_forever) {
+		/* randomize planes and set up the holes */
 		prepare_planes(data, pipe, &blue, tiling, c, output);
 
 		igt_display_commit2(&data->display, COMMIT_ATOMIC);
@@ -441,6 +440,10 @@ igt_main_args("", long_options, help_str, opt_handler, NULL)
 	}
 
 	for_each_pipe_static(pipe) {
+		igt_describe("Check that the kernel handles atomic updates of "
+			     "multiple planes correctly by changing their "
+			     "geometry and making sure the changes are "
+			     "reflected immediately after each commit.");
 		igt_subtest_group
 			run_tests_for_pipe(&data, pipe);
 	}
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [igt-dev] [PATCH i-g-t 4/5] CONTRIBUTING: Rework a bit and update
  2019-07-01 12:21 [igt-dev] [PATCH i-g-t 1/5] lib/igt_core: Add support for subtest descriptions Arkadiusz Hiler
  2019-07-01 12:21 ` [igt-dev] [PATCH i-g-t 2/5] tests/kms_hdmi_inject: Provide igt_descriptions Arkadiusz Hiler
  2019-07-01 12:21 ` [igt-dev] [PATCH i-g-t 3/5] tests/kms_plane_multiple: Describe the test Arkadiusz Hiler
@ 2019-07-01 12:21 ` Arkadiusz Hiler
  2019-07-01 13:02   ` Ser, Simon
  2019-07-01 12:21 ` [igt-dev] [PATCH i-g-t 5/5] docs: Embed subtest descriptions in the documentation Arkadiusz Hiler
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: Arkadiusz Hiler @ 2019-07-01 12:21 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala

I have split the main body of the CONTRIBUTING file to 3 sections:
 * the short welcome message
 * the code - styling, suggestions
 * sending patches - licensing informations, mailing lists, reviews, etc.

Changes for the code section contents:
 * link to the kernel coding style docs
 * be more clear on subtest naming
 * mention igt_describe()

v2: Add link to the online documentation for igt-describe. It will start
    working once the series is merged. (Simon)

Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Simon Ser <simon.ser@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Acked-by: Petri Latvala <petri.latvala@intel.com>
---
 CONTRIBUTING.md | 48 ++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index d3a3d099..6d1294ad 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -1,9 +1,43 @@
+CONTRIBUTING
+============
+
 Patches to igt-gpu-tools are very much welcome, we really want this to be the
 universal set of low-level tools and testcases for kernel graphics drivers
 on Linux and similar platforms. So please bring on porting patches, bugfixes,
 improvements for documentation and new tools and testcases.
 
-A short list of contribution guidelines:
+
+The Code
+--------
+
+- The code should follow kernel coding style:
+  https://www.kernel.org/doc/html/latest/process/coding-style.html
+
+- Testcases (subtests) have to use minus signs (-) as a word separator.
+  The generated documentation contains glossary of commonly used terms.
+
+- All new test have to be described using `igt_describe()` family of
+  functions. The description should contain the spirit of the test (what is
+  the general idea behind the test) and *not* the letter (C to English
+  translation of the test). Refer to [`igt_describe()`
+  documentation][igt-describe] for more details.
+
+- The generated documentation contains explanation of magic control blocks like
+  `igt_subtest` and `igt_fixture`. Please make sure that you understand their
+  roles and limitation before using/altering them.
+
+- Also please make full use of all the helpers and convenience macros
+  provided by the igt library. The semantic patch lib/igt.cocci can help with
+  more automatic conversions.
+
+[igt-describe]: https://drm.pages.freedesktop.org/igt-gpu-tools/igt-gpu-tools-Core.html#igt-describe
+
+
+Sending Patches
+---------------
+
+- igt-gpu-tools is MIT licensed and we require contributions to follow the
+  developer's certificate of origin: http://developercertificate.org/
 
 - Please submit patches formatted with git send-email/git format-patch or
   equivalent to:
@@ -23,14 +57,6 @@ A short list of contribution guidelines:
 
   on its first invocation.
 
-- igt-gpu-tools is MIT licensed and we require contributions to follow the
-  developer's certificate of origin: http://developercertificate.org/
-
-- When submitting new testcases please follow the naming conventions documented
-  in the generated documentation. Also please make full use of all the helpers
-  and convenience macros provided by the igt library. The semantic patch
-  lib/igt.cocci can help with the more automatic conversions.
-
 - Patches need to be reviewed on the mailing list. Exceptions only apply for
   testcases and tooling for drivers with just a single contributor (e.g. vc4).
   In this case patches must still be submitted to the mailing list first.
@@ -46,7 +72,8 @@ A short list of contribution guidelines:
 - Changes to the testcases are automatically tested. Take the results into
   account before merging.
 
-Commit rights
+
+Commit Rights
 -------------
 
 Commit rights will be granted to anyone who requests them and fulfills the
@@ -80,6 +107,7 @@ come back to the project.
 Maintainers and committers should encourage contributors to request commit
 rights, especially junior contributors tend to underestimate their skills.
 
+
 Code of Conduct
 ---------------
 
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [igt-dev] [PATCH i-g-t 5/5] docs: Embed subtest descriptions in the documentation
  2019-07-01 12:21 [igt-dev] [PATCH i-g-t 1/5] lib/igt_core: Add support for subtest descriptions Arkadiusz Hiler
                   ` (2 preceding siblings ...)
  2019-07-01 12:21 ` [igt-dev] [PATCH i-g-t 4/5] CONTRIBUTING: Rework a bit and update Arkadiusz Hiler
@ 2019-07-01 12:21 ` Arkadiusz Hiler
  2019-07-03  7:18   ` Ser, Simon
  2019-07-01 13:01 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/5] lib/igt_core: Add support for subtest descriptions Patchwork
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: Arkadiusz Hiler @ 2019-07-01 12:21 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala, Daniel Vetter

This rewrites generate_description_xml in Python, so that we generate
properly escaped XML. The switch also makes the code more manageable.

Changes in the generated docbook:

1. subtests are not simply listed anymore, they are now another (sub)section

2. subtests are now linkable,
   e.g. docs/igt-kms-tests.html#kms_hdmi_inject@inject-4k

3. subtest's section now includes output of --describe

Python is required already by gtk-doc and we are not using anything
other than the standard library.

v2: keep the part of the subtest name after the last match (Simon)
    explicitly require python3 (Petri)
v3: make sure that the tail of the subtest name after the last keyword
    match is included (Simon)

Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Simon Ser <simon.ser@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Acked-by: Petri Latvala <petri.latvala@intel.com>
---
 .../igt-gpu-tools/generate_description_xml.py | 114 ++++++++++++++++++
 .../igt-gpu-tools/generate_description_xml.sh |  46 -------
 docs/reference/igt-gpu-tools/meson.build      |   2 +-
 meson.build                                   |   3 +-
 4 files changed, 117 insertions(+), 48 deletions(-)
 create mode 100755 docs/reference/igt-gpu-tools/generate_description_xml.py
 delete mode 100644 docs/reference/igt-gpu-tools/generate_description_xml.sh

diff --git a/docs/reference/igt-gpu-tools/generate_description_xml.py b/docs/reference/igt-gpu-tools/generate_description_xml.py
new file mode 100755
index 00000000..8bb0989a
--- /dev/null
+++ b/docs/reference/igt-gpu-tools/generate_description_xml.py
@@ -0,0 +1,114 @@
+#!/usr/bin/env python3
+
+import re
+import sys
+import os.path
+import subprocess
+import xml.etree.cElementTree as ET
+
+from collections import namedtuple
+
+Subtest = namedtuple("Subtest", "name description")
+KEYWORDS=re.compile(r'\b(invalid|hang|swap|thrash|crc|tiled|tiling|rte|ctx|render|blt|bsd|vebox|exec|rpm)\b')
+
+
+def get_testlist(path):
+    "read binaries' names from test-list.txt"
+    with open(path, 'r') as f:
+        assert(f.readline() == "TESTLIST\n")
+        tests = f.readline().strip().split(" ")
+        assert(f.readline() == "END TESTLIST\n")
+
+    return tests
+
+
+def keywordize(root, text, keywords):
+    "set text for root element and wrap KEYWORDS in a <acronym>"
+    matches = list(keywords.finditer(text))
+
+    if not matches:
+        root.text = text
+        return
+
+    pos = 0
+    last_element = None
+    root.text = ""
+
+    for match in matches:
+        if match.start() > pos:
+            to_append = text[pos:match.start()]
+
+            if last_element == None:
+                root.text += to_append
+            else:
+                last_element.tail += to_append
+
+        last_element = ET.SubElement(root, "acronym")
+        last_element.tail = ""
+        last_element.text=match.group()
+        pos = match.end()
+
+    last_element.tail = text[pos:]
+
+
+def get_subtests(testdir, test):
+    "execute test and get subtests with their descriptions via --describe"
+    output = []
+    full_test_path = os.path.join(testdir, test)
+    proc = subprocess.run([full_test_path, "--describe"], stdout=subprocess.PIPE)
+    description = ""
+    current_subtest = None
+
+    for line in proc.stdout.decode().splitlines():
+        if line.startswith("SUB "):
+            output += [Subtest(current_subtest, description)]
+            description = ""
+            current_subtest = line.split(' ')[1]
+        else:
+            description += line
+
+    output += [Subtest(current_subtest, description)]
+
+    return output
+
+def main():
+    output_file   = sys.argv[1]
+    test_filter   = re.compile(sys.argv[2])
+    testlist_file = sys.argv[3]
+    testdir       = os.path.abspath(os.path.dirname(testlist_file))
+
+    root = ET.Element("refsect1")
+    ET.SubElement(root, "title").text = "Description"
+
+    tests = get_testlist(testlist_file)
+
+    for test in tests:
+        if not test_filter.match(test):
+            continue
+
+        test_section = ET.SubElement(root, "refsect2", id=test)
+        test_title = ET.SubElement(test_section, "title")
+        keywordize(test_title, test, KEYWORDS)
+
+        subtests = get_subtests(testdir, test)
+
+        # we have description with no subtest name, add it at the top level
+        if subtests and not subtests[0].name:
+            ET.SubElement(test_section, "para").text = subtests[0].description
+
+        if len(subtests) > 100:
+            ET.SubElement(test_section, "para").text = "More than 100 subtests, skipping listing"
+            continue
+
+        for name, description in subtests:
+            if not name:
+                continue
+
+            subtest_section = ET.SubElement(test_section, "refsect3", id="{}@{}".format(test, name))
+            subtest_title = ET.SubElement(subtest_section, "title")
+            keywordize(subtest_title, name, KEYWORDS)
+            ET.SubElement(subtest_section, "para").text = description
+
+    ET.ElementTree(root).write(output_file)
+
+main()
diff --git a/docs/reference/igt-gpu-tools/generate_description_xml.sh b/docs/reference/igt-gpu-tools/generate_description_xml.sh
deleted file mode 100644
index 705a7bf3..00000000
--- a/docs/reference/igt-gpu-tools/generate_description_xml.sh
+++ /dev/null
@@ -1,46 +0,0 @@
-#!/bin/sh
-
-output=$1
-filter=$2
-testlist=$3
-testdir=$(dirname $testlist)
-
-KEYWORDS="(invalid|hang|swap|thrash|crc|tiled|tiling|rte|ctx|render|blt|bsd|vebox|exec|rpm)"
-
-echo "<?xml version=\"1.0\"?>" > $output
-echo "<!DOCTYPE refsect1 PUBLIC \"-//OASIS//DTD DocBook XML V4.3//EN\"" >> $output
-echo "               \"http://www.oasis-open.org/docbook/xml/4.3/docbookx.dtd\"" >> $output
-echo "[" >> $output
-echo "  <!ENTITY % local.common.attrib \"xmlns:xi  CDATA  #FIXED 'http://www.w3.org/2003/XInclude'\">" >> $output
-echo "  <!ENTITY version SYSTEM \"version.xml\">" >> $output
-echo "]>" >> $output
-echo "<refsect1>" >> $output
-echo "<title>Description</title>" >> $output
-for test in `cat $testlist | tr ' ' '\n' | grep "^$filter" | sort`; do
-	echo "<refsect2 id=\"$test\"><title>" >> $output;
-	echo "$test" | perl -pe "s/(?<=_)$KEYWORDS(?=(_|\\W))/<acronym>\\1<\\/acronym>/g" >> $output;
-	echo "</title><para><![CDATA[" >> $output;
-	testprog=$testdir/$test;
-	 ./$testprog --help-description >> $output;
-	echo "]]></para>" >> $output;
-	if ./$testprog --list-subtests > /dev/null ; then
-		echo "<refsect3><title>Subtests</title>" >> $output;
-		subtest_list=`./$testprog --list-subtests`;
-		subtest_count=`echo $subtest_list | wc -w`;
-		if [ $subtest_count -gt 100 ]; then
-			echo "<para>This test has over 100 subtests. " >> $output;
-			echo "Run <command>$test</command> <option>--list-subtests</option> to list them.</para>" >> $output;
-		else
-			echo "<simplelist>" >> $output;
-			for subtest in $subtest_list; do
-				echo "<member>" >> $output;
-				echo "$subtest" | perl -pe "s/\\b$KEYWORDS\\b/<acronym>\\1<\\/acronym>/g" >> $output;
-				echo "</member>" >> $output;
-			done;
-			echo "</simplelist>" >> $output;
-		fi;
-		echo "</refsect3>" >> $output;
-	fi;
-	echo "</refsect2>" >> $output;
-done;
-echo "</refsect1>" >> $output
diff --git a/docs/reference/igt-gpu-tools/meson.build b/docs/reference/igt-gpu-tools/meson.build
index 4d177e49..e2bdc495 100644
--- a/docs/reference/igt-gpu-tools/meson.build
+++ b/docs/reference/igt-gpu-tools/meson.build
@@ -45,7 +45,7 @@ test_groups = [
 	'vgem',
 ]
 
-gen_description = find_program('generate_description_xml.sh')
+gen_description = find_program('generate_description_xml.py')
 gen_programs = find_program('generate_programs_xml.sh')
 
 generated_docs = []
diff --git a/meson.build b/meson.build
index f0cb2543..0629d441 100644
--- a/meson.build
+++ b/meson.build
@@ -303,7 +303,8 @@ subdir('overlay')
 subdir('man')
 
 gtk_doc = dependency('gtk-doc', required : build_docs)
-if build_tests and gtk_doc.found()
+python3 = find_program('python3', required : build_docs)
+if build_tests and gtk_doc.found() and python3.found()
 	subdir('docs')
 elif build_docs.enabled()
 	error('Documentation requires building tests')
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/5] lib/igt_core: Add support for subtest descriptions
  2019-07-01 12:21 [igt-dev] [PATCH i-g-t 1/5] lib/igt_core: Add support for subtest descriptions Arkadiusz Hiler
                   ` (3 preceding siblings ...)
  2019-07-01 12:21 ` [igt-dev] [PATCH i-g-t 5/5] docs: Embed subtest descriptions in the documentation Arkadiusz Hiler
@ 2019-07-01 13:01 ` Patchwork
  2019-07-02 16:41 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2019-07-01 13:01 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,1/5] lib/igt_core: Add support for subtest descriptions
URL   : https://patchwork.freedesktop.org/series/63037/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6390 -> IGTPW_3217
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/63037/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_3217 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_basic@bad-close:
    - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/fi-icl-u3/igt@gem_basic@bad-close.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/fi-icl-u3/igt@gem_basic@bad-close.html

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       [PASS][3] -> [INCOMPLETE][4] ([fdo#107718])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html

  * igt@gem_ringfill@basic-default-interruptible:
    - fi-icl-dsi:         [PASS][5] -> [DMESG-WARN][6] ([fdo#106107])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/fi-icl-dsi/igt@gem_ringfill@basic-default-interruptible.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/fi-icl-dsi/igt@gem_ringfill@basic-default-interruptible.html

  
#### Possible fixes ####

  * igt@gem_mmap_gtt@basic-copy:
    - fi-icl-u3:          [DMESG-WARN][7] ([fdo#107724]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/fi-icl-u3/igt@gem_mmap_gtt@basic-copy.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/fi-icl-u3/igt@gem_mmap_gtt@basic-copy.html

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [FAIL][9] ([fdo#108511]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_blt:
    - fi-skl-iommu:       [INCOMPLETE][11] ([fdo#108602]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/fi-skl-iommu/igt@i915_selftest@live_blt.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/fi-skl-iommu/igt@i915_selftest@live_blt.html

  * igt@i915_selftest@live_contexts:
    - fi-icl-dsi:         [INCOMPLETE][13] ([fdo#107713] / [fdo#108569]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/fi-icl-dsi/igt@i915_selftest@live_contexts.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/fi-icl-dsi/igt@i915_selftest@live_contexts.html

  * igt@kms_busy@basic-flip-c:
    - fi-skl-6770hq:      [SKIP][15] ([fdo#109271] / [fdo#109278]) -> [PASS][16] +2 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/fi-skl-6770hq/igt@kms_busy@basic-flip-c.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/fi-skl-6770hq/igt@kms_busy@basic-flip-c.html

  * igt@kms_flip@basic-flip-vs-dpms:
    - fi-skl-6770hq:      [SKIP][17] ([fdo#109271]) -> [PASS][18] +23 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-dpms.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/fi-skl-6770hq/igt@kms_flip@basic-flip-vs-dpms.html

  
  [fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278


Participating hosts (52 -> 44)
------------------------------

  Additional (1): fi-kbl-7567u 
  Missing    (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-gdg-551 fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * IGT: IGT_5075 -> IGTPW_3217

  CI_DRM_6390: 4c6c23fdf450ab43bb4046ac1fb1439ebf176564 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3217: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/
  IGT_5075: 03779dd3de8a57544f124d9952a6d2b3e34e34ca @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 4/5] CONTRIBUTING: Rework a bit and update
  2019-07-01 12:21 ` [igt-dev] [PATCH i-g-t 4/5] CONTRIBUTING: Rework a bit and update Arkadiusz Hiler
@ 2019-07-01 13:02   ` Ser, Simon
  0 siblings, 0 replies; 17+ messages in thread
From: Ser, Simon @ 2019-07-01 13:02 UTC (permalink / raw)
  To: Hiler, Arkadiusz, igt-dev; +Cc: Latvala, Petri

On Mon, 2019-07-01 at 15:21 +0300, Arkadiusz Hiler wrote:
> I have split the main body of the CONTRIBUTING file to 3 sections:
>  * the short welcome message
>  * the code - styling, suggestions
>  * sending patches - licensing informations, mailing lists, reviews, etc.
> 
> Changes for the code section contents:
>  * link to the kernel coding style docs
>  * be more clear on subtest naming
>  * mention igt_describe()
> 
> v2: Add link to the online documentation for igt-describe. It will start
>     working once the series is merged. (Simon)
> 
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Simon Ser <simon.ser@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Acked-by: Petri Latvala <petri.latvala@intel.com>

Reviewed-by: Simon Ser <simon.ser@intel.com>

> ---
>  CONTRIBUTING.md | 48 ++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 38 insertions(+), 10 deletions(-)
> 
> diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
> index d3a3d099..6d1294ad 100644
> --- a/CONTRIBUTING.md
> +++ b/CONTRIBUTING.md
> @@ -1,9 +1,43 @@
> +CONTRIBUTING
> +============
> +
>  Patches to igt-gpu-tools are very much welcome, we really want this to be the
>  universal set of low-level tools and testcases for kernel graphics drivers
>  on Linux and similar platforms. So please bring on porting patches, bugfixes,
>  improvements for documentation and new tools and testcases.
>  
> -A short list of contribution guidelines:
> +
> +The Code
> +--------
> +
> +- The code should follow kernel coding style:
> +  https://www.kernel.org/doc/html/latest/process/coding-style.html
> +
> +- Testcases (subtests) have to use minus signs (-) as a word separator.
> +  The generated documentation contains glossary of commonly used terms.
> +
> +- All new test have to be described using `igt_describe()` family of
> +  functions. The description should contain the spirit of the test (what is
> +  the general idea behind the test) and *not* the letter (C to English
> +  translation of the test). Refer to [`igt_describe()`
> +  documentation][igt-describe] for more details.
> +
> +- The generated documentation contains explanation of magic control blocks like
> +  `igt_subtest` and `igt_fixture`. Please make sure that you understand their
> +  roles and limitation before using/altering them.
> +
> +- Also please make full use of all the helpers and convenience macros
> +  provided by the igt library. The semantic patch lib/igt.cocci can help with
> +  more automatic conversions.
> +
> +[igt-describe]: https://drm.pages.freedesktop.org/igt-gpu-tools/igt-gpu-tools-Core.html#igt-describe
> +
> +
> +Sending Patches
> +---------------
> +
> +- igt-gpu-tools is MIT licensed and we require contributions to follow the
> +  developer's certificate of origin: http://developercertificate.org/
>  
>  - Please submit patches formatted with git send-email/git format-patch or
>    equivalent to:
> @@ -23,14 +57,6 @@ A short list of contribution guidelines:
>  
>    on its first invocation.
>  
> -- igt-gpu-tools is MIT licensed and we require contributions to follow the
> -  developer's certificate of origin: http://developercertificate.org/
> -
> -- When submitting new testcases please follow the naming conventions documented
> -  in the generated documentation. Also please make full use of all the helpers
> -  and convenience macros provided by the igt library. The semantic patch
> -  lib/igt.cocci can help with the more automatic conversions.
> -
>  - Patches need to be reviewed on the mailing list. Exceptions only apply for
>    testcases and tooling for drivers with just a single contributor (e.g. vc4).
>    In this case patches must still be submitted to the mailing list first.
> @@ -46,7 +72,8 @@ A short list of contribution guidelines:
>  - Changes to the testcases are automatically tested. Take the results into
>    account before merging.
>  
> -Commit rights
> +
> +Commit Rights
>  -------------
>  
>  Commit rights will be granted to anyone who requests them and fulfills the
> @@ -80,6 +107,7 @@ come back to the project.
>  Maintainers and committers should encourage contributors to request commit
>  rights, especially junior contributors tend to underestimate their skills.
>  
> +
>  Code of Conduct
>  ---------------
>  
> -- 
> 2.21.0
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,1/5] lib/igt_core: Add support for subtest descriptions
  2019-07-01 12:21 [igt-dev] [PATCH i-g-t 1/5] lib/igt_core: Add support for subtest descriptions Arkadiusz Hiler
                   ` (4 preceding siblings ...)
  2019-07-01 13:01 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/5] lib/igt_core: Add support for subtest descriptions Patchwork
@ 2019-07-02 16:41 ` Patchwork
  2019-07-03 13:08 ` [igt-dev] [PATCH i-g-t 1/5] " Ser, Simon
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2019-07-02 16:41 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: igt-dev

== Series Details ==

Series: series starting with [i-g-t,1/5] lib/igt_core: Add support for subtest descriptions
URL   : https://patchwork.freedesktop.org/series/63037/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6390_full -> IGTPW_3217_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/63037/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_3217_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_fence_thrash@bo-copy:
    - shard-iclb:         [PASS][1] -> [INCOMPLETE][2] ([fdo#107713] / [fdo#109100])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-iclb8/igt@gem_fence_thrash@bo-copy.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-iclb7/igt@gem_fence_thrash@bo-copy.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-glk:          [PASS][3] -> [DMESG-WARN][4] ([fdo#108686])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-glk9/igt@gem_tiled_swapping@non-threaded.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-glk9/igt@gem_tiled_swapping@non-threaded.html

  * igt@i915_pm_rc6_residency@rc6-accuracy:
    - shard-snb:          [PASS][5] -> [SKIP][6] ([fdo#109271])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-snb7/igt@i915_pm_rc6_residency@rc6-accuracy.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-snb1/igt@i915_pm_rc6_residency@rc6-accuracy.html

  * igt@i915_suspend@fence-restore-untiled:
    - shard-apl:          [PASS][7] -> [DMESG-WARN][8] ([fdo#108566])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-apl5/igt@i915_suspend@fence-restore-untiled.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-apl3/igt@i915_suspend@fence-restore-untiled.html

  * igt@kms_cursor_crc@pipe-c-cursor-64x21-sliding:
    - shard-apl:          [PASS][9] -> [FAIL][10] ([fdo#103232])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-apl2/igt@kms_cursor_crc@pipe-c-cursor-64x21-sliding.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-apl2/igt@kms_cursor_crc@pipe-c-cursor-64x21-sliding.html
    - shard-kbl:          [PASS][11] -> [FAIL][12] ([fdo#103232])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-kbl2/igt@kms_cursor_crc@pipe-c-cursor-64x21-sliding.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-kbl4/igt@kms_cursor_crc@pipe-c-cursor-64x21-sliding.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          [PASS][13] -> [FAIL][14] ([fdo#105767])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-hsw7/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-hsw8/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic.html

  * igt@kms_flip@dpms-vs-vblank-race:
    - shard-glk:          [PASS][15] -> [FAIL][16] ([fdo#103060])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-glk1/igt@kms_flip@dpms-vs-vblank-race.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-glk4/igt@kms_flip@dpms-vs-vblank-race.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw:
    - shard-iclb:         [PASS][17] -> [FAIL][18] ([fdo#103167]) +3 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-iclb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-iclb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [PASS][19] -> [SKIP][20] ([fdo#109642])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-iclb2/igt@kms_psr2_su@page_flip.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-iclb3/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-iclb:         [PASS][21] -> [SKIP][22] ([fdo#109441])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-iclb2/igt@kms_psr@psr2_primary_page_flip.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-iclb8/igt@kms_psr@psr2_primary_page_flip.html

  * igt@perf@blocking:
    - shard-iclb:         [PASS][23] -> [FAIL][24] ([fdo#110728])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-iclb8/igt@perf@blocking.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-iclb4/igt@perf@blocking.html

  
#### Possible fixes ####

  * igt@gem_exec_schedule@preemptive-hang-vebox:
    - shard-iclb:         [INCOMPLETE][25] ([fdo#107713]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-iclb7/igt@gem_exec_schedule@preemptive-hang-vebox.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-iclb7/igt@gem_exec_schedule@preemptive-hang-vebox.html

  * igt@i915_selftest@mock_requests:
    - shard-glk:          [INCOMPLETE][27] ([fdo#103359] / [k.org#198133]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-glk6/igt@i915_selftest@mock_requests.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-glk9/igt@i915_selftest@mock_requests.html

  * igt@kms_cursor_crc@pipe-a-cursor-256x256-onscreen:
    - shard-apl:          [FAIL][29] ([fdo#103232]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-apl4/igt@kms_cursor_crc@pipe-a-cursor-256x256-onscreen.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-apl4/igt@kms_cursor_crc@pipe-a-cursor-256x256-onscreen.html
    - shard-kbl:          [FAIL][31] ([fdo#103232]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-kbl3/igt@kms_cursor_crc@pipe-a-cursor-256x256-onscreen.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-kbl3/igt@kms_cursor_crc@pipe-a-cursor-256x256-onscreen.html

  * igt@kms_flip@plain-flip-fb-recreate-interruptible:
    - shard-hsw:          [INCOMPLETE][33] ([fdo#103540]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-hsw4/igt@kms_flip@plain-flip-fb-recreate-interruptible.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-hsw1/igt@kms_flip@plain-flip-fb-recreate-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [FAIL][35] ([fdo#103167]) -> [PASS][36] +2 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [INCOMPLETE][37] ([fdo#103665]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-kbl4/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-kbl3/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [DMESG-WARN][39] ([fdo#108566]) -> [PASS][40] +3 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-apl5/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-apl2/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [FAIL][41] ([fdo#103166]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-iclb7/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [FAIL][43] ([fdo#108341]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-iclb1/igt@kms_psr@no_drrs.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-iclb6/igt@kms_psr@no_drrs.html

  * igt@kms_psr@psr2_cursor_mmap_cpu:
    - shard-iclb:         [SKIP][45] ([fdo#109441]) -> [PASS][46] +2 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-iclb1/igt@kms_psr@psr2_cursor_mmap_cpu.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [FAIL][47] ([fdo#99912]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6390/shard-kbl3/igt@kms_setmode@basic.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/shard-kbl3/igt@kms_setmode@basic.html

  
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (10 -> 6)
------------------------------

  Missing    (4): pig-skl-6260u shard-skl pig-hsw-4770r pig-glk-j5005 


Build changes
-------------

  * IGT: IGT_5075 -> IGTPW_3217
  * Piglit: piglit_4509 -> None

  CI_DRM_6390: 4c6c23fdf450ab43bb4046ac1fb1439ebf176564 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3217: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/
  IGT_5075: 03779dd3de8a57544f124d9952a6d2b3e34e34ca @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3217/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 5/5] docs: Embed subtest descriptions in the documentation
  2019-07-01 12:21 ` [igt-dev] [PATCH i-g-t 5/5] docs: Embed subtest descriptions in the documentation Arkadiusz Hiler
@ 2019-07-03  7:18   ` Ser, Simon
  0 siblings, 0 replies; 17+ messages in thread
From: Ser, Simon @ 2019-07-03  7:18 UTC (permalink / raw)
  To: Hiler, Arkadiusz, igt-dev; +Cc: Latvala, Petri, daniel

On Mon, 2019-07-01 at 15:21 +0300, Arkadiusz Hiler wrote:
> This rewrites generate_description_xml in Python, so that we generate
> properly escaped XML. The switch also makes the code more manageable.
> 
> Changes in the generated docbook:
> 
> 1. subtests are not simply listed anymore, they are now another (sub)section
> 
> 2. subtests are now linkable,
>    e.g. docs/igt-kms-tests.html#kms_hdmi_inject@inject-4k
> 
> 3. subtest's section now includes output of --describe
> 
> Python is required already by gtk-doc and we are not using anything
> other than the standard library.
> 
> v2: keep the part of the subtest name after the last match (Simon)
>     explicitly require python3 (Petri)
> v3: make sure that the tail of the subtest name after the last keyword
>     match is included (Simon)
> 
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Simon Ser <simon.ser@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Acked-by: Petri Latvala <petri.latvala@intel.com>

LGTM!

Reviewed-by: Simon Ser <simon.ser@intel.com>

> ---
>  .../igt-gpu-tools/generate_description_xml.py | 114 ++++++++++++++++++
>  .../igt-gpu-tools/generate_description_xml.sh |  46 -------
>  docs/reference/igt-gpu-tools/meson.build      |   2 +-
>  meson.build                                   |   3 +-
>  4 files changed, 117 insertions(+), 48 deletions(-)
>  create mode 100755 docs/reference/igt-gpu-tools/generate_description_xml.py
>  delete mode 100644 docs/reference/igt-gpu-tools/generate_description_xml.sh
> 
> diff --git a/docs/reference/igt-gpu-tools/generate_description_xml.py b/docs/reference/igt-gpu-tools/generate_description_xml.py
> new file mode 100755
> index 00000000..8bb0989a
> --- /dev/null
> +++ b/docs/reference/igt-gpu-tools/generate_description_xml.py
> @@ -0,0 +1,114 @@
> +#!/usr/bin/env python3
> +
> +import re
> +import sys
> +import os.path
> +import subprocess
> +import xml.etree.cElementTree as ET
> +
> +from collections import namedtuple
> +
> +Subtest = namedtuple("Subtest", "name description")
> +KEYWORDS=re.compile(r'\b(invalid|hang|swap|thrash|crc|tiled|tiling|rte|ctx|render|blt|bsd|vebox|exec|rpm)\b')
> +
> +
> +def get_testlist(path):
> +    "read binaries' names from test-list.txt"
> +    with open(path, 'r') as f:
> +        assert(f.readline() == "TESTLIST\n")
> +        tests = f.readline().strip().split(" ")
> +        assert(f.readline() == "END TESTLIST\n")
> +
> +    return tests
> +
> +
> +def keywordize(root, text, keywords):
> +    "set text for root element and wrap KEYWORDS in a <acronym>"
> +    matches = list(keywords.finditer(text))
> +
> +    if not matches:
> +        root.text = text
> +        return
> +
> +    pos = 0
> +    last_element = None
> +    root.text = ""
> +
> +    for match in matches:
> +        if match.start() > pos:
> +            to_append = text[pos:match.start()]
> +
> +            if last_element == None:
> +                root.text += to_append
> +            else:
> +                last_element.tail += to_append
> +
> +        last_element = ET.SubElement(root, "acronym")
> +        last_element.tail = ""
> +        last_element.text=match.group()
> +        pos = match.end()
> +
> +    last_element.tail = text[pos:]
> +
> +
> +def get_subtests(testdir, test):
> +    "execute test and get subtests with their descriptions via --describe"
> +    output = []
> +    full_test_path = os.path.join(testdir, test)
> +    proc = subprocess.run([full_test_path, "--describe"], stdout=subprocess.PIPE)
> +    description = ""
> +    current_subtest = None
> +
> +    for line in proc.stdout.decode().splitlines():
> +        if line.startswith("SUB "):
> +            output += [Subtest(current_subtest, description)]
> +            description = ""
> +            current_subtest = line.split(' ')[1]
> +        else:
> +            description += line
> +
> +    output += [Subtest(current_subtest, description)]
> +
> +    return output
> +
> +def main():
> +    output_file   = sys.argv[1]
> +    test_filter   = re.compile(sys.argv[2])
> +    testlist_file = sys.argv[3]
> +    testdir       = os.path.abspath(os.path.dirname(testlist_file))
> +
> +    root = ET.Element("refsect1")
> +    ET.SubElement(root, "title").text = "Description"
> +
> +    tests = get_testlist(testlist_file)
> +
> +    for test in tests:
> +        if not test_filter.match(test):
> +            continue
> +
> +        test_section = ET.SubElement(root, "refsect2", id=test)
> +        test_title = ET.SubElement(test_section, "title")
> +        keywordize(test_title, test, KEYWORDS)
> +
> +        subtests = get_subtests(testdir, test)
> +
> +        # we have description with no subtest name, add it at the top level
> +        if subtests and not subtests[0].name:
> +            ET.SubElement(test_section, "para").text = subtests[0].description
> +
> +        if len(subtests) > 100:
> +            ET.SubElement(test_section, "para").text = "More than 100 subtests, skipping listing"
> +            continue
> +
> +        for name, description in subtests:
> +            if not name:
> +                continue
> +
> +            subtest_section = ET.SubElement(test_section, "refsect3", id="{}@{}".format(test, name))
> +            subtest_title = ET.SubElement(subtest_section, "title")
> +            keywordize(subtest_title, name, KEYWORDS)
> +            ET.SubElement(subtest_section, "para").text = description
> +
> +    ET.ElementTree(root).write(output_file)
> +
> +main()
> diff --git a/docs/reference/igt-gpu-tools/generate_description_xml.sh b/docs/reference/igt-gpu-tools/generate_description_xml.sh
> deleted file mode 100644
> index 705a7bf3..00000000
> --- a/docs/reference/igt-gpu-tools/generate_description_xml.sh
> +++ /dev/null
> @@ -1,46 +0,0 @@
> -#!/bin/sh
> -
> -output=$1
> -filter=$2
> -testlist=$3
> -testdir=$(dirname $testlist)
> -
> -KEYWORDS="(invalid|hang|swap|thrash|crc|tiled|tiling|rte|ctx|render|blt|bsd|vebox|exec|rpm)"
> -
> -echo "<?xml version=\"1.0\"?>" > $output
> -echo "<!DOCTYPE refsect1 PUBLIC \"-//OASIS//DTD DocBook XML V4.3//EN\"" >> $output
> -echo "               \"http://www.oasis-open.org/docbook/xml/4.3/docbookx.dtd\"" >> $output
> -echo "[" >> $output
> -echo "  <!ENTITY % local.common.attrib \"xmlns:xi  CDATA  #FIXED 'http://www.w3.org/2003/XInclude'\">;" >> $output
> -echo "  <!ENTITY version SYSTEM \"version.xml\">" >> $output
> -echo "]>" >> $output
> -echo "<refsect1>" >> $output
> -echo "<title>Description</title>" >> $output
> -for test in `cat $testlist | tr ' ' '\n' | grep "^$filter" | sort`; do
> -	echo "<refsect2 id=\"$test\"><title>" >> $output;
> -	echo "$test" | perl -pe "s/(?<=_)$KEYWORDS(?=(_|\\W))/<acronym>\\1<\\/acronym>/g" >> $output;
> -	echo "</title><para><![CDATA[" >> $output;
> -	testprog=$testdir/$test;
> -	 ./$testprog --help-description >> $output;
> -	echo "]]></para>" >> $output;
> -	if ./$testprog --list-subtests > /dev/null ; then
> -		echo "<refsect3><title>Subtests</title>" >> $output;
> -		subtest_list=`./$testprog --list-subtests`;
> -		subtest_count=`echo $subtest_list | wc -w`;
> -		if [ $subtest_count -gt 100 ]; then
> -			echo "<para>This test has over 100 subtests. " >> $output;
> -			echo "Run <command>$test</command> <option>--list-subtests</option> to list them.</para>" >> $output;
> -		else
> -			echo "<simplelist>" >> $output;
> -			for subtest in $subtest_list; do
> -				echo "<member>" >> $output;
> -				echo "$subtest" | perl -pe "s/\\b$KEYWORDS\\b/<acronym>\\1<\\/acronym>/g" >> $output;
> -				echo "</member>" >> $output;
> -			done;
> -			echo "</simplelist>" >> $output;
> -		fi;
> -		echo "</refsect3>" >> $output;
> -	fi;
> -	echo "</refsect2>" >> $output;
> -done;
> -echo "</refsect1>" >> $output
> diff --git a/docs/reference/igt-gpu-tools/meson.build b/docs/reference/igt-gpu-tools/meson.build
> index 4d177e49..e2bdc495 100644
> --- a/docs/reference/igt-gpu-tools/meson.build
> +++ b/docs/reference/igt-gpu-tools/meson.build
> @@ -45,7 +45,7 @@ test_groups = [
>  	'vgem',
>  ]
>  
> -gen_description = find_program('generate_description_xml.sh')
> +gen_description = find_program('generate_description_xml.py')
>  gen_programs = find_program('generate_programs_xml.sh')
>  
>  generated_docs = []
> diff --git a/meson.build b/meson.build
> index f0cb2543..0629d441 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -303,7 +303,8 @@ subdir('overlay')
>  subdir('man')
>  
>  gtk_doc = dependency('gtk-doc', required : build_docs)
> -if build_tests and gtk_doc.found()
> +python3 = find_program('python3', required : build_docs)
> +if build_tests and gtk_doc.found() and python3.found()
>  	subdir('docs')
>  elif build_docs.enabled()
>  	error('Documentation requires building tests')
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 1/5] lib/igt_core: Add support for subtest descriptions
  2019-07-01 12:21 [igt-dev] [PATCH i-g-t 1/5] lib/igt_core: Add support for subtest descriptions Arkadiusz Hiler
                   ` (5 preceding siblings ...)
  2019-07-02 16:41 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
@ 2019-07-03 13:08 ` Ser, Simon
  2019-07-04 11:22   ` Arkadiusz Hiler
  2019-07-04 11:55 ` [igt-dev] [PATCH 1/5 v2 i-g-t] " Arkadiusz Hiler
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: Ser, Simon @ 2019-07-03 13:08 UTC (permalink / raw)
  To: Hiler, Arkadiusz, igt-dev; +Cc: Latvala, Petri

Overall looks good. A few comments inline.

On Mon, 2019-07-01 at 15:21 +0300, Arkadiusz Hiler wrote:
> This patch adds igt_description() which attaches a description to the
> following igt_subtest or igt_subtest_group block.
> 
> Descriptions are accessible via './test --describe[=pattern]'
> 
> Subtest description is its own igt_describe as well as igt_describes
> of all the parenting igt_subtest_groups, starting from the outermost
> scope.
> 
> Examples of code and produced outputs are included in
> lib/test/igt_describe.c and as a documentation comment on igt_describe()
> macro.
> 
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Acked-by: Petri Latvala <petri.latvala@intel.com>
> ---
>  lib/igt_core.c           | 179 ++++++++++++++++++++++++--
>  lib/igt_core.h           | 137 ++++++++++++++++++--
>  lib/tests/igt_describe.c | 266 +++++++++++++++++++++++++++++++++++++++
>  lib/tests/meson.build    |   1 +
>  4 files changed, 562 insertions(+), 21 deletions(-)
>  create mode 100644 lib/tests/igt_describe.c
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 6b9f0425..8b1c7b2f 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -70,6 +70,7 @@
>  #include "igt_sysfs.h"
>  #include "igt_sysrq.h"
>  #include "igt_rc.h"
> +#include "igt_list.h"
>  
>  #define UNW_LOCAL_ONLY
>  #include <libunwind.h>
> @@ -259,6 +260,7 @@ const char *igt_interactive_debug;
>  
>  /* subtests helpers */
>  static bool list_subtests = false;
> +static bool describe_subtests = false;
>  static char *run_single_subtest = NULL;
>  static bool run_single_subtest_found = false;
>  static const char *in_subtest = NULL;
> @@ -271,6 +273,16 @@ static enum {
>  	CONT = 0, SKIP, FAIL
>  } skip_subtests_henceforth = CONT;
>  
> +static char __current_description[4096];

If it was just me, I'd intentionally make this a lot smaller to make
sure people don't start writing about their personal life in there.

I mean: if the description is bigger than 512 chars, you're doing
something wrong. And 512 chars is probably a very generous limit.
"Descriptions should fit in a Tweet".

> +
> +struct description_node {
> +	char desc[sizeof(__current_description)];
> +	struct igt_list link;
> +};
> +
> +static struct igt_list subgroup_descriptions;
> +
> +
>  bool __igt_plain_output = false;
>  
>  /* fork support state */
> @@ -285,6 +297,7 @@ enum {
>  	 * conflict with core ones
>  	 */
>  	OPT_LIST_SUBTESTS = 500,
> +	OPT_DESCRIBE_SUBTESTS,
>  	OPT_RUN_SUBTEST,
>  	OPT_DESCRIPTION,
>  	OPT_DEBUG,
> @@ -528,10 +541,59 @@ static void common_exit_handler(int sig)
>  	assert(sig != 0 || igt_exit_called);
>  }
>  
> +static void print_line_wrapping(const char *indent, const char *text)
> +{
> +	char *copy, *curr, *next_space;
> +	int current_line_length = 0;
> +	bool done = false;
> +
> +	const int total_line_length = 80;
> +	const int line_length = total_line_length - strlen(indent);
> +
> +	copy = malloc(strlen(text) + 1);
> +	memcpy(copy, text, strlen(text) + 1);
> +
> +	curr = copy;
> +
> +	printf("%s", indent);
> +
> +	while (!done) {
> +		next_space = strchr(curr, ' ');
> +
> +		if (!next_space) { /* no more spaces, print everything that is left */
> +			done = true;
> +			next_space = strchr(curr, '\0');
> +		}
> +
> +		*next_space = '\0';
> +
> +		if ((next_space - curr) + current_line_length > line_length && curr != copy) {
> +			printf("\n%s", indent);
> +			current_line_length = 0;
> +		}
> +
> +		if (current_line_length == 0)
> +			printf("%s", curr); /* first word in a line, don't space out */
> +		else
> +			printf(" %s", curr);
> +
> +		current_line_length += next_space - curr;
> +		curr = next_space + 1;
> +	}
> +
> +	printf("\n");
> +
> +	free(copy);
> +}
> +
> +
>  static void print_test_description(void)
>  {
> -	if (&__igt_test_description)
> -		printf("%s\n", __igt_test_description);
> +	if (&__igt_test_description) {
> +		print_line_wrapping("", __igt_test_description);
> +		if (describe_subtests)
> +			printf("\n");
> +	}
>  }
>  
>  static void print_version(void)
> @@ -558,6 +620,7 @@ static void print_usage(const char *help_str, bool output_on_stderr)
>  		   "  --debug[=log-domain]\n"
>  		   "  --interactive-debug[=domain]\n"
>  		   "  --help-description\n"
> +		   "  --describe\n"
>  		   "  --help|-h\n");
>  	if (help_str)
>  		fprintf(f, "%s\n", help_str);
> @@ -671,6 +734,7 @@ static int common_init(int *argc, char **argv,
>  	int c, option_index = 0, i, x;
>  	static struct option long_options[] = {
>  		{"list-subtests",     no_argument,       NULL, OPT_LIST_SUBTESTS},
> +		{"describe",          optional_argument, NULL, OPT_DESCRIBE_SUBTESTS},
>  		{"run-subtest",       required_argument, NULL, OPT_RUN_SUBTEST},
>  		{"help-description",  no_argument,       NULL, OPT_DESCRIPTION},
>  		{"debug",             optional_argument, NULL, OPT_DEBUG},
> @@ -687,6 +751,7 @@ static int common_init(int *argc, char **argv,
>  	int ret = 0;
>  
>  	common_init_env();
> +	igt_list_init(&subgroup_descriptions);
>  
>  	command_str = argv[0];
>  	if (strrchr(command_str, '/'))
> @@ -777,6 +842,13 @@ static int common_init(int *argc, char **argv,
>  			if (!run_single_subtest)
>  				list_subtests = true;
>  			break;
> +		case OPT_DESCRIBE_SUBTESTS:
> +			if (optarg)
> +				run_single_subtest = strdup(optarg);
> +			list_subtests = true;
> +			describe_subtests = true;
> +			print_test_description();
> +			break;
>  		case OPT_RUN_SUBTEST:
>  			assert(optarg);
>  			if (!list_subtests)
> @@ -934,12 +1006,41 @@ void igt_simple_init_parse_opts(int *argc, char **argv,
>  		    extra_opt_handler, handler_data);
>  }
>  
> +static void _clear_current_description(void) {
> +	__current_description[0] = '\0';
> +}
> +
> +#define __INDENT "  "

I wouldn't use a macro here. Just adding this to the body of the
function should be enough:

    static const indent[] = "  ";

> +static void __igt_print_description(const char *subtest_name, const char *file, const int line)

Unnecessary `const` for `int line`.

> +{
> +	struct description_node *desc;
> +	bool has_doc = false;
> +
> +	printf("SUB %s %s:%d:\n", subtest_name, file, line);
> +
> +	igt_list_for_each(desc, &subgroup_descriptions, link) {
> +		print_line_wrapping(__INDENT, desc->desc);
> +		printf("\n");
> +		has_doc = true;
> +	}
> +
> +	if (__current_description[0] != '\0') {
> +		print_line_wrapping(__INDENT, __current_description);
> +		printf("\n");
> +		has_doc = true;
> +	}
> +
> +	if (!has_doc)
> +		printf("%sNO DOCUMENTATION!\n\n", __INDENT);
> +}
> +#undef __INDENT
> +
>  /*
>   * Note: Testcases which use these helpers MUST NOT output anything to stdout
>   * outside of places protected by igt_run_subtest checks - the piglit
>   * runner adds every line to the subtest list.
>   */
> -bool __igt_run_subtest(const char *subtest_name)
> +bool __igt_run_subtest(const char *subtest_name, const char *file, const int line)
>  {
>  	int i;
>  
> @@ -954,18 +1055,25 @@ bool __igt_run_subtest(const char *subtest_name)
>  			igt_exit();
>  		}
>  
> -	if (list_subtests) {
> -		printf("%s\n", subtest_name);
> -		return false;
> -	}
> -
>  	if (run_single_subtest) {
> -		if (uwildmat(subtest_name, run_single_subtest) == 0)
> +		if (uwildmat(subtest_name, run_single_subtest) == 0) {
> +			_clear_current_description();
>  			return false;
> -		else
> +		} else {
>  			run_single_subtest_found = true;
> +		}
>  	}
>  
> +	if (describe_subtests) {
> +		__igt_print_description(subtest_name, file, line);
> +		_clear_current_description();
> +		return false;
> +	} else if (list_subtests) {
> +		printf("%s\n", subtest_name);
> +		return false;
> +	}
> +
> +
>  	if (skip_subtests_henceforth) {
>  		printf("%sSubtest %s: %s%s\n",
>  		       (!__igt_plain_output) ? "\x1b[1m" : "", subtest_name,
> @@ -1014,15 +1122,32 @@ bool igt_only_list_subtests(void)
>  	return list_subtests;
>  }
>  
> -void __igt_subtest_group_save(int *save)
> +
> +
> +void __igt_subtest_group_save(int *save, int *desc)

desc can be bool *.

>  {
>  	assert(test_with_subtests);
>  
> +	if (__current_description[0] != '\0') {
> +		struct description_node *new = calloc(1, sizeof(*new));
> +		memcpy(new->desc, __current_description, sizeof(__current_description));
> +		igt_list_add_tail(&new->link, &subgroup_descriptions);
> +		_clear_current_description();
> +		*desc = true;
> +	}
> +
>  	*save = skip_subtests_henceforth;
>  }
>  
> -void __igt_subtest_group_restore(int save)
> +void __igt_subtest_group_restore(int save, int desc)

desc can be bool.

>  {
> +	if (desc) {
> +		struct description_node *last =
> +			igt_list_last_entry(&subgroup_descriptions, last, link);
> +		igt_list_del(&last->link);
> +		free(last);
> +	}
> +
>  	skip_subtests_henceforth = save;
>  }
>  
> @@ -1229,6 +1354,34 @@ bool igt_can_fail(void)
>  	return !test_with_subtests || in_fixture || in_subtest;
>  }
>  
> +/**
> + * igt_describe_f:
> + * @fmt: format string containing description
> + * @...: argument used by the format string
> + *
> + * Attach a description to the following #igt_subtest or #igt_subtest_group
> + * block.
> + *
> + * Check #igt_describe for more details.
> + *
> + */
> +void igt_describe_f(const char *fmt, ...)
> +{
> +	int ret;
> +	va_list args;
> +
> +	if (!describe_subtests)
> +		return;
> +
> +	va_start(args, fmt);
> +
> +	ret = vsnprintf(__current_description, sizeof(__current_description), fmt, args);
> +
> +	va_end(args);
> +
> +	assert(ret < sizeof(__current_description));
> +}
> +
>  static bool running_under_gdb(void)
>  {
>  	char pathname[30], buf[1024];
> @@ -1540,7 +1693,7 @@ void igt_exit(void)
>  		g_key_file_free(igt_key_file);
>  
>  	if (run_single_subtest && !run_single_subtest_found) {
> -		igt_warn("Unknown subtest: %s\n", run_single_subtest);
> +		igt_critical("Unknown subtest: %s\n", run_single_subtest);
>  		exit(IGT_EXIT_INVALID);
>  	}
>  
> diff --git a/lib/igt_core.h b/lib/igt_core.h
> index 88a95ec2..1b37f142 100644
> --- a/lib/igt_core.h
> +++ b/lib/igt_core.h
> @@ -174,7 +174,7 @@ int igt_subtest_init_parse_opts(int *argc, char **argv,
>  #define igt_subtest_init(argc, argv) \
>  	igt_subtest_init_parse_opts(&argc, argv, NULL, NULL, NULL, NULL, NULL);
>  
> -bool __igt_run_subtest(const char *subtest_name);
> +bool __igt_run_subtest(const char *subtest_name, const char *file, const int line);
>  #define __igt_tokencat2(x, y) x ## y
>  
>  /**
> @@ -198,14 +198,14 @@ bool __igt_run_subtest(const char *subtest_name);
>   *
>   * This is a simpler version of igt_subtest_f()
>   */
> -#define igt_subtest(name) for (; __igt_run_subtest((name)) && \
> +#define igt_subtest(name) for (; __igt_run_subtest((name), __FILE__, __LINE__) && \
>  				   (sigsetjmp(igt_subtest_jmpbuf, 1) == 0); \
>  				   igt_success())
>  #define __igt_subtest_f(tmp, format...) \
>  	for (char tmp [256]; \
>  	     snprintf( tmp , sizeof( tmp ), \
>  		      format), \
> -	     __igt_run_subtest( tmp ) && \
> +	     __igt_run_subtest(tmp, __FILE__, __LINE__) && \
>  	     (sigsetjmp(igt_subtest_jmpbuf, 1) == 0); \
>  	     igt_success())
>  
> @@ -227,8 +227,8 @@ bool __igt_run_subtest(const char *subtest_name);
>  const char *igt_subtest_name(void);
>  bool igt_only_list_subtests(void);
>  
> -void __igt_subtest_group_save(int *);
> -void __igt_subtest_group_restore(int);
> +void __igt_subtest_group_save(int *, int *);
> +void __igt_subtest_group_restore(int, int);
>  /**
>   * igt_subtest_group:
>   *
> @@ -245,11 +245,14 @@ void __igt_subtest_group_restore(int);
>   * group will fail or skip. Subtest groups can be arbitrarily nested.
>   */
>  #define igt_subtest_group for (int igt_tokencat(__tmpint,__LINE__) = 0, \
> -			       igt_tokencat(__save,__LINE__) = 0; \
> +			       igt_tokencat(__save,__LINE__) = 0, \
> +			       igt_tokencat(__desc,__LINE__) = 0; \
>  			       igt_tokencat(__tmpint,__LINE__) < 1 && \
> -			       (__igt_subtest_group_save(& igt_tokencat(__save,__LINE__) ), true); \
> +			       (__igt_subtest_group_save(& igt_tokencat(__save,__LINE__), \
> +							 & igt_tokencat(__desc,__LINE__) ), true); \
>  			       igt_tokencat(__tmpint,__LINE__) ++, \
> -			       __igt_subtest_group_restore(igt_tokencat(__save,__LINE__) ))
> +			       __igt_subtest_group_restore(igt_tokencat(__save,__LINE__), \
> +							   igt_tokencat(__desc,__LINE__)))
>  
>  /**
>   * igt_main_args:
> @@ -385,6 +388,124 @@ static inline void igt_ignore_warn(bool value)
>  {
>  }
>  
> +__attribute__((format(printf, 1, 2)))
> +void igt_describe_f(const char *fmt, ...);
> +
> +/**
> + * igt_describe:
> + * @dsc: string containing description
> + *
> + * Attach a description to the following #igt_subtest or #igt_subtest_group
> + * block.
> + *
> + * The description should complement the test/subtest name and provide more
> + * context on what is being tested. It should explain the idea of the test and
> + * do not mention implementation details, so that it never goes out of date.
> + *
> + * DO:
> + *  * focus on the userspace's perspective
> + *  * try to capture the reason for the test's existence
> + *  * be brief
> + *
> + * DON'T:
> + *  * try to translate the code into English
> + *  * explain all the checks the test does
> + *  * delve on the implementation
> + *
> + * Good examples:
> + *  * "make sure that legacy cursor updates do not stall atomic commits"
> + *  * "check that atomic updates of many planes are indeed atomic and take
> + *     effect immediately after the commit"
> + *  * "make sure that the meta-data exposed by the kernel to the userspace
> + *     is correct and matches the used EDID"
> + *
> + * Bad examples:
> + *  * "spawn 10 threads, each pinning cpu core with a busy loop..."
> + *  * "randomly generate holes in a primary plane then try to cover each hole
> + *    with a plane and make sure that CRC matches, do 25 gazillion rounds of
> + *    that..."
> + *
> + *
> + * Resulting #igt_subtest documentation is a concatenation of its own
> + * description and all the parenting #igt_subtest_group descriptions, starting
> + * from the outermost one. Example:
> + *
> + * |[<!-- language="C" -->
> + * #include "igt.h"
> + *
> + * IGT_TEST_DESCRIPTION("Global description of the whole binary");
> + * igt_main
> + * {
> + * 	igt_describe("Desc of the subgroup with A and B");
> + * 	igt_subtest_group {
> + * 		igt_describe("Desc of the subtest A");
> + * 		igt_subtest("subtest-a") {
> + * 			...
> + * 		}
> + *
> + * 		igt_describe("Desc of the subtest B");
> + * 		igt_subtest("subtest-b") {
> + * 			...
> + * 		}
> + * 	}
> + *
> + * 	igt_describe("Desc of the subtest C");
> + * 	igt_subtest("subtest-c") {
> + * 		...
> + * 	}
> + * }
> + * ]|
> + *
> + * It's will accessible via --describe command line switch:
> + *
> + * |[
> + * $ test --describe
> + * Global description of the whole binary
> + *
> + * SUB subtest-a test.c:5:
> + *   Desc of the subgroup with A and B
> + *
> + *   Desc of the subtest A
> + *
> + * SUB subtest-b test.c:10:
> + *   Desc of the subgroup with A and B
> + *
> + *   Desc of the subtest B
> + *
> + * SUB subtest-c test.c:15:
> + *   Desc of the subtest C
> + * ]|
> + *
> + * Every single #igt_subtest does not have to be preceded with a #igt_describe
> + * as long as it has good-enough explanation provided on the #igt_subtest_group
> + * level.
> + *
> + * Example:
> + *
> + * |[<!-- language="C" -->
> + * #include "igt.h"
> + *
> + * igt_main
> + * {
> + * 	igt_describe("check xyz with different tilings");
> + * 	igt_subtest_group {
> + * 		// no need for extra description, group is enough and tiling is
> + * 		// obvious from the test name
> + * 		igt_subtest("foo-tiling-x") {
> + * 			...
> + * 		}
> + *
> + * 		igt_subtest("foo-tiling-y") {
> + * 			...
> + * 		}
> + * 	}
> + * }
> + * ]|
> + *
> + */
> +#define igt_describe(dsc) \
> +	igt_describe_f("%s", dsc)
> +
>  /**
>   * igt_assert:
>   * @expr: condition to test
> diff --git a/lib/tests/igt_describe.c b/lib/tests/igt_describe.c
> new file mode 100644
> index 00000000..a9f857bb
> --- /dev/null
> +++ b/lib/tests/igt_describe.c
> @@ -0,0 +1,266 @@
> +/*
> + * Copyright © 2019 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 <sys/wait.h>
> +#include <string.h>
> +#include <stdlib.h>
> +
> +#include "drmtest.h"
> +#include "igt_tests_common.h"
> +
> +IGT_TEST_DESCRIPTION("the top level description");
> +static void fake_main(int argc, char **argv) {
> +	igt_subtest_init(argc, argv);
> +
> +	igt_describe("Basic A");
> +	igt_subtest("A")
> +		;
> +
> +	igt_fixture
> +		printf("should not be executed!\n");
> +
> +	igt_describe("Group with B, C & D");
> +	igt_subtest_group {
> +		igt_describe("Basic B");
> +		igt_subtest("B")
> +			;
> +
> +		if (!igt_only_list_subtests())
> +			printf("should not be executed!\n");
> +
> +		igt_describe("Group with C & D");
> +		igt_subtest_group {
> +			igt_describe("Basic C");
> +			igt_subtest("C")
> +				printf("should not be executed!\n");
> +
> +			// NO DOC
> +			igt_subtest("D")
> +				;
> +		}
> +	}
> +
> +	// NO DOC
> +	igt_subtest_group {
> +		// NO DOC
> +		igt_subtest("E")
> +			;
> +	}
> +
> +	// NO DOC
> +	igt_subtest("F")
> +		;
> +
> +	igt_describe("this description should be so long that it wraps itself nicely in the terminal "
> +		     "this description should be so long that it wraps itself nicely in the terminal "
> +		     "this description should be so long that it wraps itself nicely in the terminal "
> +		     "this description should be so long that it wraps itself nicely in the terminal "
> +		     "this description should be so long that it wraps itself nicely in the terminal "
> +		     "this description should be so long that it wraps itself nicely in the terminal");
> +	igt_subtest("G")
> +		;
> +
> +	igt_describe("verylongwordthatshoudlbeprintedeventhoughitspastthewrppinglimit"
> +		     "verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit "
> +		     "verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit"
> +		     "verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit");
> +	igt_subtest("F")
> +		;
> +
> +	igt_exit();
> +}
> +
> +static const char DESCRIBE_ALL_OUTPUT[] = \
> +	"the top level description\n"
> +	"\n"
> +	"SUB A ../lib/tests/igt_describe.c:36:\n"
> +	"  Basic A\n"
> +	"\n"
> +	"SUB B ../lib/tests/igt_describe.c:45:\n"
> +	"  Group with B, C & D\n"
> +	"\n"
> +	"  Basic B\n"
> +	"\n"
> +	"SUB C ../lib/tests/igt_describe.c:54:\n"
> +	"  Group with B, C & D\n"
> +	"\n"
> +	"  Group with C & D\n"
> +	"\n"
> +	"  Basic C\n"
> +	"\n"
> +	"SUB D ../lib/tests/igt_describe.c:58:\n"
> +	"  Group with B, C & D\n"
> +	"\n"
> +	"  Group with C & D\n"
> +	"\n"
> +	"SUB E ../lib/tests/igt_describe.c:66:\n"
> +	"  NO DOCUMENTATION!\n"
> +	"\n"
> +	"SUB F ../lib/tests/igt_describe.c:71:\n"
> +	"  NO DOCUMENTATION!\n"
> +	"\n"
> +	"SUB G ../lib/tests/igt_describe.c:80:\n"
> +	"  this description should be so long that it wraps itself nicely in the terminal this\n"
> +	"  description should be so long that it wraps itself nicely in the terminal this description\n"
> +	"  should be so long that it wraps itself nicely in the terminal this description should be so\n"
> +	"  long that it wraps itself nicely in the terminal this description should be so long that it\n"
> +	"  wraps itself nicely in the terminal this description should be so long that it wraps itself\n"
> +	"  nicely in the terminal\n"
> +	"\n"
> +	"SUB F ../lib/tests/igt_describe.c:87:\n"
> +	"  verylongwordthatshoudlbeprintedeventhoughitspastthewrppinglimitverylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit\n"
> +	"  verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimitverylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit\n\n";
> +
> +static const char JUST_C_OUTPUT[] = \
> +	"the top level description\n"
> +	"\n"
> +	"SUB C ../lib/tests/igt_describe.c:54:\n"
> +	"  Group with B, C & D\n"
> +	"\n"
> +	"  Group with C & D\n"
> +	"\n"
> +	"  Basic C\n"
> +	"\n";
> +
> +static void assert_pipe_empty(int fd)
> +{
> +	char buf[5];
> +	internal_assert(0 == read(fd, buf, sizeof(buf)));
> +}
> +
> +static void read_whole_pipe(int fd, char *buf, size_t buflen)
> +{
> +	ssize_t readlen;
> +	off_t offset;
> +
> +	offset = 0;
> +	while ((readlen = read(fd, buf+offset, buflen-offset))) {

Doesn't handle EINTR.

> +		internal_assert(readlen != -1);
> +		offset += readlen;
> +	}
> +}
> +
> +static pid_t do_fork(int argc, char **argv, int *out, int *err)
> +{
> +	int outfd[2], errfd[2];
> +	pid_t pid;
> +
> +	internal_assert(pipe(outfd) != -1);
> +	internal_assert(pipe(errfd) != -1);
> +
> +	pid = fork();
> +	internal_assert(pid != -1);
> +
> +	if (pid == 0) {
> +		while ((dup2(outfd[1], STDOUT_FILENO) == -1) && (errno == EINTR)) {}
> +		while ((dup2(errfd[1], STDERR_FILENO) == -1) && (errno == EINTR)) {}

Unnecessary parentheses here.

Need to close outfd[1], errfd[1], outfd[0] and errfd[0] at this point.

> +		fake_main(argc, argv);
> +
> +		exit(-1);
> +	} else {
> +		/* close the writing ends */
> +		close(outfd[1]);
> +		close(errfd[1]);
> +
> +		*out = outfd[0];
> +		*err = errfd[0];
> +
> +		return pid;
> +	}
> +}
> +
> +static int _wait(pid_t pid, int *status) {
> +	int ret;
> +
> +	do {
> +		ret = waitpid(pid, status, 0);
> +	} while (ret == -1 && errno == EINTR);
> +
> +	return ret;

We could check ret == 0 in this function. Could also check for
WIFEXITED, since WEXITSTATUS only works in this case.

> +}
> +
> +int main(int argc, char **argv)
> +{
> +	char prog[] = "igt_describe";
> +	int status;
> +	int outfd, errfd;
> +
> +	/* describe all subtest */ {
> +		static char out[4096];
> +		char arg[] = "--describe";
> +		char *fake_argv[] = {prog, arg};
> +		int fake_argc = ARRAY_SIZE(fake_argv);
> +
> +		pid_t pid = do_fork(fake_argc, fake_argv, &outfd, &errfd);
> +
> +		read_whole_pipe(outfd, out, sizeof(out));
> +		assert_pipe_empty(errfd);
> +
> +		internal_assert(_wait(pid, &status) != -1);
> +		internal_assert(WEXITSTATUS(status) == IGT_EXIT_SUCCESS);
> +		internal_assert(0 == strcmp(DESCRIBE_ALL_OUTPUT, out));
> +
> +		close(outfd);
> +		close(errfd);
> +	}
> +
> +	/* describe C using a pattern */ {
> +		static char out[4096];
> +		char arg[] = "--describe=C";
> +		char *fake_argv[] = {prog, arg};
> +		int fake_argc = ARRAY_SIZE(fake_argv);
> +
> +		pid_t pid = do_fork(fake_argc, fake_argv, &outfd, &errfd);
> +
> +		read_whole_pipe(outfd, out, sizeof(out));
> +		assert_pipe_empty(errfd);
> +
> +		internal_assert(_wait(pid, &status) != -1);
> +		internal_assert(WEXITSTATUS(status) == IGT_EXIT_SUCCESS);
> +		internal_assert(0 == strcmp(JUST_C_OUTPUT, out));
> +
> +		close(outfd);
> +		close(errfd);
> +	}
> +
> +	/* fail describing with a bad pattern */ {
> +		static char err[4096];
> +		char arg[] = "--describe=Z";
> +		char *fake_argv[] = {prog, arg};
> +		int fake_argc = ARRAY_SIZE(fake_argv);
> +
> +		pid_t pid = do_fork(fake_argc, fake_argv, &outfd, &errfd);
> +
> +		read_whole_pipe(errfd, err, sizeof(err));
> +
> +		internal_assert(_wait(pid, &status) != -1);
> +		internal_assert(WEXITSTATUS(status) == IGT_EXIT_INVALID);
> +		internal_assert(strstr(err, "Unknown subtest: Z"));
> +
> +		close(outfd);
> +		close(errfd);
> +	}
> +
> +	return 0;
> +}
> diff --git a/lib/tests/meson.build b/lib/tests/meson.build
> index b930ee6e..4d907e34 100644
> --- a/lib/tests/meson.build
> +++ b/lib/tests/meson.build
> @@ -3,6 +3,7 @@ lib_tests = [
>  	'igt_can_fail',
>  	'igt_can_fail_simple',
>  	'igt_conflicting_args',
> +	'igt_describe',
>  	'igt_edid',
>  	'igt_exit_handler',
>  	'igt_fork',
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 1/5] lib/igt_core: Add support for subtest descriptions
  2019-07-03 13:08 ` [igt-dev] [PATCH i-g-t 1/5] " Ser, Simon
@ 2019-07-04 11:22   ` Arkadiusz Hiler
  2019-07-04 11:33     ` Ser, Simon
  0 siblings, 1 reply; 17+ messages in thread
From: Arkadiusz Hiler @ 2019-07-04 11:22 UTC (permalink / raw)
  To: Ser, Simon; +Cc: igt-dev, Latvala, Petri

On Wed, Jul 03, 2019 at 04:08:54PM +0300, Ser, Simon wrote:
> Overall looks good. A few comments inline.
> 
> On Mon, 2019-07-01 at 15:21 +0300, Arkadiusz Hiler wrote:
> > This patch adds igt_description() which attaches a description to the
> > following igt_subtest or igt_subtest_group block.
> > 
> > Descriptions are accessible via './test --describe[=pattern]'
> > 
> > Subtest description is its own igt_describe as well as igt_describes
> > of all the parenting igt_subtest_groups, starting from the outermost
> > scope.
> > 
> > Examples of code and produced outputs are included in
> > lib/test/igt_describe.c and as a documentation comment on igt_describe()
> > macro.
> > 
> > Cc: Petri Latvala <petri.latvala@intel.com>
> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > Acked-by: Petri Latvala <petri.latvala@intel.com>
> > ---
> >  lib/igt_core.c           | 179 ++++++++++++++++++++++++--
> >  lib/igt_core.h           | 137 ++++++++++++++++++--
> >  lib/tests/igt_describe.c | 266 +++++++++++++++++++++++++++++++++++++++
> >  lib/tests/meson.build    |   1 +
> >  4 files changed, 562 insertions(+), 21 deletions(-)
> >  create mode 100644 lib/tests/igt_describe.c
> > 
> > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > index 6b9f0425..8b1c7b2f 100644
> > --- a/lib/igt_core.c
> > +++ b/lib/igt_core.c
> > @@ -70,6 +70,7 @@
> >  #include "igt_sysfs.h"
> >  #include "igt_sysrq.h"
> >  #include "igt_rc.h"
> > +#include "igt_list.h"
> >  
> >  #define UNW_LOCAL_ONLY
> >  #include <libunwind.h>
> > @@ -259,6 +260,7 @@ const char *igt_interactive_debug;
> >  
> >  /* subtests helpers */
> >  static bool list_subtests = false;
> > +static bool describe_subtests = false;
> >  static char *run_single_subtest = NULL;
> >  static bool run_single_subtest_found = false;
> >  static const char *in_subtest = NULL;
> > @@ -271,6 +273,16 @@ static enum {
> >  	CONT = 0, SKIP, FAIL
> >  } skip_subtests_henceforth = CONT;
> >  
> > +static char __current_description[4096];
> 
> If it was just me, I'd intentionally make this a lot smaller to make
> sure people don't start writing about their personal life in there.
> 
> I mean: if the description is bigger than 512 chars, you're doing
> something wrong. And 512 chars is probably a very generous limit.
> "Descriptions should fit in a Tweet".

fair point, reduced to 512

<SNIP>
> > +#define __INDENT "  "
> 
> I wouldn't use a macro here. Just adding this to the body of the
> function should be enough:
> 
>     static const indent[] = "  ";

It was a leftover from back when this was defined for a couple of
functions. Fixed.

<SNIP>
> > -void __igt_subtest_group_save(int *save)
> > +
> > +
> > +void __igt_subtest_group_save(int *save, int *desc)
> 
> desc can be bool *.

#define igt_subtest_group for (int igt_tokencat(__tmpint,__LINE__) = 0, \
			       igt_tokencat(__save,__LINE__) = 0, \
			       igt_tokencat(__desc,__LINE__) = 0; \
			       igt_tokencat(__tmpint,__LINE__) < 1 && \
			       (__igt_subtest_group_save(& igt_tokencat(__save,__LINE__), \
							 & igt_tokencat(__desc,__LINE__) ), true); \
			       igt_tokencat(__tmpint,__LINE__) ++, \
			       __igt_subtest_group_restore(igt_tokencat(__save,__LINE__), \
							   igt_tokencat(__desc,__LINE__)))

Because of that it's int. We could make the function take bool, but that
just adds some complications with type casting.

<SNIP>
> > +static void read_whole_pipe(int fd, char *buf, size_t buflen)
> > +{
> > +	ssize_t readlen;
> > +	off_t offset;
> > +
> > +	offset = 0;
> > +	while ((readlen = read(fd, buf+offset, buflen-offset))) {
> 
> Doesn't handle EINTR.

if (readlen == -1) {
	if (errno == EINTR) {
		continue;
	} else {
		printf("read filed with %s\n", strerror(errno));
		exit(1);
	}
}

<SNIP>
> > diff --git a/lib/tests/igt_describe.c b/lib/tests/igt_describe.c
> > new file mode 100644
> > index 00000000..a9f857bb
> > --- /dev/null
> > +++ b/lib/tests/igt_describe.c
<SNIP>
> > +static int _wait(pid_t pid, int *status) {
> > +	int ret;
> > +
> > +	do {
> > +		ret = waitpid(pid, status, 0);
> > +	} while (ret == -1 && errno == EINTR);
> > +
> > +	return ret;
> 
> We could check ret == 0 in this function.

If wait() or waitpid() returns because the status of a child process is
available, these functions shall return a value equal to the process ID
of the child process for which status is reported. If wait() or
waitpid() returns due to the delivery of a signal to the calling
process, -1 shall be returned and errno set to [EINTR]. If waitpid() was
invoked with WNOHANG set in options, it has at least one child process
specified by pid for which status is not available, and status is not
available for any process specified by pid, 0 is returned. Otherwise,
(pid_t)-1 shall be returned, and errno set to indicate the error. 


So I am not really sure why we would check for (ret == 0) here.
Can you elaborate?

> Could also check for WIFEXITED, since WEXITSTATUS only works in this
> case.

I'll add internal_assert(WIFEXITED(status));

Thanks for the review!

-- 
Cheers,
Arek
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 1/5] lib/igt_core: Add support for subtest descriptions
  2019-07-04 11:22   ` Arkadiusz Hiler
@ 2019-07-04 11:33     ` Ser, Simon
  0 siblings, 0 replies; 17+ messages in thread
From: Ser, Simon @ 2019-07-04 11:33 UTC (permalink / raw)
  To: Hiler, Arkadiusz; +Cc: igt-dev, Latvala, Petri

On Thu, 2019-07-04 at 14:22 +0300, Arkadiusz Hiler wrote:
> On Wed, Jul 03, 2019 at 04:08:54PM +0300, Ser, Simon wrote:
> > Overall looks good. A few comments inline.
> > 
> > On Mon, 2019-07-01 at 15:21 +0300, Arkadiusz Hiler wrote:
> > > This patch adds igt_description() which attaches a description to the
> > > following igt_subtest or igt_subtest_group block.
> > > 
> > > Descriptions are accessible via './test --describe[=pattern]'
> > > 
> > > Subtest description is its own igt_describe as well as igt_describes
> > > of all the parenting igt_subtest_groups, starting from the outermost
> > > scope.
> > > 
> > > Examples of code and produced outputs are included in
> > > lib/test/igt_describe.c and as a documentation comment on igt_describe()
> > > macro.
> > > 
> > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > > Acked-by: Petri Latvala <petri.latvala@intel.com>
> > > ---
> > >  lib/igt_core.c           | 179 ++++++++++++++++++++++++--
> > >  lib/igt_core.h           | 137 ++++++++++++++++++--
> > >  lib/tests/igt_describe.c | 266 +++++++++++++++++++++++++++++++++++++++
> > >  lib/tests/meson.build    |   1 +
> > >  4 files changed, 562 insertions(+), 21 deletions(-)
> > >  create mode 100644 lib/tests/igt_describe.c
> > > 
> > > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > > index 6b9f0425..8b1c7b2f 100644
> > > --- a/lib/igt_core.c
> > > +++ b/lib/igt_core.c
> > > @@ -70,6 +70,7 @@
> > >  #include "igt_sysfs.h"
> > >  #include "igt_sysrq.h"
> > >  #include "igt_rc.h"
> > > +#include "igt_list.h"
> > >  
> > >  #define UNW_LOCAL_ONLY
> > >  #include <libunwind.h>
> > > @@ -259,6 +260,7 @@ const char *igt_interactive_debug;
> > >  
> > >  /* subtests helpers */
> > >  static bool list_subtests = false;
> > > +static bool describe_subtests = false;
> > >  static char *run_single_subtest = NULL;
> > >  static bool run_single_subtest_found = false;
> > >  static const char *in_subtest = NULL;
> > > @@ -271,6 +273,16 @@ static enum {
> > >  	CONT = 0, SKIP, FAIL
> > >  } skip_subtests_henceforth = CONT;
> > >  
> > > +static char __current_description[4096];
> > 
> > If it was just me, I'd intentionally make this a lot smaller to make
> > sure people don't start writing about their personal life in there.
> > 
> > I mean: if the description is bigger than 512 chars, you're doing
> > something wrong. And 512 chars is probably a very generous limit.
> > "Descriptions should fit in a Tweet".
> 
> fair point, reduced to 512
> 
> <SNIP>
> > > +#define __INDENT "  "
> > 
> > I wouldn't use a macro here. Just adding this to the body of the
> > function should be enough:
> > 
> >     static const indent[] = "  ";
> 
> It was a leftover from back when this was defined for a couple of
> functions. Fixed.
> 
> <SNIP>
> > > -void __igt_subtest_group_save(int *save)
> > > +
> > > +
> > > +void __igt_subtest_group_save(int *save, int *desc)
> > 
> > desc can be bool *.
> 
> #define igt_subtest_group for (int igt_tokencat(__tmpint,__LINE__) = 0, \
> 			       igt_tokencat(__save,__LINE__) = 0, \
> 			       igt_tokencat(__desc,__LINE__) = 0; \
> 			       igt_tokencat(__tmpint,__LINE__) < 1 && \
> 			       (__igt_subtest_group_save(& igt_tokencat(__save,__LINE__), \
> 							 & igt_tokencat(__desc,__LINE__) ), true); \
> 			       igt_tokencat(__tmpint,__LINE__) ++, \
> 			       __igt_subtest_group_restore(igt_tokencat(__save,__LINE__), \
> 							   igt_tokencat(__desc,__LINE__)))
> 
> Because of that it's int. We could make the function take bool, but that
> just adds some complications with type casting.

I see, fine by me then

> <SNIP>
> > > +static void read_whole_pipe(int fd, char *buf, size_t buflen)
> > > +{
> > > +	ssize_t readlen;
> > > +	off_t offset;
> > > +
> > > +	offset = 0;
> > > +	while ((readlen = read(fd, buf+offset, buflen-offset))) {
> > 
> > Doesn't handle EINTR.
> 
> if (readlen == -1) {
> 	if (errno == EINTR) {
> 		continue;
> 	} else {
> 		printf("read filed with %s\n", strerror(errno));
> 		exit(1);
> 	}
> }

Looks good minus the typo.

(Also the else branch is unnecessary, but this is not important.)

> <SNIP>
> > > diff --git a/lib/tests/igt_describe.c b/lib/tests/igt_describe.c
> > > new file mode 100644
> > > index 00000000..a9f857bb
> > > --- /dev/null
> > > +++ b/lib/tests/igt_describe.c
> <SNIP>
> > > +static int _wait(pid_t pid, int *status) {
> > > +	int ret;
> > > +
> > > +	do {
> > > +		ret = waitpid(pid, status, 0);
> > > +	} while (ret == -1 && errno == EINTR);
> > > +
> > > +	return ret;
> > 
> > We could check ret == 0 in this function.
> 
> If wait() or waitpid() returns because the status of a child process is
> available, these functions shall return a value equal to the process ID
> of the child process for which status is reported. If wait() or
> waitpid() returns due to the delivery of a signal to the calling
> process, -1 shall be returned and errno set to [EINTR]. If waitpid() was
> invoked with WNOHANG set in options, it has at least one child process
> specified by pid for which status is not available, and status is not
> available for any process specified by pid, 0 is returned. Otherwise,
> (pid_t)-1 shall be returned, and errno set to indicate the error. 
> 
> 
> So I am not really sure why we would check for (ret == 0) here.
> Can you elaborate?

Sorry, I meant check for ret >= 0.

> > Could also check for WIFEXITED, since WEXITSTATUS only works in this
> > case.
> 
> I'll add internal_assert(WIFEXITED(status));
> 
> Thanks for the review!
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [igt-dev] [PATCH 1/5 v2 i-g-t] lib/igt_core: Add support for subtest descriptions
  2019-07-01 12:21 [igt-dev] [PATCH i-g-t 1/5] lib/igt_core: Add support for subtest descriptions Arkadiusz Hiler
                   ` (6 preceding siblings ...)
  2019-07-03 13:08 ` [igt-dev] [PATCH i-g-t 1/5] " Ser, Simon
@ 2019-07-04 11:55 ` Arkadiusz Hiler
  2019-07-04 13:02   ` Ser, Simon
  2019-07-04 12:48 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [1/5,v2,i-g-t] lib/igt_core: Add support for subtest descriptions (rev2) Patchwork
  2019-07-05 17:06 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  9 siblings, 1 reply; 17+ messages in thread
From: Arkadiusz Hiler @ 2019-07-04 11:55 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala

This patch adds igt_description() which attaches a description to the
following igt_subtest or igt_subtest_group block.

Descriptions are accessible via './test --describe[=pattern]'

Subtest description is its own igt_describe as well as igt_describes
of all the parenting igt_subtest_groups, starting from the outermost
scope.

Examples of code and produced outputs are included in
lib/test/igt_describe.c and as a documentation comment on igt_describe()
macro.

v2: address Simon's review

Cc: Simon Ser <simon.ser@intel.com>
Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Acked-by: Petri Latvala <petri.latvala@intel.com>
---
 delta with v1: https://hiler.eu/p/ab062f76.txt

 lib/igt_core.c           | 179 +++++++++++++++++++++++--
 lib/igt_core.h           | 137 +++++++++++++++++--
 lib/tests/igt_describe.c | 280 +++++++++++++++++++++++++++++++++++++++
 lib/tests/meson.build    |   1 +
 4 files changed, 576 insertions(+), 21 deletions(-)
 create mode 100644 lib/tests/igt_describe.c

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 6b9f0425..425848a2 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -70,6 +70,7 @@
 #include "igt_sysfs.h"
 #include "igt_sysrq.h"
 #include "igt_rc.h"
+#include "igt_list.h"
 
 #define UNW_LOCAL_ONLY
 #include <libunwind.h>
@@ -259,6 +260,7 @@ const char *igt_interactive_debug;
 
 /* subtests helpers */
 static bool list_subtests = false;
+static bool describe_subtests = false;
 static char *run_single_subtest = NULL;
 static bool run_single_subtest_found = false;
 static const char *in_subtest = NULL;
@@ -271,6 +273,16 @@ static enum {
 	CONT = 0, SKIP, FAIL
 } skip_subtests_henceforth = CONT;
 
+static char __current_description[512];
+
+struct description_node {
+	char desc[sizeof(__current_description)];
+	struct igt_list link;
+};
+
+static struct igt_list subgroup_descriptions;
+
+
 bool __igt_plain_output = false;
 
 /* fork support state */
@@ -285,6 +297,7 @@ enum {
 	 * conflict with core ones
 	 */
 	OPT_LIST_SUBTESTS = 500,
+	OPT_DESCRIBE_SUBTESTS,
 	OPT_RUN_SUBTEST,
 	OPT_DESCRIPTION,
 	OPT_DEBUG,
@@ -528,10 +541,59 @@ static void common_exit_handler(int sig)
 	assert(sig != 0 || igt_exit_called);
 }
 
+static void print_line_wrapping(const char *indent, const char *text)
+{
+	char *copy, *curr, *next_space;
+	int current_line_length = 0;
+	bool done = false;
+
+	const int total_line_length = 80;
+	const int line_length = total_line_length - strlen(indent);
+
+	copy = malloc(strlen(text) + 1);
+	memcpy(copy, text, strlen(text) + 1);
+
+	curr = copy;
+
+	printf("%s", indent);
+
+	while (!done) {
+		next_space = strchr(curr, ' ');
+
+		if (!next_space) { /* no more spaces, print everything that is left */
+			done = true;
+			next_space = strchr(curr, '\0');
+		}
+
+		*next_space = '\0';
+
+		if ((next_space - curr) + current_line_length > line_length && curr != copy) {
+			printf("\n%s", indent);
+			current_line_length = 0;
+		}
+
+		if (current_line_length == 0)
+			printf("%s", curr); /* first word in a line, don't space out */
+		else
+			printf(" %s", curr);
+
+		current_line_length += next_space - curr;
+		curr = next_space + 1;
+	}
+
+	printf("\n");
+
+	free(copy);
+}
+
+
 static void print_test_description(void)
 {
-	if (&__igt_test_description)
-		printf("%s\n", __igt_test_description);
+	if (&__igt_test_description) {
+		print_line_wrapping("", __igt_test_description);
+		if (describe_subtests)
+			printf("\n");
+	}
 }
 
 static void print_version(void)
@@ -558,6 +620,7 @@ static void print_usage(const char *help_str, bool output_on_stderr)
 		   "  --debug[=log-domain]\n"
 		   "  --interactive-debug[=domain]\n"
 		   "  --help-description\n"
+		   "  --describe\n"
 		   "  --help|-h\n");
 	if (help_str)
 		fprintf(f, "%s\n", help_str);
@@ -671,6 +734,7 @@ static int common_init(int *argc, char **argv,
 	int c, option_index = 0, i, x;
 	static struct option long_options[] = {
 		{"list-subtests",     no_argument,       NULL, OPT_LIST_SUBTESTS},
+		{"describe",          optional_argument, NULL, OPT_DESCRIBE_SUBTESTS},
 		{"run-subtest",       required_argument, NULL, OPT_RUN_SUBTEST},
 		{"help-description",  no_argument,       NULL, OPT_DESCRIPTION},
 		{"debug",             optional_argument, NULL, OPT_DEBUG},
@@ -687,6 +751,7 @@ static int common_init(int *argc, char **argv,
 	int ret = 0;
 
 	common_init_env();
+	igt_list_init(&subgroup_descriptions);
 
 	command_str = argv[0];
 	if (strrchr(command_str, '/'))
@@ -777,6 +842,13 @@ static int common_init(int *argc, char **argv,
 			if (!run_single_subtest)
 				list_subtests = true;
 			break;
+		case OPT_DESCRIBE_SUBTESTS:
+			if (optarg)
+				run_single_subtest = strdup(optarg);
+			list_subtests = true;
+			describe_subtests = true;
+			print_test_description();
+			break;
 		case OPT_RUN_SUBTEST:
 			assert(optarg);
 			if (!list_subtests)
@@ -934,12 +1006,41 @@ void igt_simple_init_parse_opts(int *argc, char **argv,
 		    extra_opt_handler, handler_data);
 }
 
+static void _clear_current_description(void) {
+	__current_description[0] = '\0';
+}
+
+static void __igt_print_description(const char *subtest_name, const char *file, int line)
+{
+	struct description_node *desc;
+	const char indent[] = "  ";
+	bool has_doc = false;
+
+
+	printf("SUB %s %s:%d:\n", subtest_name, file, line);
+
+	igt_list_for_each(desc, &subgroup_descriptions, link) {
+		print_line_wrapping(indent, desc->desc);
+		printf("\n");
+		has_doc = true;
+	}
+
+	if (__current_description[0] != '\0') {
+		print_line_wrapping(indent, __current_description);
+		printf("\n");
+		has_doc = true;
+	}
+
+	if (!has_doc)
+		printf("%sNO DOCUMENTATION!\n\n", indent);
+}
+
 /*
  * Note: Testcases which use these helpers MUST NOT output anything to stdout
  * outside of places protected by igt_run_subtest checks - the piglit
  * runner adds every line to the subtest list.
  */
-bool __igt_run_subtest(const char *subtest_name)
+bool __igt_run_subtest(const char *subtest_name, const char *file, const int line)
 {
 	int i;
 
@@ -954,18 +1055,25 @@ bool __igt_run_subtest(const char *subtest_name)
 			igt_exit();
 		}
 
-	if (list_subtests) {
-		printf("%s\n", subtest_name);
-		return false;
-	}
-
 	if (run_single_subtest) {
-		if (uwildmat(subtest_name, run_single_subtest) == 0)
+		if (uwildmat(subtest_name, run_single_subtest) == 0) {
+			_clear_current_description();
 			return false;
-		else
+		} else {
 			run_single_subtest_found = true;
+		}
 	}
 
+	if (describe_subtests) {
+		__igt_print_description(subtest_name, file, line);
+		_clear_current_description();
+		return false;
+	} else if (list_subtests) {
+		printf("%s\n", subtest_name);
+		return false;
+	}
+
+
 	if (skip_subtests_henceforth) {
 		printf("%sSubtest %s: %s%s\n",
 		       (!__igt_plain_output) ? "\x1b[1m" : "", subtest_name,
@@ -1014,15 +1122,32 @@ bool igt_only_list_subtests(void)
 	return list_subtests;
 }
 
-void __igt_subtest_group_save(int *save)
+
+
+void __igt_subtest_group_save(int *save, int *desc)
 {
 	assert(test_with_subtests);
 
+	if (__current_description[0] != '\0') {
+		struct description_node *new = calloc(1, sizeof(*new));
+		memcpy(new->desc, __current_description, sizeof(__current_description));
+		igt_list_add_tail(&new->link, &subgroup_descriptions);
+		_clear_current_description();
+		*desc = true;
+	}
+
 	*save = skip_subtests_henceforth;
 }
 
-void __igt_subtest_group_restore(int save)
+void __igt_subtest_group_restore(int save, int desc)
 {
+	if (desc) {
+		struct description_node *last =
+			igt_list_last_entry(&subgroup_descriptions, last, link);
+		igt_list_del(&last->link);
+		free(last);
+	}
+
 	skip_subtests_henceforth = save;
 }
 
@@ -1229,6 +1354,34 @@ bool igt_can_fail(void)
 	return !test_with_subtests || in_fixture || in_subtest;
 }
 
+/**
+ * igt_describe_f:
+ * @fmt: format string containing description
+ * @...: argument used by the format string
+ *
+ * Attach a description to the following #igt_subtest or #igt_subtest_group
+ * block.
+ *
+ * Check #igt_describe for more details.
+ *
+ */
+void igt_describe_f(const char *fmt, ...)
+{
+	int ret;
+	va_list args;
+
+	if (!describe_subtests)
+		return;
+
+	va_start(args, fmt);
+
+	ret = vsnprintf(__current_description, sizeof(__current_description), fmt, args);
+
+	va_end(args);
+
+	assert(ret < sizeof(__current_description));
+}
+
 static bool running_under_gdb(void)
 {
 	char pathname[30], buf[1024];
@@ -1540,7 +1693,7 @@ void igt_exit(void)
 		g_key_file_free(igt_key_file);
 
 	if (run_single_subtest && !run_single_subtest_found) {
-		igt_warn("Unknown subtest: %s\n", run_single_subtest);
+		igt_critical("Unknown subtest: %s\n", run_single_subtest);
 		exit(IGT_EXIT_INVALID);
 	}
 
diff --git a/lib/igt_core.h b/lib/igt_core.h
index 88a95ec2..1b37f142 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -174,7 +174,7 @@ int igt_subtest_init_parse_opts(int *argc, char **argv,
 #define igt_subtest_init(argc, argv) \
 	igt_subtest_init_parse_opts(&argc, argv, NULL, NULL, NULL, NULL, NULL);
 
-bool __igt_run_subtest(const char *subtest_name);
+bool __igt_run_subtest(const char *subtest_name, const char *file, const int line);
 #define __igt_tokencat2(x, y) x ## y
 
 /**
@@ -198,14 +198,14 @@ bool __igt_run_subtest(const char *subtest_name);
  *
  * This is a simpler version of igt_subtest_f()
  */
-#define igt_subtest(name) for (; __igt_run_subtest((name)) && \
+#define igt_subtest(name) for (; __igt_run_subtest((name), __FILE__, __LINE__) && \
 				   (sigsetjmp(igt_subtest_jmpbuf, 1) == 0); \
 				   igt_success())
 #define __igt_subtest_f(tmp, format...) \
 	for (char tmp [256]; \
 	     snprintf( tmp , sizeof( tmp ), \
 		      format), \
-	     __igt_run_subtest( tmp ) && \
+	     __igt_run_subtest(tmp, __FILE__, __LINE__) && \
 	     (sigsetjmp(igt_subtest_jmpbuf, 1) == 0); \
 	     igt_success())
 
@@ -227,8 +227,8 @@ bool __igt_run_subtest(const char *subtest_name);
 const char *igt_subtest_name(void);
 bool igt_only_list_subtests(void);
 
-void __igt_subtest_group_save(int *);
-void __igt_subtest_group_restore(int);
+void __igt_subtest_group_save(int *, int *);
+void __igt_subtest_group_restore(int, int);
 /**
  * igt_subtest_group:
  *
@@ -245,11 +245,14 @@ void __igt_subtest_group_restore(int);
  * group will fail or skip. Subtest groups can be arbitrarily nested.
  */
 #define igt_subtest_group for (int igt_tokencat(__tmpint,__LINE__) = 0, \
-			       igt_tokencat(__save,__LINE__) = 0; \
+			       igt_tokencat(__save,__LINE__) = 0, \
+			       igt_tokencat(__desc,__LINE__) = 0; \
 			       igt_tokencat(__tmpint,__LINE__) < 1 && \
-			       (__igt_subtest_group_save(& igt_tokencat(__save,__LINE__) ), true); \
+			       (__igt_subtest_group_save(& igt_tokencat(__save,__LINE__), \
+							 & igt_tokencat(__desc,__LINE__) ), true); \
 			       igt_tokencat(__tmpint,__LINE__) ++, \
-			       __igt_subtest_group_restore(igt_tokencat(__save,__LINE__) ))
+			       __igt_subtest_group_restore(igt_tokencat(__save,__LINE__), \
+							   igt_tokencat(__desc,__LINE__)))
 
 /**
  * igt_main_args:
@@ -385,6 +388,124 @@ static inline void igt_ignore_warn(bool value)
 {
 }
 
+__attribute__((format(printf, 1, 2)))
+void igt_describe_f(const char *fmt, ...);
+
+/**
+ * igt_describe:
+ * @dsc: string containing description
+ *
+ * Attach a description to the following #igt_subtest or #igt_subtest_group
+ * block.
+ *
+ * The description should complement the test/subtest name and provide more
+ * context on what is being tested. It should explain the idea of the test and
+ * do not mention implementation details, so that it never goes out of date.
+ *
+ * DO:
+ *  * focus on the userspace's perspective
+ *  * try to capture the reason for the test's existence
+ *  * be brief
+ *
+ * DON'T:
+ *  * try to translate the code into English
+ *  * explain all the checks the test does
+ *  * delve on the implementation
+ *
+ * Good examples:
+ *  * "make sure that legacy cursor updates do not stall atomic commits"
+ *  * "check that atomic updates of many planes are indeed atomic and take
+ *     effect immediately after the commit"
+ *  * "make sure that the meta-data exposed by the kernel to the userspace
+ *     is correct and matches the used EDID"
+ *
+ * Bad examples:
+ *  * "spawn 10 threads, each pinning cpu core with a busy loop..."
+ *  * "randomly generate holes in a primary plane then try to cover each hole
+ *    with a plane and make sure that CRC matches, do 25 gazillion rounds of
+ *    that..."
+ *
+ *
+ * Resulting #igt_subtest documentation is a concatenation of its own
+ * description and all the parenting #igt_subtest_group descriptions, starting
+ * from the outermost one. Example:
+ *
+ * |[<!-- language="C" -->
+ * #include "igt.h"
+ *
+ * IGT_TEST_DESCRIPTION("Global description of the whole binary");
+ * igt_main
+ * {
+ * 	igt_describe("Desc of the subgroup with A and B");
+ * 	igt_subtest_group {
+ * 		igt_describe("Desc of the subtest A");
+ * 		igt_subtest("subtest-a") {
+ * 			...
+ * 		}
+ *
+ * 		igt_describe("Desc of the subtest B");
+ * 		igt_subtest("subtest-b") {
+ * 			...
+ * 		}
+ * 	}
+ *
+ * 	igt_describe("Desc of the subtest C");
+ * 	igt_subtest("subtest-c") {
+ * 		...
+ * 	}
+ * }
+ * ]|
+ *
+ * It's will accessible via --describe command line switch:
+ *
+ * |[
+ * $ test --describe
+ * Global description of the whole binary
+ *
+ * SUB subtest-a test.c:5:
+ *   Desc of the subgroup with A and B
+ *
+ *   Desc of the subtest A
+ *
+ * SUB subtest-b test.c:10:
+ *   Desc of the subgroup with A and B
+ *
+ *   Desc of the subtest B
+ *
+ * SUB subtest-c test.c:15:
+ *   Desc of the subtest C
+ * ]|
+ *
+ * Every single #igt_subtest does not have to be preceded with a #igt_describe
+ * as long as it has good-enough explanation provided on the #igt_subtest_group
+ * level.
+ *
+ * Example:
+ *
+ * |[<!-- language="C" -->
+ * #include "igt.h"
+ *
+ * igt_main
+ * {
+ * 	igt_describe("check xyz with different tilings");
+ * 	igt_subtest_group {
+ * 		// no need for extra description, group is enough and tiling is
+ * 		// obvious from the test name
+ * 		igt_subtest("foo-tiling-x") {
+ * 			...
+ * 		}
+ *
+ * 		igt_subtest("foo-tiling-y") {
+ * 			...
+ * 		}
+ * 	}
+ * }
+ * ]|
+ *
+ */
+#define igt_describe(dsc) \
+	igt_describe_f("%s", dsc)
+
 /**
  * igt_assert:
  * @expr: condition to test
diff --git a/lib/tests/igt_describe.c b/lib/tests/igt_describe.c
new file mode 100644
index 00000000..a4457fa9
--- /dev/null
+++ b/lib/tests/igt_describe.c
@@ -0,0 +1,280 @@
+/*
+ * Copyright © 2019 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 <sys/wait.h>
+#include <string.h>
+#include <stdlib.h>
+
+#include "drmtest.h"
+#include "igt_tests_common.h"
+
+IGT_TEST_DESCRIPTION("the top level description");
+static void fake_main(int argc, char **argv) {
+	igt_subtest_init(argc, argv);
+
+	igt_describe("Basic A");
+	igt_subtest("A")
+		;
+
+	igt_fixture
+		printf("should not be executed!\n");
+
+	igt_describe("Group with B, C & D");
+	igt_subtest_group {
+		igt_describe("Basic B");
+		igt_subtest("B")
+			;
+
+		if (!igt_only_list_subtests())
+			printf("should not be executed!\n");
+
+		igt_describe("Group with C & D");
+		igt_subtest_group {
+			igt_describe("Basic C");
+			igt_subtest("C")
+				printf("should not be executed!\n");
+
+			// NO DOC
+			igt_subtest("D")
+				;
+		}
+	}
+
+	// NO DOC
+	igt_subtest_group {
+		// NO DOC
+		igt_subtest("E")
+			;
+	}
+
+	// NO DOC
+	igt_subtest("F")
+		;
+
+	igt_describe("this description should be so long that it wraps itself nicely in the terminal "
+		     "this description should be so long that it wraps itself nicely in the terminal "
+		     "this description should be so long that it wraps itself nicely in the terminal "
+		     "this description should be so long that it wraps itself nicely in the terminal "
+		     "this description should be so long that it wraps itself nicely in the terminal "
+		     "this description should be so long that it wraps itself nicely in the terminal");
+	igt_subtest("G")
+		;
+
+	igt_describe("verylongwordthatshoudlbeprintedeventhoughitspastthewrppinglimit"
+		     "verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit "
+		     "verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit"
+		     "verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit");
+	igt_subtest("F")
+		;
+
+	igt_exit();
+}
+
+static const char DESCRIBE_ALL_OUTPUT[] = \
+	"the top level description\n"
+	"\n"
+	"SUB A ../lib/tests/igt_describe.c:36:\n"
+	"  Basic A\n"
+	"\n"
+	"SUB B ../lib/tests/igt_describe.c:45:\n"
+	"  Group with B, C & D\n"
+	"\n"
+	"  Basic B\n"
+	"\n"
+	"SUB C ../lib/tests/igt_describe.c:54:\n"
+	"  Group with B, C & D\n"
+	"\n"
+	"  Group with C & D\n"
+	"\n"
+	"  Basic C\n"
+	"\n"
+	"SUB D ../lib/tests/igt_describe.c:58:\n"
+	"  Group with B, C & D\n"
+	"\n"
+	"  Group with C & D\n"
+	"\n"
+	"SUB E ../lib/tests/igt_describe.c:66:\n"
+	"  NO DOCUMENTATION!\n"
+	"\n"
+	"SUB F ../lib/tests/igt_describe.c:71:\n"
+	"  NO DOCUMENTATION!\n"
+	"\n"
+	"SUB G ../lib/tests/igt_describe.c:80:\n"
+	"  this description should be so long that it wraps itself nicely in the terminal this\n"
+	"  description should be so long that it wraps itself nicely in the terminal this description\n"
+	"  should be so long that it wraps itself nicely in the terminal this description should be so\n"
+	"  long that it wraps itself nicely in the terminal this description should be so long that it\n"
+	"  wraps itself nicely in the terminal this description should be so long that it wraps itself\n"
+	"  nicely in the terminal\n"
+	"\n"
+	"SUB F ../lib/tests/igt_describe.c:87:\n"
+	"  verylongwordthatshoudlbeprintedeventhoughitspastthewrppinglimitverylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit\n"
+	"  verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimitverylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit\n\n";
+
+static const char JUST_C_OUTPUT[] = \
+	"the top level description\n"
+	"\n"
+	"SUB C ../lib/tests/igt_describe.c:54:\n"
+	"  Group with B, C & D\n"
+	"\n"
+	"  Group with C & D\n"
+	"\n"
+	"  Basic C\n"
+	"\n";
+
+static void assert_pipe_empty(int fd)
+{
+	char buf[5];
+	internal_assert(0 == read(fd, buf, sizeof(buf)));
+}
+
+static void read_whole_pipe(int fd, char *buf, size_t buflen)
+{
+	ssize_t readlen;
+	off_t offset;
+
+	offset = 0;
+	while ((readlen = read(fd, buf+offset, buflen-offset))) {
+		if (readlen == -1) {
+			if (errno == EINTR) {
+				continue;
+			} else {
+				printf("read failed with %s\n", strerror(errno));
+				exit(1);
+			}
+		}
+		internal_assert(readlen != -1);
+		offset += readlen;
+	}
+}
+
+static pid_t do_fork(int argc, char **argv, int *out, int *err)
+{
+	int outfd[2], errfd[2];
+	pid_t pid;
+
+	internal_assert(pipe(outfd) != -1);
+	internal_assert(pipe(errfd) != -1);
+
+	pid = fork();
+	internal_assert(pid != -1);
+
+	if (pid == 0) {
+		while (dup2(outfd[1], STDOUT_FILENO) == -1 && errno == EINTR) {}
+		while (dup2(errfd[1], STDERR_FILENO) == -1 && errno == EINTR) {}
+
+		close(outfd[0]);
+		close(outfd[1]);
+		close(errfd[0]);
+		close(errfd[1]);
+
+		fake_main(argc, argv);
+
+		exit(-1);
+	} else {
+		/* close the writing ends */
+		close(outfd[1]);
+		close(errfd[1]);
+
+		*out = outfd[0];
+		*err = errfd[0];
+
+		return pid;
+	}
+}
+
+static int _wait(pid_t pid, int *status) {
+	int ret;
+
+	do {
+		ret = waitpid(pid, status, 0);
+	} while (ret == -1 && errno == EINTR);
+
+	return ret;
+}
+
+int main(int argc, char **argv)
+{
+	char prog[] = "igt_describe";
+	int status;
+	int outfd, errfd;
+
+	/* describe all subtest */ {
+		static char out[4096];
+		char arg[] = "--describe";
+		char *fake_argv[] = {prog, arg};
+		int fake_argc = ARRAY_SIZE(fake_argv);
+
+		pid_t pid = do_fork(fake_argc, fake_argv, &outfd, &errfd);
+
+		read_whole_pipe(outfd, out, sizeof(out));
+		assert_pipe_empty(errfd);
+
+		internal_assert(_wait(pid, &status) != -1);
+		internal_assert(WIFEXITED(status));
+		internal_assert(WEXITSTATUS(status) == IGT_EXIT_SUCCESS);
+		internal_assert(0 == strcmp(DESCRIBE_ALL_OUTPUT, out));
+
+		close(outfd);
+		close(errfd);
+	}
+
+	/* describe C using a pattern */ {
+		static char out[4096];
+		char arg[] = "--describe=C";
+		char *fake_argv[] = {prog, arg};
+		int fake_argc = ARRAY_SIZE(fake_argv);
+
+		pid_t pid = do_fork(fake_argc, fake_argv, &outfd, &errfd);
+
+		read_whole_pipe(outfd, out, sizeof(out));
+		assert_pipe_empty(errfd);
+
+		internal_assert(_wait(pid, &status) != -1);
+		internal_assert(WEXITSTATUS(status) == IGT_EXIT_SUCCESS);
+		internal_assert(0 == strcmp(JUST_C_OUTPUT, out));
+
+		close(outfd);
+		close(errfd);
+	}
+
+	/* fail describing with a bad pattern */ {
+		static char err[4096];
+		char arg[] = "--describe=Z";
+		char *fake_argv[] = {prog, arg};
+		int fake_argc = ARRAY_SIZE(fake_argv);
+
+		pid_t pid = do_fork(fake_argc, fake_argv, &outfd, &errfd);
+
+		read_whole_pipe(errfd, err, sizeof(err));
+
+		internal_assert(_wait(pid, &status) != -1);
+		internal_assert(WEXITSTATUS(status) == IGT_EXIT_INVALID);
+		internal_assert(strstr(err, "Unknown subtest: Z"));
+
+		close(outfd);
+		close(errfd);
+	}
+
+	return 0;
+}
diff --git a/lib/tests/meson.build b/lib/tests/meson.build
index b930ee6e..4d907e34 100644
--- a/lib/tests/meson.build
+++ b/lib/tests/meson.build
@@ -3,6 +3,7 @@ lib_tests = [
 	'igt_can_fail',
 	'igt_can_fail_simple',
 	'igt_conflicting_args',
+	'igt_describe',
 	'igt_edid',
 	'igt_exit_handler',
 	'igt_fork',
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for series starting with [1/5,v2,i-g-t] lib/igt_core: Add support for subtest descriptions (rev2)
  2019-07-01 12:21 [igt-dev] [PATCH i-g-t 1/5] lib/igt_core: Add support for subtest descriptions Arkadiusz Hiler
                   ` (7 preceding siblings ...)
  2019-07-04 11:55 ` [igt-dev] [PATCH 1/5 v2 i-g-t] " Arkadiusz Hiler
@ 2019-07-04 12:48 ` Patchwork
  2019-07-05 17:06 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  9 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2019-07-04 12:48 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: igt-dev

== Series Details ==

Series: series starting with [1/5,v2,i-g-t] lib/igt_core: Add support for subtest descriptions (rev2)
URL   : https://patchwork.freedesktop.org/series/63037/
State : success

== Summary ==

CI Bug Log - changes from IGT_5083 -> IGTPW_3241
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/63037/revisions/2/mbox/

Known issues
------------

  Here are the changes found in IGTPW_3241 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-hsw-4770:        [PASS][1] -> [SKIP][2] ([fdo#109271]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/fi-hsw-4770/igt@i915_pm_rpm@basic-pci-d3-state.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/fi-hsw-4770/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@i915_selftest@live_sanitycheck:
    - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4] ([fdo#107724])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/fi-icl-u3/igt@i915_selftest@live_sanitycheck.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/fi-icl-u3/igt@i915_selftest@live_sanitycheck.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [PASS][5] -> [FAIL][6] ([fdo#109485])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u2:          [PASS][7] -> [FAIL][8] ([fdo#103167])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-blb-e6850:       [PASS][9] -> [INCOMPLETE][10] ([fdo#107718])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/fi-blb-e6850/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/fi-blb-e6850/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485


Participating hosts (54 -> 46)
------------------------------

  Additional (1): fi-apl-guc 
  Missing    (9): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-pnv-d510 fi-icl-y fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * IGT: IGT_5083 -> IGTPW_3241

  CI_DRM_6410: 813df7e0ee11d1a97f45462bd48eaa3757c8c813 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3241: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/
  IGT_5083: 4d710ea530aca69304df37191802476f406746ca @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [igt-dev] [PATCH 1/5 v2 i-g-t] lib/igt_core: Add support for subtest descriptions
  2019-07-04 11:55 ` [igt-dev] [PATCH 1/5 v2 i-g-t] " Arkadiusz Hiler
@ 2019-07-04 13:02   ` Ser, Simon
  0 siblings, 0 replies; 17+ messages in thread
From: Ser, Simon @ 2019-07-04 13:02 UTC (permalink / raw)
  To: Hiler, Arkadiusz, igt-dev; +Cc: Latvala, Petri

On Thu, 2019-07-04 at 14:55 +0300, Arkadiusz Hiler wrote:
> This patch adds igt_description() which attaches a description to the
> following igt_subtest or igt_subtest_group block.
> 
> Descriptions are accessible via './test --describe[=pattern]'
> 
> Subtest description is its own igt_describe as well as igt_describes
> of all the parenting igt_subtest_groups, starting from the outermost
> scope.
> 
> Examples of code and produced outputs are included in
> lib/test/igt_describe.c and as a documentation comment on igt_describe()
> macro.
> 
> v2: address Simon's review
> 
> Cc: Simon Ser <simon.ser@intel.com>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Acked-by: Petri Latvala <petri.latvala@intel.com>

LGTM

Reviewed-by: Simon Ser <simon.ser@intel.com>

> ---
>  delta with v1: https://hiler.eu/p/ab062f76.txt
> 
>  lib/igt_core.c           | 179 +++++++++++++++++++++++--
>  lib/igt_core.h           | 137 +++++++++++++++++--
>  lib/tests/igt_describe.c | 280 +++++++++++++++++++++++++++++++++++++++
>  lib/tests/meson.build    |   1 +
>  4 files changed, 576 insertions(+), 21 deletions(-)
>  create mode 100644 lib/tests/igt_describe.c
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 6b9f0425..425848a2 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -70,6 +70,7 @@
>  #include "igt_sysfs.h"
>  #include "igt_sysrq.h"
>  #include "igt_rc.h"
> +#include "igt_list.h"
>  
>  #define UNW_LOCAL_ONLY
>  #include <libunwind.h>
> @@ -259,6 +260,7 @@ const char *igt_interactive_debug;
>  
>  /* subtests helpers */
>  static bool list_subtests = false;
> +static bool describe_subtests = false;
>  static char *run_single_subtest = NULL;
>  static bool run_single_subtest_found = false;
>  static const char *in_subtest = NULL;
> @@ -271,6 +273,16 @@ static enum {
>  	CONT = 0, SKIP, FAIL
>  } skip_subtests_henceforth = CONT;
>  
> +static char __current_description[512];
> +
> +struct description_node {
> +	char desc[sizeof(__current_description)];
> +	struct igt_list link;
> +};
> +
> +static struct igt_list subgroup_descriptions;
> +
> +
>  bool __igt_plain_output = false;
>  
>  /* fork support state */
> @@ -285,6 +297,7 @@ enum {
>  	 * conflict with core ones
>  	 */
>  	OPT_LIST_SUBTESTS = 500,
> +	OPT_DESCRIBE_SUBTESTS,
>  	OPT_RUN_SUBTEST,
>  	OPT_DESCRIPTION,
>  	OPT_DEBUG,
> @@ -528,10 +541,59 @@ static void common_exit_handler(int sig)
>  	assert(sig != 0 || igt_exit_called);
>  }
>  
> +static void print_line_wrapping(const char *indent, const char *text)
> +{
> +	char *copy, *curr, *next_space;
> +	int current_line_length = 0;
> +	bool done = false;
> +
> +	const int total_line_length = 80;
> +	const int line_length = total_line_length - strlen(indent);
> +
> +	copy = malloc(strlen(text) + 1);
> +	memcpy(copy, text, strlen(text) + 1);
> +
> +	curr = copy;
> +
> +	printf("%s", indent);
> +
> +	while (!done) {
> +		next_space = strchr(curr, ' ');
> +
> +		if (!next_space) { /* no more spaces, print everything that is left */
> +			done = true;
> +			next_space = strchr(curr, '\0');
> +		}
> +
> +		*next_space = '\0';
> +
> +		if ((next_space - curr) + current_line_length > line_length && curr != copy) {
> +			printf("\n%s", indent);
> +			current_line_length = 0;
> +		}
> +
> +		if (current_line_length == 0)
> +			printf("%s", curr); /* first word in a line, don't space out */
> +		else
> +			printf(" %s", curr);
> +
> +		current_line_length += next_space - curr;
> +		curr = next_space + 1;
> +	}
> +
> +	printf("\n");
> +
> +	free(copy);
> +}
> +
> +
>  static void print_test_description(void)
>  {
> -	if (&__igt_test_description)
> -		printf("%s\n", __igt_test_description);
> +	if (&__igt_test_description) {
> +		print_line_wrapping("", __igt_test_description);
> +		if (describe_subtests)
> +			printf("\n");
> +	}
>  }
>  
>  static void print_version(void)
> @@ -558,6 +620,7 @@ static void print_usage(const char *help_str, bool output_on_stderr)
>  		   "  --debug[=log-domain]\n"
>  		   "  --interactive-debug[=domain]\n"
>  		   "  --help-description\n"
> +		   "  --describe\n"
>  		   "  --help|-h\n");
>  	if (help_str)
>  		fprintf(f, "%s\n", help_str);
> @@ -671,6 +734,7 @@ static int common_init(int *argc, char **argv,
>  	int c, option_index = 0, i, x;
>  	static struct option long_options[] = {
>  		{"list-subtests",     no_argument,       NULL, OPT_LIST_SUBTESTS},
> +		{"describe",          optional_argument, NULL, OPT_DESCRIBE_SUBTESTS},
>  		{"run-subtest",       required_argument, NULL, OPT_RUN_SUBTEST},
>  		{"help-description",  no_argument,       NULL, OPT_DESCRIPTION},
>  		{"debug",             optional_argument, NULL, OPT_DEBUG},
> @@ -687,6 +751,7 @@ static int common_init(int *argc, char **argv,
>  	int ret = 0;
>  
>  	common_init_env();
> +	igt_list_init(&subgroup_descriptions);
>  
>  	command_str = argv[0];
>  	if (strrchr(command_str, '/'))
> @@ -777,6 +842,13 @@ static int common_init(int *argc, char **argv,
>  			if (!run_single_subtest)
>  				list_subtests = true;
>  			break;
> +		case OPT_DESCRIBE_SUBTESTS:
> +			if (optarg)
> +				run_single_subtest = strdup(optarg);
> +			list_subtests = true;
> +			describe_subtests = true;
> +			print_test_description();
> +			break;
>  		case OPT_RUN_SUBTEST:
>  			assert(optarg);
>  			if (!list_subtests)
> @@ -934,12 +1006,41 @@ void igt_simple_init_parse_opts(int *argc, char **argv,
>  		    extra_opt_handler, handler_data);
>  }
>  
> +static void _clear_current_description(void) {
> +	__current_description[0] = '\0';
> +}
> +
> +static void __igt_print_description(const char *subtest_name, const char *file, int line)
> +{
> +	struct description_node *desc;
> +	const char indent[] = "  ";
> +	bool has_doc = false;
> +
> +
> +	printf("SUB %s %s:%d:\n", subtest_name, file, line);
> +
> +	igt_list_for_each(desc, &subgroup_descriptions, link) {
> +		print_line_wrapping(indent, desc->desc);
> +		printf("\n");
> +		has_doc = true;
> +	}
> +
> +	if (__current_description[0] != '\0') {
> +		print_line_wrapping(indent, __current_description);
> +		printf("\n");
> +		has_doc = true;
> +	}
> +
> +	if (!has_doc)
> +		printf("%sNO DOCUMENTATION!\n\n", indent);
> +}
> +
>  /*
>   * Note: Testcases which use these helpers MUST NOT output anything to stdout
>   * outside of places protected by igt_run_subtest checks - the piglit
>   * runner adds every line to the subtest list.
>   */
> -bool __igt_run_subtest(const char *subtest_name)
> +bool __igt_run_subtest(const char *subtest_name, const char *file, const int line)
>  {
>  	int i;
>  
> @@ -954,18 +1055,25 @@ bool __igt_run_subtest(const char *subtest_name)
>  			igt_exit();
>  		}
>  
> -	if (list_subtests) {
> -		printf("%s\n", subtest_name);
> -		return false;
> -	}
> -
>  	if (run_single_subtest) {
> -		if (uwildmat(subtest_name, run_single_subtest) == 0)
> +		if (uwildmat(subtest_name, run_single_subtest) == 0) {
> +			_clear_current_description();
>  			return false;
> -		else
> +		} else {
>  			run_single_subtest_found = true;
> +		}
>  	}
>  
> +	if (describe_subtests) {
> +		__igt_print_description(subtest_name, file, line);
> +		_clear_current_description();
> +		return false;
> +	} else if (list_subtests) {
> +		printf("%s\n", subtest_name);
> +		return false;
> +	}
> +
> +
>  	if (skip_subtests_henceforth) {
>  		printf("%sSubtest %s: %s%s\n",
>  		       (!__igt_plain_output) ? "\x1b[1m" : "", subtest_name,
> @@ -1014,15 +1122,32 @@ bool igt_only_list_subtests(void)
>  	return list_subtests;
>  }
>  
> -void __igt_subtest_group_save(int *save)
> +
> +
> +void __igt_subtest_group_save(int *save, int *desc)
>  {
>  	assert(test_with_subtests);
>  
> +	if (__current_description[0] != '\0') {
> +		struct description_node *new = calloc(1, sizeof(*new));
> +		memcpy(new->desc, __current_description, sizeof(__current_description));
> +		igt_list_add_tail(&new->link, &subgroup_descriptions);
> +		_clear_current_description();
> +		*desc = true;
> +	}
> +
>  	*save = skip_subtests_henceforth;
>  }
>  
> -void __igt_subtest_group_restore(int save)
> +void __igt_subtest_group_restore(int save, int desc)
>  {
> +	if (desc) {
> +		struct description_node *last =
> +			igt_list_last_entry(&subgroup_descriptions, last, link);
> +		igt_list_del(&last->link);
> +		free(last);
> +	}
> +
>  	skip_subtests_henceforth = save;
>  }
>  
> @@ -1229,6 +1354,34 @@ bool igt_can_fail(void)
>  	return !test_with_subtests || in_fixture || in_subtest;
>  }
>  
> +/**
> + * igt_describe_f:
> + * @fmt: format string containing description
> + * @...: argument used by the format string
> + *
> + * Attach a description to the following #igt_subtest or #igt_subtest_group
> + * block.
> + *
> + * Check #igt_describe for more details.
> + *
> + */
> +void igt_describe_f(const char *fmt, ...)
> +{
> +	int ret;
> +	va_list args;
> +
> +	if (!describe_subtests)
> +		return;
> +
> +	va_start(args, fmt);
> +
> +	ret = vsnprintf(__current_description, sizeof(__current_description), fmt, args);
> +
> +	va_end(args);
> +
> +	assert(ret < sizeof(__current_description));
> +}
> +
>  static bool running_under_gdb(void)
>  {
>  	char pathname[30], buf[1024];
> @@ -1540,7 +1693,7 @@ void igt_exit(void)
>  		g_key_file_free(igt_key_file);
>  
>  	if (run_single_subtest && !run_single_subtest_found) {
> -		igt_warn("Unknown subtest: %s\n", run_single_subtest);
> +		igt_critical("Unknown subtest: %s\n", run_single_subtest);
>  		exit(IGT_EXIT_INVALID);
>  	}
>  
> diff --git a/lib/igt_core.h b/lib/igt_core.h
> index 88a95ec2..1b37f142 100644
> --- a/lib/igt_core.h
> +++ b/lib/igt_core.h
> @@ -174,7 +174,7 @@ int igt_subtest_init_parse_opts(int *argc, char **argv,
>  #define igt_subtest_init(argc, argv) \
>  	igt_subtest_init_parse_opts(&argc, argv, NULL, NULL, NULL, NULL, NULL);
>  
> -bool __igt_run_subtest(const char *subtest_name);
> +bool __igt_run_subtest(const char *subtest_name, const char *file, const int line);
>  #define __igt_tokencat2(x, y) x ## y
>  
>  /**
> @@ -198,14 +198,14 @@ bool __igt_run_subtest(const char *subtest_name);
>   *
>   * This is a simpler version of igt_subtest_f()
>   */
> -#define igt_subtest(name) for (; __igt_run_subtest((name)) && \
> +#define igt_subtest(name) for (; __igt_run_subtest((name), __FILE__, __LINE__) && \
>  				   (sigsetjmp(igt_subtest_jmpbuf, 1) == 0); \
>  				   igt_success())
>  #define __igt_subtest_f(tmp, format...) \
>  	for (char tmp [256]; \
>  	     snprintf( tmp , sizeof( tmp ), \
>  		      format), \
> -	     __igt_run_subtest( tmp ) && \
> +	     __igt_run_subtest(tmp, __FILE__, __LINE__) && \
>  	     (sigsetjmp(igt_subtest_jmpbuf, 1) == 0); \
>  	     igt_success())
>  
> @@ -227,8 +227,8 @@ bool __igt_run_subtest(const char *subtest_name);
>  const char *igt_subtest_name(void);
>  bool igt_only_list_subtests(void);
>  
> -void __igt_subtest_group_save(int *);
> -void __igt_subtest_group_restore(int);
> +void __igt_subtest_group_save(int *, int *);
> +void __igt_subtest_group_restore(int, int);
>  /**
>   * igt_subtest_group:
>   *
> @@ -245,11 +245,14 @@ void __igt_subtest_group_restore(int);
>   * group will fail or skip. Subtest groups can be arbitrarily nested.
>   */
>  #define igt_subtest_group for (int igt_tokencat(__tmpint,__LINE__) = 0, \
> -			       igt_tokencat(__save,__LINE__) = 0; \
> +			       igt_tokencat(__save,__LINE__) = 0, \
> +			       igt_tokencat(__desc,__LINE__) = 0; \
>  			       igt_tokencat(__tmpint,__LINE__) < 1 && \
> -			       (__igt_subtest_group_save(& igt_tokencat(__save,__LINE__) ), true); \
> +			       (__igt_subtest_group_save(& igt_tokencat(__save,__LINE__), \
> +							 & igt_tokencat(__desc,__LINE__) ), true); \
>  			       igt_tokencat(__tmpint,__LINE__) ++, \
> -			       __igt_subtest_group_restore(igt_tokencat(__save,__LINE__) ))
> +			       __igt_subtest_group_restore(igt_tokencat(__save,__LINE__), \
> +							   igt_tokencat(__desc,__LINE__)))
>  
>  /**
>   * igt_main_args:
> @@ -385,6 +388,124 @@ static inline void igt_ignore_warn(bool value)
>  {
>  }
>  
> +__attribute__((format(printf, 1, 2)))
> +void igt_describe_f(const char *fmt, ...);
> +
> +/**
> + * igt_describe:
> + * @dsc: string containing description
> + *
> + * Attach a description to the following #igt_subtest or #igt_subtest_group
> + * block.
> + *
> + * The description should complement the test/subtest name and provide more
> + * context on what is being tested. It should explain the idea of the test and
> + * do not mention implementation details, so that it never goes out of date.
> + *
> + * DO:
> + *  * focus on the userspace's perspective
> + *  * try to capture the reason for the test's existence
> + *  * be brief
> + *
> + * DON'T:
> + *  * try to translate the code into English
> + *  * explain all the checks the test does
> + *  * delve on the implementation
> + *
> + * Good examples:
> + *  * "make sure that legacy cursor updates do not stall atomic commits"
> + *  * "check that atomic updates of many planes are indeed atomic and take
> + *     effect immediately after the commit"
> + *  * "make sure that the meta-data exposed by the kernel to the userspace
> + *     is correct and matches the used EDID"
> + *
> + * Bad examples:
> + *  * "spawn 10 threads, each pinning cpu core with a busy loop..."
> + *  * "randomly generate holes in a primary plane then try to cover each hole
> + *    with a plane and make sure that CRC matches, do 25 gazillion rounds of
> + *    that..."
> + *
> + *
> + * Resulting #igt_subtest documentation is a concatenation of its own
> + * description and all the parenting #igt_subtest_group descriptions, starting
> + * from the outermost one. Example:
> + *
> + * |[<!-- language="C" -->
> + * #include "igt.h"
> + *
> + * IGT_TEST_DESCRIPTION("Global description of the whole binary");
> + * igt_main
> + * {
> + * 	igt_describe("Desc of the subgroup with A and B");
> + * 	igt_subtest_group {
> + * 		igt_describe("Desc of the subtest A");
> + * 		igt_subtest("subtest-a") {
> + * 			...
> + * 		}
> + *
> + * 		igt_describe("Desc of the subtest B");
> + * 		igt_subtest("subtest-b") {
> + * 			...
> + * 		}
> + * 	}
> + *
> + * 	igt_describe("Desc of the subtest C");
> + * 	igt_subtest("subtest-c") {
> + * 		...
> + * 	}
> + * }
> + * ]|
> + *
> + * It's will accessible via --describe command line switch:
> + *
> + * |[
> + * $ test --describe
> + * Global description of the whole binary
> + *
> + * SUB subtest-a test.c:5:
> + *   Desc of the subgroup with A and B
> + *
> + *   Desc of the subtest A
> + *
> + * SUB subtest-b test.c:10:
> + *   Desc of the subgroup with A and B
> + *
> + *   Desc of the subtest B
> + *
> + * SUB subtest-c test.c:15:
> + *   Desc of the subtest C
> + * ]|
> + *
> + * Every single #igt_subtest does not have to be preceded with a #igt_describe
> + * as long as it has good-enough explanation provided on the #igt_subtest_group
> + * level.
> + *
> + * Example:
> + *
> + * |[<!-- language="C" -->
> + * #include "igt.h"
> + *
> + * igt_main
> + * {
> + * 	igt_describe("check xyz with different tilings");
> + * 	igt_subtest_group {
> + * 		// no need for extra description, group is enough and tiling is
> + * 		// obvious from the test name
> + * 		igt_subtest("foo-tiling-x") {
> + * 			...
> + * 		}
> + *
> + * 		igt_subtest("foo-tiling-y") {
> + * 			...
> + * 		}
> + * 	}
> + * }
> + * ]|
> + *
> + */
> +#define igt_describe(dsc) \
> +	igt_describe_f("%s", dsc)
> +
>  /**
>   * igt_assert:
>   * @expr: condition to test
> diff --git a/lib/tests/igt_describe.c b/lib/tests/igt_describe.c
> new file mode 100644
> index 00000000..a4457fa9
> --- /dev/null
> +++ b/lib/tests/igt_describe.c
> @@ -0,0 +1,280 @@
> +/*
> + * Copyright © 2019 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 <sys/wait.h>
> +#include <string.h>
> +#include <stdlib.h>
> +
> +#include "drmtest.h"
> +#include "igt_tests_common.h"
> +
> +IGT_TEST_DESCRIPTION("the top level description");
> +static void fake_main(int argc, char **argv) {
> +	igt_subtest_init(argc, argv);
> +
> +	igt_describe("Basic A");
> +	igt_subtest("A")
> +		;
> +
> +	igt_fixture
> +		printf("should not be executed!\n");
> +
> +	igt_describe("Group with B, C & D");
> +	igt_subtest_group {
> +		igt_describe("Basic B");
> +		igt_subtest("B")
> +			;
> +
> +		if (!igt_only_list_subtests())
> +			printf("should not be executed!\n");
> +
> +		igt_describe("Group with C & D");
> +		igt_subtest_group {
> +			igt_describe("Basic C");
> +			igt_subtest("C")
> +				printf("should not be executed!\n");
> +
> +			// NO DOC
> +			igt_subtest("D")
> +				;
> +		}
> +	}
> +
> +	// NO DOC
> +	igt_subtest_group {
> +		// NO DOC
> +		igt_subtest("E")
> +			;
> +	}
> +
> +	// NO DOC
> +	igt_subtest("F")
> +		;
> +
> +	igt_describe("this description should be so long that it wraps itself nicely in the terminal "
> +		     "this description should be so long that it wraps itself nicely in the terminal "
> +		     "this description should be so long that it wraps itself nicely in the terminal "
> +		     "this description should be so long that it wraps itself nicely in the terminal "
> +		     "this description should be so long that it wraps itself nicely in the terminal "
> +		     "this description should be so long that it wraps itself nicely in the terminal");
> +	igt_subtest("G")
> +		;
> +
> +	igt_describe("verylongwordthatshoudlbeprintedeventhoughitspastthewrppinglimit"
> +		     "verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit "
> +		     "verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit"
> +		     "verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit");
> +	igt_subtest("F")
> +		;
> +
> +	igt_exit();
> +}
> +
> +static const char DESCRIBE_ALL_OUTPUT[] = \
> +	"the top level description\n"
> +	"\n"
> +	"SUB A ../lib/tests/igt_describe.c:36:\n"
> +	"  Basic A\n"
> +	"\n"
> +	"SUB B ../lib/tests/igt_describe.c:45:\n"
> +	"  Group with B, C & D\n"
> +	"\n"
> +	"  Basic B\n"
> +	"\n"
> +	"SUB C ../lib/tests/igt_describe.c:54:\n"
> +	"  Group with B, C & D\n"
> +	"\n"
> +	"  Group with C & D\n"
> +	"\n"
> +	"  Basic C\n"
> +	"\n"
> +	"SUB D ../lib/tests/igt_describe.c:58:\n"
> +	"  Group with B, C & D\n"
> +	"\n"
> +	"  Group with C & D\n"
> +	"\n"
> +	"SUB E ../lib/tests/igt_describe.c:66:\n"
> +	"  NO DOCUMENTATION!\n"
> +	"\n"
> +	"SUB F ../lib/tests/igt_describe.c:71:\n"
> +	"  NO DOCUMENTATION!\n"
> +	"\n"
> +	"SUB G ../lib/tests/igt_describe.c:80:\n"
> +	"  this description should be so long that it wraps itself nicely in the terminal this\n"
> +	"  description should be so long that it wraps itself nicely in the terminal this description\n"
> +	"  should be so long that it wraps itself nicely in the terminal this description should be so\n"
> +	"  long that it wraps itself nicely in the terminal this description should be so long that it\n"
> +	"  wraps itself nicely in the terminal this description should be so long that it wraps itself\n"
> +	"  nicely in the terminal\n"
> +	"\n"
> +	"SUB F ../lib/tests/igt_describe.c:87:\n"
> +	"  verylongwordthatshoudlbeprintedeventhoughitspastthewrppinglimitverylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit\n"
> +	"  verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimitverylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit\n\n";
> +
> +static const char JUST_C_OUTPUT[] = \
> +	"the top level description\n"
> +	"\n"
> +	"SUB C ../lib/tests/igt_describe.c:54:\n"
> +	"  Group with B, C & D\n"
> +	"\n"
> +	"  Group with C & D\n"
> +	"\n"
> +	"  Basic C\n"
> +	"\n";
> +
> +static void assert_pipe_empty(int fd)
> +{
> +	char buf[5];
> +	internal_assert(0 == read(fd, buf, sizeof(buf)));
> +}
> +
> +static void read_whole_pipe(int fd, char *buf, size_t buflen)
> +{
> +	ssize_t readlen;
> +	off_t offset;
> +
> +	offset = 0;
> +	while ((readlen = read(fd, buf+offset, buflen-offset))) {
> +		if (readlen == -1) {
> +			if (errno == EINTR) {
> +				continue;
> +			} else {
> +				printf("read failed with %s\n", strerror(errno));
> +				exit(1);
> +			}
> +		}
> +		internal_assert(readlen != -1);
> +		offset += readlen;
> +	}
> +}
> +
> +static pid_t do_fork(int argc, char **argv, int *out, int *err)
> +{
> +	int outfd[2], errfd[2];
> +	pid_t pid;
> +
> +	internal_assert(pipe(outfd) != -1);
> +	internal_assert(pipe(errfd) != -1);
> +
> +	pid = fork();
> +	internal_assert(pid != -1);
> +
> +	if (pid == 0) {
> +		while (dup2(outfd[1], STDOUT_FILENO) == -1 && errno == EINTR) {}
> +		while (dup2(errfd[1], STDERR_FILENO) == -1 && errno == EINTR) {}
> +
> +		close(outfd[0]);
> +		close(outfd[1]);
> +		close(errfd[0]);
> +		close(errfd[1]);
> +
> +		fake_main(argc, argv);
> +
> +		exit(-1);
> +	} else {
> +		/* close the writing ends */
> +		close(outfd[1]);
> +		close(errfd[1]);
> +
> +		*out = outfd[0];
> +		*err = errfd[0];
> +
> +		return pid;
> +	}
> +}
> +
> +static int _wait(pid_t pid, int *status) {
> +	int ret;
> +
> +	do {
> +		ret = waitpid(pid, status, 0);
> +	} while (ret == -1 && errno == EINTR);
> +
> +	return ret;
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	char prog[] = "igt_describe";
> +	int status;
> +	int outfd, errfd;
> +
> +	/* describe all subtest */ {
> +		static char out[4096];
> +		char arg[] = "--describe";
> +		char *fake_argv[] = {prog, arg};
> +		int fake_argc = ARRAY_SIZE(fake_argv);
> +
> +		pid_t pid = do_fork(fake_argc, fake_argv, &outfd, &errfd);
> +
> +		read_whole_pipe(outfd, out, sizeof(out));
> +		assert_pipe_empty(errfd);
> +
> +		internal_assert(_wait(pid, &status) != -1);
> +		internal_assert(WIFEXITED(status));
> +		internal_assert(WEXITSTATUS(status) == IGT_EXIT_SUCCESS);
> +		internal_assert(0 == strcmp(DESCRIBE_ALL_OUTPUT, out));
> +
> +		close(outfd);
> +		close(errfd);
> +	}
> +
> +	/* describe C using a pattern */ {
> +		static char out[4096];
> +		char arg[] = "--describe=C";
> +		char *fake_argv[] = {prog, arg};
> +		int fake_argc = ARRAY_SIZE(fake_argv);
> +
> +		pid_t pid = do_fork(fake_argc, fake_argv, &outfd, &errfd);
> +
> +		read_whole_pipe(outfd, out, sizeof(out));
> +		assert_pipe_empty(errfd);
> +
> +		internal_assert(_wait(pid, &status) != -1);
> +		internal_assert(WEXITSTATUS(status) == IGT_EXIT_SUCCESS);
> +		internal_assert(0 == strcmp(JUST_C_OUTPUT, out));
> +
> +		close(outfd);
> +		close(errfd);
> +	}
> +
> +	/* fail describing with a bad pattern */ {
> +		static char err[4096];
> +		char arg[] = "--describe=Z";
> +		char *fake_argv[] = {prog, arg};
> +		int fake_argc = ARRAY_SIZE(fake_argv);
> +
> +		pid_t pid = do_fork(fake_argc, fake_argv, &outfd, &errfd);
> +
> +		read_whole_pipe(errfd, err, sizeof(err));
> +
> +		internal_assert(_wait(pid, &status) != -1);
> +		internal_assert(WEXITSTATUS(status) == IGT_EXIT_INVALID);
> +		internal_assert(strstr(err, "Unknown subtest: Z"));
> +
> +		close(outfd);
> +		close(errfd);
> +	}
> +
> +	return 0;
> +}
> diff --git a/lib/tests/meson.build b/lib/tests/meson.build
> index b930ee6e..4d907e34 100644
> --- a/lib/tests/meson.build
> +++ b/lib/tests/meson.build
> @@ -3,6 +3,7 @@ lib_tests = [
>  	'igt_can_fail',
>  	'igt_can_fail_simple',
>  	'igt_conflicting_args',
> +	'igt_describe',
>  	'igt_edid',
>  	'igt_exit_handler',
>  	'igt_fork',
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [igt-dev] ✓ Fi.CI.IGT: success for series starting with [1/5,v2,i-g-t] lib/igt_core: Add support for subtest descriptions (rev2)
  2019-07-01 12:21 [igt-dev] [PATCH i-g-t 1/5] lib/igt_core: Add support for subtest descriptions Arkadiusz Hiler
                   ` (8 preceding siblings ...)
  2019-07-04 12:48 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [1/5,v2,i-g-t] lib/igt_core: Add support for subtest descriptions (rev2) Patchwork
@ 2019-07-05 17:06 ` Patchwork
  9 siblings, 0 replies; 17+ messages in thread
From: Patchwork @ 2019-07-05 17:06 UTC (permalink / raw)
  To: Arkadiusz Hiler; +Cc: igt-dev

== Series Details ==

Series: series starting with [1/5,v2,i-g-t] lib/igt_core: Add support for subtest descriptions (rev2)
URL   : https://patchwork.freedesktop.org/series/63037/
State : success

== Summary ==

CI Bug Log - changes from IGT_5083_full -> IGTPW_3241_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/63037/revisions/2/mbox/

Known issues
------------

  Here are the changes found in IGTPW_3241_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_busy@close-race:
    - shard-hsw:          [PASS][1] -> [DMESG-FAIL][2] ([fdo#111063])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-hsw4/igt@gem_busy@close-race.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-hsw4/igt@gem_busy@close-race.html
    - shard-apl:          [PASS][3] -> [DMESG-FAIL][4] ([fdo#111063])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-apl4/igt@gem_busy@close-race.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-apl3/igt@gem_busy@close-race.html

  * igt@gem_exec_balancer@smoke:
    - shard-iclb:         [PASS][5] -> [SKIP][6] ([fdo#110854])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-iclb1/igt@gem_exec_balancer@smoke.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-iclb8/igt@gem_exec_balancer@smoke.html

  * igt@i915_pm_rpm@i2c:
    - shard-iclb:         [PASS][7] -> [FAIL][8] ([fdo#104097])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-iclb7/igt@i915_pm_rpm@i2c.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-iclb8/igt@i915_pm_rpm@i2c.html

  * igt@kms_cursor_crc@pipe-c-cursor-alpha-opaque:
    - shard-kbl:          [PASS][9] -> [FAIL][10] ([fdo#103232])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-kbl6/igt@kms_cursor_crc@pipe-c-cursor-alpha-opaque.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-kbl7/igt@kms_cursor_crc@pipe-c-cursor-alpha-opaque.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt:
    - shard-glk:          [PASS][11] -> [FAIL][12] ([fdo#103167])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-glk5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-glk1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt.html
    - shard-apl:          [PASS][13] -> [FAIL][14] ([fdo#103167])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-apl5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-apl7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt.html
    - shard-kbl:          [PASS][15] -> [FAIL][16] ([fdo#103167])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-kbl1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
    - shard-snb:          [PASS][17] -> [SKIP][18] ([fdo#109271]) +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-snb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-snb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-pwrite:
    - shard-iclb:         [PASS][19] -> [FAIL][20] ([fdo#103167]) +2 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-pwrite.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-pwrite.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [PASS][21] -> [SKIP][22] ([fdo#109441]) +2 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-iclb4/igt@kms_psr@psr2_sprite_plane_move.html

  
#### Possible fixes ####

  * igt@gem_workarounds@suspend-resume-context:
    - shard-apl:          [DMESG-WARN][23] ([fdo#108566]) -> [PASS][24] +1 similar issue
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-apl6/igt@gem_workarounds@suspend-resume-context.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-apl3/igt@gem_workarounds@suspend-resume-context.html

  * igt@i915_pm_rc6_residency@rc6-accuracy:
    - shard-kbl:          [SKIP][25] ([fdo#109271]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-kbl2/igt@i915_pm_rc6_residency@rc6-accuracy.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-kbl7/igt@i915_pm_rc6_residency@rc6-accuracy.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-kbl:          [DMESG-WARN][27] ([fdo#108566]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-kbl6/igt@i915_suspend@fence-restore-tiled2untiled.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-kbl7/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic:
    - shard-hsw:          [FAIL][29] ([fdo#103355]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-hsw1/igt@kms_cursor_legacy@cursor-vs-flip-atomic.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-hsw8/igt@kms_cursor_legacy@cursor-vs-flip-atomic.html

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          [FAIL][31] ([fdo#105363]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-glk3/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-glk1/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@2x-modeset-vs-vblank-race-interruptible:
    - shard-glk:          [FAIL][33] ([fdo#103060]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-glk3/igt@kms_flip@2x-modeset-vs-vblank-race-interruptible.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-glk1/igt@kms_flip@2x-modeset-vs-vblank-race-interruptible.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt:
    - shard-iclb:         [FAIL][35] ([fdo#103167]) -> [PASS][36] +7 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-iclb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [FAIL][37] ([fdo#103166]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-iclb4/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [SKIP][39] ([fdo#109642] / [fdo#111068]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-iclb1/igt@kms_psr2_su@page_flip.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-iclb2/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@psr2_suspend:
    - shard-iclb:         [SKIP][41] ([fdo#109441]) -> [PASS][42] +2 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-iclb4/igt@kms_psr@psr2_suspend.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-iclb2/igt@kms_psr@psr2_suspend.html

  * igt@perf_pmu@rc6-runtime-pm:
    - shard-kbl:          [FAIL][43] ([fdo#105010]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-kbl7/igt@perf_pmu@rc6-runtime-pm.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-kbl7/igt@perf_pmu@rc6-runtime-pm.html

  
#### Warnings ####

  * igt@gem_ctx_sseu@engines:
    - shard-hsw:          [SKIP][45] ([fdo#109271]) -> [INCOMPLETE][46] ([fdo#103540])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5083/shard-hsw5/igt@gem_ctx_sseu@engines.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/shard-hsw8/igt@gem_ctx_sseu@engines.html

  
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#104097]: https://bugs.freedesktop.org/show_bug.cgi?id=104097
  [fdo#105010]: https://bugs.freedesktop.org/show_bug.cgi?id=105010
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110854]: https://bugs.freedesktop.org/show_bug.cgi?id=110854
  [fdo#111063]: https://bugs.freedesktop.org/show_bug.cgi?id=111063
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068


Participating hosts (7 -> 6)
------------------------------

  Missing    (1): shard-skl 


Build changes
-------------

  * IGT: IGT_5083 -> IGTPW_3241

  CI_DRM_6410: 813df7e0ee11d1a97f45462bd48eaa3757c8c813 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3241: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/
  IGT_5083: 4d710ea530aca69304df37191802476f406746ca @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3241/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [igt-dev] [PATCH i-g-t 1/5] lib/igt_core: Add support for subtest descriptions
@ 2019-06-17 10:54 Arkadiusz Hiler
  0 siblings, 0 replies; 17+ messages in thread
From: Arkadiusz Hiler @ 2019-06-17 10:54 UTC (permalink / raw)
  To: igt-dev; +Cc: Petri Latvala

This patch adds igt_description() which attaches a description to the
following igt_subtest or igt_subtest_group block.

Descriptions are accessible via './test --describe[=pattern]'

Subtest description is its own igt_describe as well as igt_describes
of all the parenting igt_subtest_groups, starting from the outermost
scope.

Examples of code and produced outputs are included in
lib/test/igt_describe.c and as a documentation comment on igt_describe()
macro.

Cc: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 lib/igt_core.c           | 179 ++++++++++++++++++++++++--
 lib/igt_core.h           | 137 ++++++++++++++++++--
 lib/tests/igt_describe.c | 266 +++++++++++++++++++++++++++++++++++++++
 lib/tests/meson.build    |   1 +
 4 files changed, 562 insertions(+), 21 deletions(-)
 create mode 100644 lib/tests/igt_describe.c

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 6b9f0425..8b1c7b2f 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -70,6 +70,7 @@
 #include "igt_sysfs.h"
 #include "igt_sysrq.h"
 #include "igt_rc.h"
+#include "igt_list.h"
 
 #define UNW_LOCAL_ONLY
 #include <libunwind.h>
@@ -259,6 +260,7 @@ const char *igt_interactive_debug;
 
 /* subtests helpers */
 static bool list_subtests = false;
+static bool describe_subtests = false;
 static char *run_single_subtest = NULL;
 static bool run_single_subtest_found = false;
 static const char *in_subtest = NULL;
@@ -271,6 +273,16 @@ static enum {
 	CONT = 0, SKIP, FAIL
 } skip_subtests_henceforth = CONT;
 
+static char __current_description[4096];
+
+struct description_node {
+	char desc[sizeof(__current_description)];
+	struct igt_list link;
+};
+
+static struct igt_list subgroup_descriptions;
+
+
 bool __igt_plain_output = false;
 
 /* fork support state */
@@ -285,6 +297,7 @@ enum {
 	 * conflict with core ones
 	 */
 	OPT_LIST_SUBTESTS = 500,
+	OPT_DESCRIBE_SUBTESTS,
 	OPT_RUN_SUBTEST,
 	OPT_DESCRIPTION,
 	OPT_DEBUG,
@@ -528,10 +541,59 @@ static void common_exit_handler(int sig)
 	assert(sig != 0 || igt_exit_called);
 }
 
+static void print_line_wrapping(const char *indent, const char *text)
+{
+	char *copy, *curr, *next_space;
+	int current_line_length = 0;
+	bool done = false;
+
+	const int total_line_length = 80;
+	const int line_length = total_line_length - strlen(indent);
+
+	copy = malloc(strlen(text) + 1);
+	memcpy(copy, text, strlen(text) + 1);
+
+	curr = copy;
+
+	printf("%s", indent);
+
+	while (!done) {
+		next_space = strchr(curr, ' ');
+
+		if (!next_space) { /* no more spaces, print everything that is left */
+			done = true;
+			next_space = strchr(curr, '\0');
+		}
+
+		*next_space = '\0';
+
+		if ((next_space - curr) + current_line_length > line_length && curr != copy) {
+			printf("\n%s", indent);
+			current_line_length = 0;
+		}
+
+		if (current_line_length == 0)
+			printf("%s", curr); /* first word in a line, don't space out */
+		else
+			printf(" %s", curr);
+
+		current_line_length += next_space - curr;
+		curr = next_space + 1;
+	}
+
+	printf("\n");
+
+	free(copy);
+}
+
+
 static void print_test_description(void)
 {
-	if (&__igt_test_description)
-		printf("%s\n", __igt_test_description);
+	if (&__igt_test_description) {
+		print_line_wrapping("", __igt_test_description);
+		if (describe_subtests)
+			printf("\n");
+	}
 }
 
 static void print_version(void)
@@ -558,6 +620,7 @@ static void print_usage(const char *help_str, bool output_on_stderr)
 		   "  --debug[=log-domain]\n"
 		   "  --interactive-debug[=domain]\n"
 		   "  --help-description\n"
+		   "  --describe\n"
 		   "  --help|-h\n");
 	if (help_str)
 		fprintf(f, "%s\n", help_str);
@@ -671,6 +734,7 @@ static int common_init(int *argc, char **argv,
 	int c, option_index = 0, i, x;
 	static struct option long_options[] = {
 		{"list-subtests",     no_argument,       NULL, OPT_LIST_SUBTESTS},
+		{"describe",          optional_argument, NULL, OPT_DESCRIBE_SUBTESTS},
 		{"run-subtest",       required_argument, NULL, OPT_RUN_SUBTEST},
 		{"help-description",  no_argument,       NULL, OPT_DESCRIPTION},
 		{"debug",             optional_argument, NULL, OPT_DEBUG},
@@ -687,6 +751,7 @@ static int common_init(int *argc, char **argv,
 	int ret = 0;
 
 	common_init_env();
+	igt_list_init(&subgroup_descriptions);
 
 	command_str = argv[0];
 	if (strrchr(command_str, '/'))
@@ -777,6 +842,13 @@ static int common_init(int *argc, char **argv,
 			if (!run_single_subtest)
 				list_subtests = true;
 			break;
+		case OPT_DESCRIBE_SUBTESTS:
+			if (optarg)
+				run_single_subtest = strdup(optarg);
+			list_subtests = true;
+			describe_subtests = true;
+			print_test_description();
+			break;
 		case OPT_RUN_SUBTEST:
 			assert(optarg);
 			if (!list_subtests)
@@ -934,12 +1006,41 @@ void igt_simple_init_parse_opts(int *argc, char **argv,
 		    extra_opt_handler, handler_data);
 }
 
+static void _clear_current_description(void) {
+	__current_description[0] = '\0';
+}
+
+#define __INDENT "  "
+static void __igt_print_description(const char *subtest_name, const char *file, const int line)
+{
+	struct description_node *desc;
+	bool has_doc = false;
+
+	printf("SUB %s %s:%d:\n", subtest_name, file, line);
+
+	igt_list_for_each(desc, &subgroup_descriptions, link) {
+		print_line_wrapping(__INDENT, desc->desc);
+		printf("\n");
+		has_doc = true;
+	}
+
+	if (__current_description[0] != '\0') {
+		print_line_wrapping(__INDENT, __current_description);
+		printf("\n");
+		has_doc = true;
+	}
+
+	if (!has_doc)
+		printf("%sNO DOCUMENTATION!\n\n", __INDENT);
+}
+#undef __INDENT
+
 /*
  * Note: Testcases which use these helpers MUST NOT output anything to stdout
  * outside of places protected by igt_run_subtest checks - the piglit
  * runner adds every line to the subtest list.
  */
-bool __igt_run_subtest(const char *subtest_name)
+bool __igt_run_subtest(const char *subtest_name, const char *file, const int line)
 {
 	int i;
 
@@ -954,18 +1055,25 @@ bool __igt_run_subtest(const char *subtest_name)
 			igt_exit();
 		}
 
-	if (list_subtests) {
-		printf("%s\n", subtest_name);
-		return false;
-	}
-
 	if (run_single_subtest) {
-		if (uwildmat(subtest_name, run_single_subtest) == 0)
+		if (uwildmat(subtest_name, run_single_subtest) == 0) {
+			_clear_current_description();
 			return false;
-		else
+		} else {
 			run_single_subtest_found = true;
+		}
 	}
 
+	if (describe_subtests) {
+		__igt_print_description(subtest_name, file, line);
+		_clear_current_description();
+		return false;
+	} else if (list_subtests) {
+		printf("%s\n", subtest_name);
+		return false;
+	}
+
+
 	if (skip_subtests_henceforth) {
 		printf("%sSubtest %s: %s%s\n",
 		       (!__igt_plain_output) ? "\x1b[1m" : "", subtest_name,
@@ -1014,15 +1122,32 @@ bool igt_only_list_subtests(void)
 	return list_subtests;
 }
 
-void __igt_subtest_group_save(int *save)
+
+
+void __igt_subtest_group_save(int *save, int *desc)
 {
 	assert(test_with_subtests);
 
+	if (__current_description[0] != '\0') {
+		struct description_node *new = calloc(1, sizeof(*new));
+		memcpy(new->desc, __current_description, sizeof(__current_description));
+		igt_list_add_tail(&new->link, &subgroup_descriptions);
+		_clear_current_description();
+		*desc = true;
+	}
+
 	*save = skip_subtests_henceforth;
 }
 
-void __igt_subtest_group_restore(int save)
+void __igt_subtest_group_restore(int save, int desc)
 {
+	if (desc) {
+		struct description_node *last =
+			igt_list_last_entry(&subgroup_descriptions, last, link);
+		igt_list_del(&last->link);
+		free(last);
+	}
+
 	skip_subtests_henceforth = save;
 }
 
@@ -1229,6 +1354,34 @@ bool igt_can_fail(void)
 	return !test_with_subtests || in_fixture || in_subtest;
 }
 
+/**
+ * igt_describe_f:
+ * @fmt: format string containing description
+ * @...: argument used by the format string
+ *
+ * Attach a description to the following #igt_subtest or #igt_subtest_group
+ * block.
+ *
+ * Check #igt_describe for more details.
+ *
+ */
+void igt_describe_f(const char *fmt, ...)
+{
+	int ret;
+	va_list args;
+
+	if (!describe_subtests)
+		return;
+
+	va_start(args, fmt);
+
+	ret = vsnprintf(__current_description, sizeof(__current_description), fmt, args);
+
+	va_end(args);
+
+	assert(ret < sizeof(__current_description));
+}
+
 static bool running_under_gdb(void)
 {
 	char pathname[30], buf[1024];
@@ -1540,7 +1693,7 @@ void igt_exit(void)
 		g_key_file_free(igt_key_file);
 
 	if (run_single_subtest && !run_single_subtest_found) {
-		igt_warn("Unknown subtest: %s\n", run_single_subtest);
+		igt_critical("Unknown subtest: %s\n", run_single_subtest);
 		exit(IGT_EXIT_INVALID);
 	}
 
diff --git a/lib/igt_core.h b/lib/igt_core.h
index 88a95ec2..1b37f142 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -174,7 +174,7 @@ int igt_subtest_init_parse_opts(int *argc, char **argv,
 #define igt_subtest_init(argc, argv) \
 	igt_subtest_init_parse_opts(&argc, argv, NULL, NULL, NULL, NULL, NULL);
 
-bool __igt_run_subtest(const char *subtest_name);
+bool __igt_run_subtest(const char *subtest_name, const char *file, const int line);
 #define __igt_tokencat2(x, y) x ## y
 
 /**
@@ -198,14 +198,14 @@ bool __igt_run_subtest(const char *subtest_name);
  *
  * This is a simpler version of igt_subtest_f()
  */
-#define igt_subtest(name) for (; __igt_run_subtest((name)) && \
+#define igt_subtest(name) for (; __igt_run_subtest((name), __FILE__, __LINE__) && \
 				   (sigsetjmp(igt_subtest_jmpbuf, 1) == 0); \
 				   igt_success())
 #define __igt_subtest_f(tmp, format...) \
 	for (char tmp [256]; \
 	     snprintf( tmp , sizeof( tmp ), \
 		      format), \
-	     __igt_run_subtest( tmp ) && \
+	     __igt_run_subtest(tmp, __FILE__, __LINE__) && \
 	     (sigsetjmp(igt_subtest_jmpbuf, 1) == 0); \
 	     igt_success())
 
@@ -227,8 +227,8 @@ bool __igt_run_subtest(const char *subtest_name);
 const char *igt_subtest_name(void);
 bool igt_only_list_subtests(void);
 
-void __igt_subtest_group_save(int *);
-void __igt_subtest_group_restore(int);
+void __igt_subtest_group_save(int *, int *);
+void __igt_subtest_group_restore(int, int);
 /**
  * igt_subtest_group:
  *
@@ -245,11 +245,14 @@ void __igt_subtest_group_restore(int);
  * group will fail or skip. Subtest groups can be arbitrarily nested.
  */
 #define igt_subtest_group for (int igt_tokencat(__tmpint,__LINE__) = 0, \
-			       igt_tokencat(__save,__LINE__) = 0; \
+			       igt_tokencat(__save,__LINE__) = 0, \
+			       igt_tokencat(__desc,__LINE__) = 0; \
 			       igt_tokencat(__tmpint,__LINE__) < 1 && \
-			       (__igt_subtest_group_save(& igt_tokencat(__save,__LINE__) ), true); \
+			       (__igt_subtest_group_save(& igt_tokencat(__save,__LINE__), \
+							 & igt_tokencat(__desc,__LINE__) ), true); \
 			       igt_tokencat(__tmpint,__LINE__) ++, \
-			       __igt_subtest_group_restore(igt_tokencat(__save,__LINE__) ))
+			       __igt_subtest_group_restore(igt_tokencat(__save,__LINE__), \
+							   igt_tokencat(__desc,__LINE__)))
 
 /**
  * igt_main_args:
@@ -385,6 +388,124 @@ static inline void igt_ignore_warn(bool value)
 {
 }
 
+__attribute__((format(printf, 1, 2)))
+void igt_describe_f(const char *fmt, ...);
+
+/**
+ * igt_describe:
+ * @dsc: string containing description
+ *
+ * Attach a description to the following #igt_subtest or #igt_subtest_group
+ * block.
+ *
+ * The description should complement the test/subtest name and provide more
+ * context on what is being tested. It should explain the idea of the test and
+ * do not mention implementation details, so that it never goes out of date.
+ *
+ * DO:
+ *  * focus on the userspace's perspective
+ *  * try to capture the reason for the test's existence
+ *  * be brief
+ *
+ * DON'T:
+ *  * try to translate the code into English
+ *  * explain all the checks the test does
+ *  * delve on the implementation
+ *
+ * Good examples:
+ *  * "make sure that legacy cursor updates do not stall atomic commits"
+ *  * "check that atomic updates of many planes are indeed atomic and take
+ *     effect immediately after the commit"
+ *  * "make sure that the meta-data exposed by the kernel to the userspace
+ *     is correct and matches the used EDID"
+ *
+ * Bad examples:
+ *  * "spawn 10 threads, each pinning cpu core with a busy loop..."
+ *  * "randomly generate holes in a primary plane then try to cover each hole
+ *    with a plane and make sure that CRC matches, do 25 gazillion rounds of
+ *    that..."
+ *
+ *
+ * Resulting #igt_subtest documentation is a concatenation of its own
+ * description and all the parenting #igt_subtest_group descriptions, starting
+ * from the outermost one. Example:
+ *
+ * |[<!-- language="C" -->
+ * #include "igt.h"
+ *
+ * IGT_TEST_DESCRIPTION("Global description of the whole binary");
+ * igt_main
+ * {
+ * 	igt_describe("Desc of the subgroup with A and B");
+ * 	igt_subtest_group {
+ * 		igt_describe("Desc of the subtest A");
+ * 		igt_subtest("subtest-a") {
+ * 			...
+ * 		}
+ *
+ * 		igt_describe("Desc of the subtest B");
+ * 		igt_subtest("subtest-b") {
+ * 			...
+ * 		}
+ * 	}
+ *
+ * 	igt_describe("Desc of the subtest C");
+ * 	igt_subtest("subtest-c") {
+ * 		...
+ * 	}
+ * }
+ * ]|
+ *
+ * It's will accessible via --describe command line switch:
+ *
+ * |[
+ * $ test --describe
+ * Global description of the whole binary
+ *
+ * SUB subtest-a test.c:5:
+ *   Desc of the subgroup with A and B
+ *
+ *   Desc of the subtest A
+ *
+ * SUB subtest-b test.c:10:
+ *   Desc of the subgroup with A and B
+ *
+ *   Desc of the subtest B
+ *
+ * SUB subtest-c test.c:15:
+ *   Desc of the subtest C
+ * ]|
+ *
+ * Every single #igt_subtest does not have to be preceded with a #igt_describe
+ * as long as it has good-enough explanation provided on the #igt_subtest_group
+ * level.
+ *
+ * Example:
+ *
+ * |[<!-- language="C" -->
+ * #include "igt.h"
+ *
+ * igt_main
+ * {
+ * 	igt_describe("check xyz with different tilings");
+ * 	igt_subtest_group {
+ * 		// no need for extra description, group is enough and tiling is
+ * 		// obvious from the test name
+ * 		igt_subtest("foo-tiling-x") {
+ * 			...
+ * 		}
+ *
+ * 		igt_subtest("foo-tiling-y") {
+ * 			...
+ * 		}
+ * 	}
+ * }
+ * ]|
+ *
+ */
+#define igt_describe(dsc) \
+	igt_describe_f("%s", dsc)
+
 /**
  * igt_assert:
  * @expr: condition to test
diff --git a/lib/tests/igt_describe.c b/lib/tests/igt_describe.c
new file mode 100644
index 00000000..a9f857bb
--- /dev/null
+++ b/lib/tests/igt_describe.c
@@ -0,0 +1,266 @@
+/*
+ * Copyright © 2019 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 <sys/wait.h>
+#include <string.h>
+#include <stdlib.h>
+
+#include "drmtest.h"
+#include "igt_tests_common.h"
+
+IGT_TEST_DESCRIPTION("the top level description");
+static void fake_main(int argc, char **argv) {
+	igt_subtest_init(argc, argv);
+
+	igt_describe("Basic A");
+	igt_subtest("A")
+		;
+
+	igt_fixture
+		printf("should not be executed!\n");
+
+	igt_describe("Group with B, C & D");
+	igt_subtest_group {
+		igt_describe("Basic B");
+		igt_subtest("B")
+			;
+
+		if (!igt_only_list_subtests())
+			printf("should not be executed!\n");
+
+		igt_describe("Group with C & D");
+		igt_subtest_group {
+			igt_describe("Basic C");
+			igt_subtest("C")
+				printf("should not be executed!\n");
+
+			// NO DOC
+			igt_subtest("D")
+				;
+		}
+	}
+
+	// NO DOC
+	igt_subtest_group {
+		// NO DOC
+		igt_subtest("E")
+			;
+	}
+
+	// NO DOC
+	igt_subtest("F")
+		;
+
+	igt_describe("this description should be so long that it wraps itself nicely in the terminal "
+		     "this description should be so long that it wraps itself nicely in the terminal "
+		     "this description should be so long that it wraps itself nicely in the terminal "
+		     "this description should be so long that it wraps itself nicely in the terminal "
+		     "this description should be so long that it wraps itself nicely in the terminal "
+		     "this description should be so long that it wraps itself nicely in the terminal");
+	igt_subtest("G")
+		;
+
+	igt_describe("verylongwordthatshoudlbeprintedeventhoughitspastthewrppinglimit"
+		     "verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit "
+		     "verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit"
+		     "verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit");
+	igt_subtest("F")
+		;
+
+	igt_exit();
+}
+
+static const char DESCRIBE_ALL_OUTPUT[] = \
+	"the top level description\n"
+	"\n"
+	"SUB A ../lib/tests/igt_describe.c:36:\n"
+	"  Basic A\n"
+	"\n"
+	"SUB B ../lib/tests/igt_describe.c:45:\n"
+	"  Group with B, C & D\n"
+	"\n"
+	"  Basic B\n"
+	"\n"
+	"SUB C ../lib/tests/igt_describe.c:54:\n"
+	"  Group with B, C & D\n"
+	"\n"
+	"  Group with C & D\n"
+	"\n"
+	"  Basic C\n"
+	"\n"
+	"SUB D ../lib/tests/igt_describe.c:58:\n"
+	"  Group with B, C & D\n"
+	"\n"
+	"  Group with C & D\n"
+	"\n"
+	"SUB E ../lib/tests/igt_describe.c:66:\n"
+	"  NO DOCUMENTATION!\n"
+	"\n"
+	"SUB F ../lib/tests/igt_describe.c:71:\n"
+	"  NO DOCUMENTATION!\n"
+	"\n"
+	"SUB G ../lib/tests/igt_describe.c:80:\n"
+	"  this description should be so long that it wraps itself nicely in the terminal this\n"
+	"  description should be so long that it wraps itself nicely in the terminal this description\n"
+	"  should be so long that it wraps itself nicely in the terminal this description should be so\n"
+	"  long that it wraps itself nicely in the terminal this description should be so long that it\n"
+	"  wraps itself nicely in the terminal this description should be so long that it wraps itself\n"
+	"  nicely in the terminal\n"
+	"\n"
+	"SUB F ../lib/tests/igt_describe.c:87:\n"
+	"  verylongwordthatshoudlbeprintedeventhoughitspastthewrppinglimitverylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit\n"
+	"  verylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimitverylongwordthatshoudlbeprintedeventhoughitspastthewrappinglimit\n\n";
+
+static const char JUST_C_OUTPUT[] = \
+	"the top level description\n"
+	"\n"
+	"SUB C ../lib/tests/igt_describe.c:54:\n"
+	"  Group with B, C & D\n"
+	"\n"
+	"  Group with C & D\n"
+	"\n"
+	"  Basic C\n"
+	"\n";
+
+static void assert_pipe_empty(int fd)
+{
+	char buf[5];
+	internal_assert(0 == read(fd, buf, sizeof(buf)));
+}
+
+static void read_whole_pipe(int fd, char *buf, size_t buflen)
+{
+	ssize_t readlen;
+	off_t offset;
+
+	offset = 0;
+	while ((readlen = read(fd, buf+offset, buflen-offset))) {
+		internal_assert(readlen != -1);
+		offset += readlen;
+	}
+}
+
+static pid_t do_fork(int argc, char **argv, int *out, int *err)
+{
+	int outfd[2], errfd[2];
+	pid_t pid;
+
+	internal_assert(pipe(outfd) != -1);
+	internal_assert(pipe(errfd) != -1);
+
+	pid = fork();
+	internal_assert(pid != -1);
+
+	if (pid == 0) {
+		while ((dup2(outfd[1], STDOUT_FILENO) == -1) && (errno == EINTR)) {}
+		while ((dup2(errfd[1], STDERR_FILENO) == -1) && (errno == EINTR)) {}
+
+		fake_main(argc, argv);
+
+		exit(-1);
+	} else {
+		/* close the writing ends */
+		close(outfd[1]);
+		close(errfd[1]);
+
+		*out = outfd[0];
+		*err = errfd[0];
+
+		return pid;
+	}
+}
+
+static int _wait(pid_t pid, int *status) {
+	int ret;
+
+	do {
+		ret = waitpid(pid, status, 0);
+	} while (ret == -1 && errno == EINTR);
+
+	return ret;
+}
+
+int main(int argc, char **argv)
+{
+	char prog[] = "igt_describe";
+	int status;
+	int outfd, errfd;
+
+	/* describe all subtest */ {
+		static char out[4096];
+		char arg[] = "--describe";
+		char *fake_argv[] = {prog, arg};
+		int fake_argc = ARRAY_SIZE(fake_argv);
+
+		pid_t pid = do_fork(fake_argc, fake_argv, &outfd, &errfd);
+
+		read_whole_pipe(outfd, out, sizeof(out));
+		assert_pipe_empty(errfd);
+
+		internal_assert(_wait(pid, &status) != -1);
+		internal_assert(WEXITSTATUS(status) == IGT_EXIT_SUCCESS);
+		internal_assert(0 == strcmp(DESCRIBE_ALL_OUTPUT, out));
+
+		close(outfd);
+		close(errfd);
+	}
+
+	/* describe C using a pattern */ {
+		static char out[4096];
+		char arg[] = "--describe=C";
+		char *fake_argv[] = {prog, arg};
+		int fake_argc = ARRAY_SIZE(fake_argv);
+
+		pid_t pid = do_fork(fake_argc, fake_argv, &outfd, &errfd);
+
+		read_whole_pipe(outfd, out, sizeof(out));
+		assert_pipe_empty(errfd);
+
+		internal_assert(_wait(pid, &status) != -1);
+		internal_assert(WEXITSTATUS(status) == IGT_EXIT_SUCCESS);
+		internal_assert(0 == strcmp(JUST_C_OUTPUT, out));
+
+		close(outfd);
+		close(errfd);
+	}
+
+	/* fail describing with a bad pattern */ {
+		static char err[4096];
+		char arg[] = "--describe=Z";
+		char *fake_argv[] = {prog, arg};
+		int fake_argc = ARRAY_SIZE(fake_argv);
+
+		pid_t pid = do_fork(fake_argc, fake_argv, &outfd, &errfd);
+
+		read_whole_pipe(errfd, err, sizeof(err));
+
+		internal_assert(_wait(pid, &status) != -1);
+		internal_assert(WEXITSTATUS(status) == IGT_EXIT_INVALID);
+		internal_assert(strstr(err, "Unknown subtest: Z"));
+
+		close(outfd);
+		close(errfd);
+	}
+
+	return 0;
+}
diff --git a/lib/tests/meson.build b/lib/tests/meson.build
index b930ee6e..4d907e34 100644
--- a/lib/tests/meson.build
+++ b/lib/tests/meson.build
@@ -3,6 +3,7 @@ lib_tests = [
 	'igt_can_fail',
 	'igt_can_fail_simple',
 	'igt_conflicting_args',
+	'igt_describe',
 	'igt_edid',
 	'igt_exit_handler',
 	'igt_fork',
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2019-07-05 17:06 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-01 12:21 [igt-dev] [PATCH i-g-t 1/5] lib/igt_core: Add support for subtest descriptions Arkadiusz Hiler
2019-07-01 12:21 ` [igt-dev] [PATCH i-g-t 2/5] tests/kms_hdmi_inject: Provide igt_descriptions Arkadiusz Hiler
2019-07-01 12:21 ` [igt-dev] [PATCH i-g-t 3/5] tests/kms_plane_multiple: Describe the test Arkadiusz Hiler
2019-07-01 12:21 ` [igt-dev] [PATCH i-g-t 4/5] CONTRIBUTING: Rework a bit and update Arkadiusz Hiler
2019-07-01 13:02   ` Ser, Simon
2019-07-01 12:21 ` [igt-dev] [PATCH i-g-t 5/5] docs: Embed subtest descriptions in the documentation Arkadiusz Hiler
2019-07-03  7:18   ` Ser, Simon
2019-07-01 13:01 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/5] lib/igt_core: Add support for subtest descriptions Patchwork
2019-07-02 16:41 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2019-07-03 13:08 ` [igt-dev] [PATCH i-g-t 1/5] " Ser, Simon
2019-07-04 11:22   ` Arkadiusz Hiler
2019-07-04 11:33     ` Ser, Simon
2019-07-04 11:55 ` [igt-dev] [PATCH 1/5 v2 i-g-t] " Arkadiusz Hiler
2019-07-04 13:02   ` Ser, Simon
2019-07-04 12:48 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [1/5,v2,i-g-t] lib/igt_core: Add support for subtest descriptions (rev2) Patchwork
2019-07-05 17:06 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2019-06-17 10:54 [igt-dev] [PATCH i-g-t 1/5] lib/igt_core: Add support for subtest descriptions Arkadiusz Hiler

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.