Re: [PATCH i-g-t v2 3/6] runner: Add attachments directory content in subtests results

[Gustavo Sousa <gustavo.sousa@intel.com>]

Zbigniew Kempczyński <zbigniew.kempczynski@intel.com> writes:

> Files produced by hooks which lands in attachments directory should
> be called in piglit style, what means they should be in form:
>
> igt@test@subtest@@unique-filename-from-hook
> igt@test@subtest@dynsubtest@@unique-filename-from-hook
>
> depending is it normal subtest or dynamic subtest.
>
> Hooks introduced IGT_HOOK_TEST_FULLNAME env variable, which contains
>
> igt@test@subtest
> igt@test@subtest@dynsubtest
>
> and it should be used as prefix part (before '@@') of attachment
> filename when hook script writes some content to it.
>
> Such defined contract between hook script and resultgen allows to
> distinguish subtest/dynsubtest attachments structure in results.json.
> So string before '@@' is a subtest/dynsubtest name in results file,
> and string after it is a unique key of a base64 file content added
> to results.

I'm not sure we want to go this path based on the current feedback.
However, *if* we end up embedding this stuff into results.json, I think I
would prefer that the test object itself would contain an "attachments"
key containing the same tree structure of the attachments directory.

--
Gustavo Sousa

>
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
> Cc: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> Cc: Ryszard Knop <ryszard.knop@intel.com>
> Cc: Krzysztof Karas <krzysztof.karas@intel.com>
> ---
>  runner/resultgen.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 97 insertions(+)
>
> diff --git a/runner/resultgen.c b/runner/resultgen.c
> index 29fc5cbb15..c4ebbd7b6e 100644
> --- a/runner/resultgen.c
> +++ b/runner/resultgen.c
> @@ -1142,6 +1142,100 @@ static bool fill_from_dmesg(int fd,
>  	return true;
>  }
>  
> +static char *get_base64(int dirfd, const char *filename)
> +{
> +	struct stat st;
> +	int fd;
> +	unsigned char *data;
> +	char *base64 = NULL;
> +
> +	if (fstatat(dirfd, filename, &st, 0)) {
> +		fprintf(stderr, "Can't stat %s\n", filename);
> +		return NULL;
> +	}
> +
> +	fd = openat(dirfd, filename, O_RDONLY);
> +	if (fd < 0) {
> +		fprintf(stderr, "Can't open %s\n", filename);
> +		return NULL;
> +	}
> +
> +	data = malloc(st.st_size);
> +	if (!data) {
> +		fprintf(stderr, "Cannot allocate %zu bytes for data buffer\n",
> +			st.st_size);
> +		goto err_alloc;
> +	}
> +
> +	if (read(fd, data, st.st_size) != st.st_size) {
> +		fprintf(stderr, "Cannot read %zu bytes from data buffer\n",
> +			st.st_size);
> +		goto err_read;
> +	}
> +
> +	base64 = g_base64_encode(data, st.st_size);
> +
> +err_read:
> +	free(data);
> +err_alloc:
> +	close(fd);
> +
> +	return base64;
> +}
> +
> +static bool fill_from_attachments(int dirfd, struct json_t *tests)
> +{
> +	struct json_t *obj = NULL, *attobj = NULL;
> +	char attname[32];
> +	int atdir;
> +	struct dirent *entry;
> +	DIR *dir;
> +
> +	snprintf(attname, sizeof(attname), "%s", DIR_ATTACHMENTS);
> +	if ((atdir = openat(dirfd, attname, O_DIRECTORY | O_RDONLY | O_CLOEXEC)) < 0) {
> +		fprintf(stderr, "Error opening '%s' dir\n", DIR_ATTACHMENTS);
> +		return false;
> +	}
> +
> +	dir = fdopendir(atdir);
> +	if (!dir) {
> +		close(atdir);
> +		return false;
> +	}
> +
> +	while ((entry = readdir(dir))) {
> +		char *base64_data, *piglit_name, *p;
> +
> +		if (strcmp(entry->d_name, ".") == 0 ||
> +		    strcmp(entry->d_name, "..") == 0)
> +			continue;
> +
> +		piglit_name = strdup(entry->d_name);
> +		p = strstr(piglit_name, "@@");
> +		if (!p) {
> +			free(piglit_name);
> +			continue;
> +		}
> +		*p = '\0';
> +		p += 2;
> +
> +		obj = get_or_create_json_object(tests, piglit_name);
> +		attobj = get_or_create_json_object(obj, "attachments");
> +
> +		base64_data = get_base64(atdir, entry->d_name);
> +		if (base64_data) {
> +			json_object_set_new(attobj, p,
> +					    escaped_json_stringn(base64_data, strlen(base64_data)));
> +			free(base64_data);
> +		}
> +		free(piglit_name);
> +	}
> +
> +	closedir(dir);
> +
> +	return true;
> +}
> +
>  static const char *result_from_exitcode(int exitcode)
>  {
>  	switch (exitcode) {
> @@ -2230,6 +2324,9 @@ static bool parse_test_directory(int dirfd,
>  		fprintf(stderr, "Error parsing output files (dmesg.txt)\n");
>  	}
>  
> +	if (!fill_from_attachments(dirfd, results->tests))
> +		fprintf(stderr, "Error parsing attachments directory\n");
> +
>  	override_results(entry->binary, &subtests, results->tests);
>  	prune_subtests(settings, entry, &subtests, results->tests);
>  
> -- 
> 2.43.0