Re: [LTP] [PATCH 1/2] metadata: add tests grouping support

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> 1. Source file path - the two nearest parent directories (immediate
>    parent first), skipping 'kernel' as too generic. For example:
>    - testcases/kernel/syscalls/clone/clone01.c -> clone, syscalls
>    - testcases/kernel/kvm/kvm_pagefault01.c -> kvm
>    - testcases/cve/cve-2017-16939.c -> cve
> 
> 2. @group tags in the doc comment block, e.g.:
>    /*    * Test description.
>     *
>     * @group stress

Maybe call it groups and allow white-space separated list?

I'm not really sure about the syntax, but I guess that @groups foo bar
will do.

>     */
> 
> Add test case for @group tag parsing.
> 
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  metadata/metaparse.c         | 88 ++++++++++++++++++++++++++++++++++++++++++++
>  metadata/tests/groups.c      | 11 ++++++
>  metadata/tests/groups.c.json | 13 +++++++
>  3 files changed, 112 insertions(+)
> 
> diff --git a/metadata/metaparse.c b/metadata/metaparse.c
> index 561cbb9d2d54689988c9aa49d591628696bcf847..6bc4b7af60c7449d4b60a1252fa58fed77e03066 100644
> --- a/metadata/metaparse.c
> +++ b/metadata/metaparse.c
> @@ -1168,6 +1168,92 @@ static void print_help(const char *prgname)
>  	exit(0);
>  }
>  
> +/*
> + * Add groups derived from the source file path.
> + *
> + * Groups are the two nearest parent directories (immediate parent
> + * first), skipping 'kernel' as it's too generic:
> + *
> + *   testcases/kernel/syscalls/clone/clone01.c  -> clone, syscalls
> + *   testcases/kernel/kvm/kvm_pagefault01.c     -> kvm
> + *   testcases/cve/cve-2017-16939.c             -> cve
> + */
> +static void add_path_groups(struct data_node *groups, const char *fname)
> +{
> +	char buf[256];
> +	int offsets[8];
> +	int ndirs = 0;
> +	int ngroups = 0;
> +	char *p;
> +
> +	if (strncmp(fname, "testcases/", 10))
> +		return;
> +
> +	snprintf(buf, sizeof(buf), "%s", fname + 10);

Maybe avoid static buffers here with:

	buf = strdup(fname + 10);

> +	p = strtok(buf, "/");
> +	while (p && ndirs < 8) {
> +		offsets[ndirs++] = p - buf;

Why do we store offset rather than the pointer to the string?

		dirs[ndirs++] = p;

> +		p = strtok(NULL, "/");
> +	}
> +
> +	/* Last element is the filename, skip it */
> +	ndirs--;
> +
> +	for (int j = ndirs - 1; j >= 0 && ngroups < 2; j--) {
> +		if (!strcmp(buf + offsets[j], "kernel"))
> +			continue;
> +
> +		data_node_array_add(groups, data_node_string(buf + offsets[j]));
> +		ngroups++;
> +	}

We can avoid the complex loop by adding a function to add the group with
the "kernel" filter and just calling it twice here:

	add_group(groups, dirs[ndirs-1]);
	add_group(groups, dirs[ndirs-2]);

> +}
> +
> +/*
> + * Add groups from @group tags in the doc comment block.
> + */
> +static void add_doc_groups(struct data_node *groups, struct data_node *doc)
> +{
> +	if (!doc || doc->type != DATA_ARRAY)
> +		return;
> +
> +	for (unsigned int i = 0; i < data_node_array_len(doc); i++) {
> +		struct data_node *line = doc->array.array[i];
> +		const char *s;
> +
> +		if (line->type != DATA_STRING)
> +			continue;
> +
> +		s = line->string.val;
> +
> +		while (*s && (*s == ' ' || *s == '\t'))
> +			s++;
> +
> +		if (strncmp(s, "@group ", 7))
> +			continue;
> +
> +		s += 7;
> +		while (*s && (*s == ' ' || *s == '\t'))
> +			s++;
> +
> +		if (*s)
> +			data_node_array_add(groups, data_node_string(s));
> +	}

If we hook up into the multiline_comment() function we can consume the
line as well so that it does not appear in the parsed doc string. We
would have to pass the groups node from the main all the way to
multiline_comment() something as (uncomplete):

diff --git a/metadata/metaparse.c b/metadata/metaparse.c
index 561cbb9d2..441eeb80b 100644
--- a/metadata/metaparse.c
+++ b/metadata/metaparse.c
@@ -50,7 +50,12 @@ static const char *eat_asterisk_space(const char *c)
        return c;
 }

-static void multiline_comment(FILE *f, struct data_node *doc)
+static int parse_groups(struct data_node *groups, char *line)
+{
+       ...
+}
+
+static void multiline_comment(FILE *f, struct data_node *doc, struct data_node *groups)
 {
        int c;
        int state = 0;
@@ -65,8 +70,11 @@ static void multiline_comment(FILE *f, struct data_node *doc)
                                struct data_node *line;
                                buf[bufp] = 0;
                                line = data_node_string(eat_asterisk_space(buf));
-                               if (data_node_array_add(doc, line))
-                                       WARN("doc string comment truncated");
+
+                               if (!parse_groups(groups, line)) {
+                                       if (data_node_array_add(doc, line))
+                                               WARN("doc string comment truncated");
+                               }
                                bufp = 0;
                                continue;
                        }
@@ -100,7 +108,7 @@ static void multiline_comment(FILE *f, struct data_node *doc)

 static const char doc_prefix[] = "\\\n";

-static void maybe_doc_comment(FILE *f, struct data_node *doc)
+static void maybe_doc_comment(FILE *f, struct data_node *doc, struct data_groups *groups)
 {
        int c, i;

@@ -113,14 +121,14 @@ static void maybe_doc_comment(FILE *f, struct data_node *doc)
                if (c == '*')
                        ungetc(c, f);

-               multiline_comment(f, NULL);
+               multiline_comment(f, NULL, NULL);
                return;
        }

-       multiline_comment(f, doc);
+       multiline_comment(f, doc, groups);
 }

-static void maybe_comment(FILE *f, struct data_node *doc)
+static void maybe_comment(FILE *f, struct data_node *doc, struct data_node *groups)
 {
        int c = getc(f);

@@ -129,7 +137,7 @@ static void maybe_comment(FILE *f, struct data_node *doc)
                remove_to_newline(f);
        break;
        case '*':
-               maybe_doc_comment(f, doc);
+               maybe_doc_comment(f, doc, groups);
        break;
        default:
                ungetc(c, f);
@@ -194,7 +202,7 @@ static char *next_token2(FILE *f, char *buf, size_t buf_len, struct data_node *d
                        buf[i++] = c;
                break;
                case '/':
-                       maybe_comment(f, doc);
+                       maybe_comment(f, doc, groups);
                break;
                case '"':
                        in_str = 1;


> +}
> +
> +static void build_groups(struct data_node *res, const char *fname)
> +{
> +	struct data_node *groups = data_node_array();
> +
> +	add_path_groups(groups, fname);
> +	add_doc_groups(groups, data_node_hash_get(res, "doc"));
> +
> +	if (data_node_array_len(groups))
> +		data_node_hash_add(res, "groups", groups);
> +	else
> +		data_node_free(groups);
> +}
> +
>  int main(int argc, char *argv[])
>  {
>  	unsigned int i, j;
> @@ -1238,6 +1324,8 @@ int main(int argc, char *argv[])
>  	}
>  
>  	data_node_hash_add(res, "fname", data_node_string(argv[optind]));
> +	build_groups(res, argv[optind]);
> +
>  	printf("  \"%s\": ", strip_name(argv[optind]));
>  	data_to_json(res, stdout, 2);
>  	data_node_free(res);
> diff --git a/metadata/tests/groups.c b/metadata/tests/groups.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..82f07111c1506c634f13822ee6aa95f574eb19a5
> --- /dev/null
> +++ b/metadata/tests/groups.c
> @@ -0,0 +1,11 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +/*\
> + * Test for @group tag parsing.
> + *
> + * @group stress
> + * @group regression
> + */
> +
> +static struct tst_test test = {
> +};
> diff --git a/metadata/tests/groups.c.json b/metadata/tests/groups.c.json
> new file mode 100644
> index 0000000000000000000000000000000000000000..4683e6cf07eeebc60faefb9aead9370dc3f631aa
> --- /dev/null
> +++ b/metadata/tests/groups.c.json
> @@ -0,0 +1,13 @@
> +  "groups": {
> +   "doc": [
> +    "Test for @group tag parsing.",
> +    "",
> +    "@group stress",
> +    "@group regression"
> +   ],
> +   "fname": "groups.c",
> +   "groups": [
> +    "stress",
> +    "regression"
> +   ]
> +  }
> \ No newline at end of file
> 
> -- 
> 2.51.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp