Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i-g-t] runner: Parse results harder
@ 2025-02-26 19:33 Kamil Konieczny
  2025-02-27 16:37 ` Knop, Ryszard
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Kamil Konieczny @ 2025-02-26 19:33 UTC (permalink / raw)
  To: igt-dev
  Cc: Kamil Konieczny, Ewelina Musial, Lucas De Marchi, Ryszard Knop,
	Petri Latvala

  Sometimes an error happens in kernel or in test that leaves
output files in corrupted or incorrect state. While runner or
resume will just move on to executing next test, when generating
results it could end up with no results.json

  Try processing outputs a little more persistently and use any
output file left there, even if only dmesg.txt. Also, when no
useful output files were present, instead of breaking out add
notrun.

  Inform about processing results for each test so a problem
could be spotted more easily.

Cc: Ewelina Musial <ewelina.musial@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Ryszard Knop <ryszard.knop@intel.com>
Cc: Petri Latvala <adrinael@adrinael.net>
Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
---
 runner/executor.c  | 21 +++++++++++++++++++++
 runner/executor.h  |  1 +
 runner/resultgen.c | 45 ++++++++++++++++++++++++++++++---------------
 3 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/runner/executor.c b/runner/executor.c
index 2abb18732..96b8b5057 100644
--- a/runner/executor.c
+++ b/runner/executor.c
@@ -576,6 +576,27 @@ bool open_output_files(int dirfd, int *fds, bool write)
 	return true;
 }
 
+/**
+ * open_output_files_rdonly:
+ * @dirfd: fd of output directory with err.txt, dmesg.txt and other files
+ * @fds: array for fd's of opened output files
+ *
+ * Tries to open output files in read-only mode and saves file descriptors
+ * in fds arrray.
+ *
+ * Returns: true if all files opened, false otherwise
+ */
+bool open_output_files_rdonly(int dirfd, int *fds)
+{
+	bool ret = true;
+
+	for (int i = 0; i < _F_LAST; i++)
+		if ((fds[i] = openat(dirfd, filenames[i], O_RDONLY)) < 0)
+			ret = false; /* Remember failure */
+
+	return ret;
+}
+
 void close_outputs(int *fds)
 {
 	int i;
diff --git a/runner/executor.h b/runner/executor.h
index ab6a0c176..56c7323a8 100644
--- a/runner/executor.h
+++ b/runner/executor.h
@@ -26,6 +26,7 @@ enum {
 };
 
 bool open_output_files(int dirfd, int *fds, bool write);
+bool open_output_files_rdonly(int dirfd, int *fds);
 void close_outputs(int *fds);
 
 /*
diff --git a/runner/resultgen.c b/runner/resultgen.c
index 0d3a569cf..b50c417e3 100644
--- a/runner/resultgen.c
+++ b/runner/resultgen.c
@@ -2176,12 +2176,26 @@ static bool parse_test_directory(int dirfd,
 {
 	int fds[_F_LAST];
 	struct subtest_list subtests = {};
-	bool status = true;
 	int commsparsed;
 
-	if (!open_output_files(dirfd, fds, false)) {
-		fprintf(stderr, "Error opening output files\n");
-		return false;
+	if (!open_output_files_rdonly(dirfd, fds)) {
+		struct stat statbuf;
+		size_t sz = 0;
+
+		for (int i = 0; i < _F_LAST; ++i) {
+			if (fds[i] > 0 && !fstat(fds[i], &statbuf) && statbuf.st_size != 0)
+				sz = statbuf.st_size;
+		}
+
+		if (sz == 0) {
+			/* no output saved in any file */
+			fprintf(stderr, "results: Error opening output files\n");
+			close_outputs(fds);
+
+			return false;
+		}
+
+		fprintf(stderr, "results: Missing few output file(s)\n");
 	}
 
 	/*
@@ -2191,8 +2205,6 @@ static bool parse_test_directory(int dirfd,
 	commsparsed = fill_from_comms(fds[_F_SOCKET], entry, &subtests, results);
 	if (commsparsed == COMMSPARSE_ERROR) {
 		fprintf(stderr, "Error parsing output files (comms)\n");
-		status = false;
-		goto parse_output_end;
 	}
 
 	if (commsparsed == COMMSPARSE_EMPTY) {
@@ -2200,20 +2212,17 @@ static bool parse_test_directory(int dirfd,
 		 * fill_from_journal fills the subtests struct and
 		 * adds timeout results where applicable.
 		 */
-		fill_from_journal(fds[_F_JOURNAL], entry, &subtests, results);
+		if (fds[_F_JOURNAL] > 0)
+			fill_from_journal(fds[_F_JOURNAL], entry, &subtests, results);
 
 		if (!fill_from_output(fds[_F_OUT], entry->binary, "out", &subtests, results->tests) ||
 		    !fill_from_output(fds[_F_ERR], entry->binary, "err", &subtests, results->tests)) {
 			fprintf(stderr, "Error parsing output files (out.txt, err.txt)\n");
-			status = false;
-			goto parse_output_end;
 		}
 	}
 
 	if (!fill_from_dmesg(fds[_F_DMESG], settings, entry->binary, &subtests, results->tests)) {
 		fprintf(stderr, "Error parsing output files (dmesg.txt)\n");
-		status = false;
-		goto parse_output_end;
 	}
 
 	override_results(entry->binary, &subtests, results->tests);
@@ -2221,11 +2230,10 @@ static bool parse_test_directory(int dirfd,
 
 	add_to_totals(entry->binary, &subtests, results);
 
- parse_output_end:
 	close_outputs(fds);
 	free_subtests(&subtests);
 
-	return status;
+	return true;
 }
 
 static void try_add_notrun_results(const struct job_list_entry *entry,
@@ -2359,14 +2367,21 @@ struct json_object *generate_results_json(int dirfd)
 		char name[16];
 
 		snprintf(name, 16, "%zd", i);
+		fprintf(stderr, "results: parsing output: %s/ for test: %s\n",
+			name, job_list.entries[i].binary);
 		if ((testdirfd = openat(dirfd, name, O_DIRECTORY | O_RDONLY)) < 0) {
+			if (settings.log_level >= LOG_LEVEL_NORMAL)
+				fprintf(stderr, "results: no output, setting notrun\n)");
+
 			try_add_notrun_results(&job_list.entries[i], &settings, &results);
 			continue;
 		}
 
 		if (!parse_test_directory(testdirfd, &job_list.entries[i], &settings, &results)) {
-			close(testdirfd);
-			return NULL;
+			if (settings.log_level >= LOG_LEVEL_NORMAL)
+				fprintf(stderr, "results: no useful output, setting notrun\n");
+
+			try_add_notrun_results(&job_list.entries[i], &settings, &results);
 		}
 		close(testdirfd);
 	}
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [PATCH i-g-t] runner: Parse results harder
@ 2025-02-28 17:15 Kamil Konieczny
  2025-03-03 11:02 ` Krzysztof Karas
  0 siblings, 1 reply; 11+ messages in thread
From: Kamil Konieczny @ 2025-02-28 17:15 UTC (permalink / raw)
  To: igt-dev
  Cc: Kamil Konieczny, Ewelina Musial, Lucas De Marchi, Ryszard Knop,
	Petri Latvala

  Sometimes an error happens in kernel or in test that leaves
output files in corrupted or incorrect state. While runner or
resume will just move on to executing next test, when generating
results it could end up with no results.json

  Try processing outputs a little more persistently and use any
output file left there, even if only dmesg.txt. Also, when no
useful output files were present, instead of breaking out add
notrun.

  Inform about processing results for each test so a problem
could be spotted more easily.

v2: removed ')' from 'notrun\n)' (Kamil)
 using bool var, added more prints about errors (Ryszard)

Cc: Ewelina Musial <ewelina.musial@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: Ryszard Knop <ryszard.knop@intel.com>
Cc: Petri Latvala <adrinael@adrinael.net>
Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
---
 runner/executor.c  | 31 ++++++++++++++++++++++
 runner/executor.h  |  2 ++
 runner/resultgen.c | 66 +++++++++++++++++++++++++++++++++++-----------
 3 files changed, 84 insertions(+), 15 deletions(-)

diff --git a/runner/executor.c b/runner/executor.c
index 2abb18732..2040f0f33 100644
--- a/runner/executor.c
+++ b/runner/executor.c
@@ -576,6 +576,29 @@ bool open_output_files(int dirfd, int *fds, bool write)
 	return true;
 }
 
+/**
+ * open_output_files_rdonly:
+ * @dirfd: fd of output directory with err.txt, dmesg.txt and other files
+ * @fds: array for fd's of opened output files
+ *
+ * Tries to open output files in read-only mode and saves file descriptors
+ * in fds arrray.
+ *
+ * Returns: true if all files opened, false otherwise
+ */
+bool open_output_files_rdonly(int dirfd, int *fds)
+{
+	bool ret = true;
+
+	for (int i = 0; i < _F_LAST; i++)
+		if ((fds[i] = openat(dirfd, filenames[i], O_RDONLY)) < 0) {
+			fds[i] = -errno;
+			ret = false; /* Remember failure */
+		}
+
+	return ret;
+}
+
 void close_outputs(int *fds)
 {
 	int i;
@@ -585,6 +608,14 @@ void close_outputs(int *fds)
 	}
 }
 
+const char *get_out_filename(int fid)
+{
+	if (fid >= 0 && fid < _F_LAST)
+		return filenames[fid];
+
+	return "output-filename-index-error";
+}
+
 /* Returns the number of bytes written to disk, or a negative number on error */
 static long dump_dmesg(int kmsgfd, int outfd, ssize_t size)
 {
diff --git a/runner/executor.h b/runner/executor.h
index ab6a0c176..3b1cabcf5 100644
--- a/runner/executor.h
+++ b/runner/executor.h
@@ -26,7 +26,9 @@ enum {
 };
 
 bool open_output_files(int dirfd, int *fds, bool write);
+bool open_output_files_rdonly(int dirfd, int *fds);
 void close_outputs(int *fds);
+const char *get_out_filename(int fid);
 
 /*
  * Initialize execute_state object to a state where it's ready to
diff --git a/runner/resultgen.c b/runner/resultgen.c
index 0d3a569cf..df4be69a6 100644
--- a/runner/resultgen.c
+++ b/runner/resultgen.c
@@ -2176,12 +2176,47 @@ static bool parse_test_directory(int dirfd,
 {
 	int fds[_F_LAST];
 	struct subtest_list subtests = {};
-	bool status = true;
 	int commsparsed;
 
-	if (!open_output_files(dirfd, fds, false)) {
-		fprintf(stderr, "Error opening output files\n");
-		return false;
+	if (!open_output_files_rdonly(dirfd, fds)) {
+		struct stat statbuf;
+		size_t fsizes[_F_LAST]; /* checks for both comm and out/err empty */
+		int fst_err;
+		bool valid_out_files = false;
+
+		for (int i = 0; i < _F_LAST; ++i) {
+			fsizes[i] = 0;
+			if (fds[i] > 0) {
+				fst_err = fstat(fds[i], &statbuf);
+				if (!fst_err)
+					fsizes[i] = statbuf.st_size;
+				else
+					fprintf(stderr, "results: %s: stats failed: %s\n",
+						get_out_filename(i), strerror(fst_err));
+			} else {
+				fprintf(stderr, "results: %s: open failed: %s\n",
+					get_out_filename(i), strerror(-fds[i]));
+			}
+		}
+
+		if (fsizes[_F_SOCKET] || fsizes[_F_OUT] || fsizes[_F_ERR] ||
+		    fsizes[_F_DMESG])
+			valid_out_files = true;
+		else
+			valid_out_files = false;
+
+		if (!valid_out_files) {
+			/* no output saved in any file */
+			fprintf(stderr, "results: no valid output files\n");
+			close_outputs(fds);
+
+			return false;
+		}
+
+		if (!fsizes[_F_SOCKET] && !fsizes[_F_OUT] && !fsizes[_F_ERR])
+			fprintf(stderr, "results: comms/err/out empty, using only dmesg\n");
+
+		fprintf(stderr, "results: missing few output file(s)\n");
 	}
 
 	/*
@@ -2191,8 +2226,6 @@ static bool parse_test_directory(int dirfd,
 	commsparsed = fill_from_comms(fds[_F_SOCKET], entry, &subtests, results);
 	if (commsparsed == COMMSPARSE_ERROR) {
 		fprintf(stderr, "Error parsing output files (comms)\n");
-		status = false;
-		goto parse_output_end;
 	}
 
 	if (commsparsed == COMMSPARSE_EMPTY) {
@@ -2200,20 +2233,17 @@ static bool parse_test_directory(int dirfd,
 		 * fill_from_journal fills the subtests struct and
 		 * adds timeout results where applicable.
 		 */
-		fill_from_journal(fds[_F_JOURNAL], entry, &subtests, results);
+		if (fds[_F_JOURNAL] > 0)
+			fill_from_journal(fds[_F_JOURNAL], entry, &subtests, results);
 
 		if (!fill_from_output(fds[_F_OUT], entry->binary, "out", &subtests, results->tests) ||
 		    !fill_from_output(fds[_F_ERR], entry->binary, "err", &subtests, results->tests)) {
 			fprintf(stderr, "Error parsing output files (out.txt, err.txt)\n");
-			status = false;
-			goto parse_output_end;
 		}
 	}
 
 	if (!fill_from_dmesg(fds[_F_DMESG], settings, entry->binary, &subtests, results->tests)) {
 		fprintf(stderr, "Error parsing output files (dmesg.txt)\n");
-		status = false;
-		goto parse_output_end;
 	}
 
 	override_results(entry->binary, &subtests, results->tests);
@@ -2221,11 +2251,10 @@ static bool parse_test_directory(int dirfd,
 
 	add_to_totals(entry->binary, &subtests, results);
 
- parse_output_end:
 	close_outputs(fds);
 	free_subtests(&subtests);
 
-	return status;
+	return true;
 }
 
 static void try_add_notrun_results(const struct job_list_entry *entry,
@@ -2359,14 +2388,21 @@ struct json_object *generate_results_json(int dirfd)
 		char name[16];
 
 		snprintf(name, 16, "%zd", i);
+		fprintf(stderr, "results: parsing output: %s/ for test: %s\n",
+			name, job_list.entries[i].binary);
 		if ((testdirfd = openat(dirfd, name, O_DIRECTORY | O_RDONLY)) < 0) {
+			if (settings.log_level >= LOG_LEVEL_NORMAL)
+				fprintf(stderr, "results: no output, setting notrun\n");
+
 			try_add_notrun_results(&job_list.entries[i], &settings, &results);
 			continue;
 		}
 
 		if (!parse_test_directory(testdirfd, &job_list.entries[i], &settings, &results)) {
-			close(testdirfd);
-			return NULL;
+			if (settings.log_level >= LOG_LEVEL_NORMAL)
+				fprintf(stderr, "results: no useful output, setting notrun\n");
+
+			try_add_notrun_results(&job_list.entries[i], &settings, &results);
 		}
 		close(testdirfd);
 	}
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-03-03 11:03 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-26 19:33 [PATCH i-g-t] runner: Parse results harder Kamil Konieczny
2025-02-27 16:37 ` Knop, Ryszard
2025-02-28 17:08   ` Kamil Konieczny
2025-02-28  0:49 ` ✓ Xe.CI.BAT: success for " Patchwork
2025-02-28  1:06 ` ✓ i915.CI.BAT: " Patchwork
2025-02-28  5:21 ` ✗ Xe.CI.Full: failure " Patchwork
2025-02-28 15:52   ` Kamil Konieczny
2025-02-28 10:59 ` ✗ i915.CI.Full: " Patchwork
2025-02-28 15:53   ` Kamil Konieczny
  -- strict thread matches above, loose matches on Subject: below --
2025-02-28 17:15 [PATCH i-g-t] " Kamil Konieczny
2025-03-03 11:02 ` Krzysztof Karas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox