[ptest-runner][PATCH 0/4] Stop running ptests in parallel

[ptest-runner][PATCH 0/4] Stop running ptests in parallel

[Joshua Watt]

07d8a67 ("runner: Correctly handle running parallel tests") made an
incorrect assumption that it was OK to run ptests in parallel and
interleave the output. This is not correct since OE selftest expects all
text between a BEGIN and END block to be for a single ptest. Revert the
commit to run in parallel and add the correct fix for the bug with
running multiple ptests in a single invocation, as well as a few other
fixes

Joshua Watt (4):
  Revert "runner: Correctly handle running parallel tests"
  Recreate pipe for each test
  Report if child dies from a signal
  Change test timeout to be total elapsed time

 tests/data/hang/ptest/run-ptest   |   1 +
 tests/data/signal/ptest/run-ptest |  10 ++
 tests/utils.c                     |   4 +-
 utils.c                           | 290 +++++++++++++-----------------
 4 files changed, 140 insertions(+), 165 deletions(-)
 create mode 100755 tests/data/signal/ptest/run-ptest

-- 
2.33.0

[ptest-runner][PATCH 1/4] Revert "runner: Correctly handle running parallel tests"

[Joshua Watt]

This reverts commit 07d8a676aa962ecc5ec264ec33b0074adf2a8733.

This commit incorrectly assumed that ptest ran tests in parallel, which
is not true. Doing so interleaves the test output in the log file, which
the OE selftest parser cannot handle and thus breaks the test cases.

Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
---
 tests/utils.c |  39 +-------
 utils.c       | 248 +++++++++++++++++++++-----------------------------
 2 files changed, 105 insertions(+), 182 deletions(-)

diff --git a/tests/utils.c b/tests/utils.c
index 4560e93..19657ee 100644
--- a/tests/utils.c
+++ b/tests/utils.c
@@ -50,10 +50,9 @@ static char *ptests_found[] = {
 	"glibc",
 	"hang",
 	"python",
-	"signal",
 	NULL
 };
-static int ptests_found_length = 7;
+static int ptests_found_length = 6;
 static char *ptests_not_found[] = {
 	"busybox",
 	"perl",
@@ -187,7 +186,6 @@ START_TEST(test_run_ptests)
 	head = get_available_ptests(opts_directory);
 	ptest_list_remove(head, "hang", 1);
 	ptest_list_remove(head, "fail", 1);
-	ptest_list_remove(head, "signal", 1);
 
 	rc = run_ptests(head, opts, "test_run_ptests", fp_stdout, fp_stderr);
 	ck_assert(rc == 0);
@@ -217,8 +215,8 @@ search_for_timeout_and_duration(const int rp, FILE *fp_stdout)
 		found_duration = found_duration ? found_duration : find_word(line, duration_str);
 	}
 
-	ck_assert_msg(found_timeout == true, "TIMEOUT not found");
-	ck_assert_msg(found_duration == true, "DURATION not found");
+	ck_assert(found_timeout == true);
+	ck_assert(found_duration == true);
 }
 
 START_TEST(test_run_timeout_duration_ptest)
@@ -232,36 +230,6 @@ START_TEST(test_run_timeout_duration_ptest)
 }
 END_TEST
 
-static void
-search_for_signal_and_duration(const int rp, FILE *fp_stdout)
-{
-	char line_buf[PRINT_PTEST_BUF_SIZE];
-	bool found_error = false, found_duration = false;
-	char *line = NULL;
-
-	ck_assert(rp != 0);
-
-	while ((line = fgets(line_buf, PRINT_PTEST_BUF_SIZE, fp_stdout)) != NULL) {
-		// once true, stay true
-		found_error = found_error ? found_error : find_word(line, "ERROR: Exited with signal");
-		found_duration = found_duration ? found_duration : find_word(line, "DURATION");
-	}
-
-	ck_assert_msg(found_error == true, "ERROR not found");
-	ck_assert_msg(found_duration == true, "DURATION not found");
-}
-
-START_TEST(test_run_signal_ptest)
-{
-	struct ptest_list *head = get_available_ptests(opts_directory);
-	unsigned int timeout = 10;
-
-	test_ptest_expected_failure(head, timeout, "signal", search_for_signal_and_duration);
-
-	ptest_list_free_all(head);
-}
-END_TEST
-
 static void
 search_for_fail(const int rp, FILE *fp_stdout)
 {
@@ -350,7 +318,6 @@ utils_suite(void)
 	tcase_add_test(tc_core, test_filter_ptests);
 	tcase_add_test(tc_core, test_run_ptests);
 	tcase_add_test(tc_core, test_run_timeout_duration_ptest);
-	tcase_add_test(tc_core, test_run_signal_ptest);
 	tcase_add_test(tc_core, test_run_fail_ptest);
 	tcase_add_test(tc_core, test_xml_pass);
 	tcase_add_test(tc_core, test_xml_fail);
diff --git a/utils.c b/utils.c
index c444b2a..6a6e848 100644
--- a/utils.c
+++ b/utils.c
@@ -60,20 +60,11 @@ static struct {
 	FILE *fps[2];
 
 	unsigned int timeout;
+	int timeouted;
+	pid_t pid;
 	int padding1;
 } _child_reader;
 
-struct running_test {
-	struct running_test *next;
-	char *ptest_dir;
-	pid_t pid;
-	time_t start_time;
-	time_t end_time;
-	int status;
-	bool timed_out;
-	bool exited;
-};
-
 static inline char *
 get_stime(char *stime, size_t size, time_t t)
 {
@@ -353,18 +344,16 @@ run_child(char *run_ptest, int fd_stdout, int fd_stderr)
 	/* exit(1); not needed? */
 }
 
-static void
-wait_child(struct running_test *test, int options)
+static inline int
+wait_child(pid_t pid)
 {
-	int status;
+	int status = -1;
 
-	if (!test->exited) {
-		if (waitpid(test->pid, &status, options) == test->pid) {
-			test->status = status;
-			test->end_time = time(NULL);
-			test->exited = true;
-		}
-	}
+	waitpid(pid, &status, 0);
+	if (WIFEXITED(status))
+		status = WEXITSTATUS(status);
+
+	return status;
 }
 
 /* Returns an integer file descriptor.
@@ -427,9 +416,10 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 	pid_t child;
 	int pipefd_stdout[2];
 	int pipefd_stderr[2];
+	time_t sttime, entime;
+	time_t duration;
 	int slave;
 	int pgid = -1;
-	struct running_test *running_tests = NULL;
 
 	if (opts.xml_filename) {
 		xh = xml_create(ptest_list_length(head), opts.xml_filename);
@@ -457,6 +447,7 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 		_child_reader.fps[0] = fp;
 		_child_reader.fps[1] = fp_stderr;
 		_child_reader.timeout = opts.timeout;
+		_child_reader.timeouted = 0;
 
 		fprintf(fp, "START: %s\n", progname);
 		PTEST_LIST_ITERATE_START(head, p)
@@ -500,158 +491,123 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 				run_child(p->run_ptest, pipefd_stdout[1], pipefd_stderr[1]);
 
 			} else {
-				struct running_test *test = malloc(sizeof(*test));
-				test->pid = child;
-				test->status = 0;
-				test->timed_out = false;
-				test->exited = false;
-				test->ptest_dir = ptest_dir;
-				test->start_time = time(NULL);
-				test->next = running_tests;
-				running_tests = test;
+				int status;
+
+				/* Close write ends of the pipe, otherwise this process will never get EOF when the child dies */
+				do_close(&pipefd_stdout[1]);
+				do_close(&pipefd_stderr[1]);
 
+				_child_reader.pid = child;
 				if (setpgid(child, pgid) == -1) {
 					fprintf(fp, "ERROR: setpgid() failed, %s\n", strerror(errno));
 				}
 
-				fprintf(fp, "%s\n", get_stime(stime, GET_STIME_BUF_SIZE, test->start_time));
+				sttime = time(NULL);
+				fprintf(fp, "%s\n", get_stime(stime, GET_STIME_BUF_SIZE, sttime));
 				fprintf(fp, "BEGIN: %s\n", ptest_dir);
-			}
-		PTEST_LIST_ITERATE_END
-
-		/*
-		 * Close write ends of the pipe, otherwise this process will
-		 * never get EOF when the child dies
-		 */
-		do_close(&pipefd_stdout[1]);
-		do_close(&pipefd_stderr[1]);
 
-		set_nonblocking(_child_reader.fds[0]);
-		set_nonblocking(_child_reader.fds[1]);
+				set_nonblocking(_child_reader.fds[0]);
+				set_nonblocking(_child_reader.fds[1]);
 
-		struct pollfd pfds[2];
-		for (int i = 0; i < 2; i++) {
-			pfds[i].fd = _child_reader.fds[i];
-			pfds[i].events = POLLIN;
-		}
-
-		while (true) {
-			/*
-			 * Check all the poll file descriptors. Only when all
-			 * of them are done (negative) will the poll() loop
-			 * exit. This ensures all output is read from all
-			 * children.
-			 */
-			bool done = true;
-			for (int i = 0; i < 2; i++) {
-				if (pfds[i].fd >= 0) {
-					done = false;
-					break;
+				struct pollfd pfds[2];
+				for (int i = 0; i < 2; i++) {
+					pfds[i].fd = _child_reader.fds[i];
+					pfds[i].events = POLLIN;
 				}
-			}
 
-			if (done) {
-				break;
-			}
-
-			int ret = poll(pfds, 2, _child_reader.timeout*1000);
+				while (true) {
+					/*
+					 * Check all the poll file descriptors.
+					 * Only when all of them are done
+					 * (negative) will we exit the poll()
+					 * loop
+					 */
+					bool done = true;
+					for (int i = 0; i < 2; i++) {
+						if (pfds[i].fd >= 0) {
+							done = false;
+							break;
+						}
+					}
 
-			for (int i = 0; i < 2; i++) {
-				if (pfds[i].revents & (POLLIN | POLLHUP)) {
-					char buf[WAIT_CHILD_BUF_MAX_SIZE];
-					ssize_t n = read(pfds[i].fd, buf, sizeof(buf));
+					if (done) {
+						break;
+					}
 
-					if (n == 0) {
-						/* Closed */
-						pfds[i].fd = -1;
-						continue;
+					int ret = poll(pfds, 2, _child_reader.timeout*1000);
+
+					if (ret == 0 && !_child_reader.timeouted) {
+						/* kill the child if we haven't
+						 * already. Note that we
+						 * continue to read data from
+						 * the pipes until EOF to make
+						 * sure we get all the output
+						 */
+						kill(-_child_reader.pid, SIGKILL);
+						_child_reader.timeouted = 1;
 					}
 
-					if (n < 0) {
-						if (errno != EAGAIN && errno != EWOULDBLOCK) {
-							pfds[i].fd = -1;
-							fprintf(stderr, "Error reading from stream %d: %s\n", i, strerror(errno));
+					for (int i = 0; i < 2; i++) {
+						if (pfds[i].revents & (POLLIN | POLLHUP)) {
+							char buf[WAIT_CHILD_BUF_MAX_SIZE];
+							ssize_t n = read(pfds[i].fd, buf, sizeof(buf));
+
+							if (n == 0) {
+								/* Closed */
+								pfds[i].fd = -1;
+								continue;
+							}
+
+							if (n < 0) {
+								if (errno != EAGAIN && errno != EWOULDBLOCK) {
+									pfds[i].fd = -1;
+									fprintf(stderr, "Error reading from stream %d: %s\n", i, strerror(errno));
+								}
+								continue;
+							} else {
+								fwrite(buf, (size_t)n, 1, _child_reader.fps[i]);
+							}
 						}
-						continue;
-					} else {
-						fwrite(buf, (size_t)n, 1, _child_reader.fps[i]);
 					}
 				}
-			}
+				collect_system_state(_child_reader.fps[0]);
 
-			for (struct running_test *test = running_tests; test; test = test->next) {
-				/*
-				 * Check if this child has exited yet.
-				 */
-				wait_child(test, WNOHANG);
+				for (int i = 0; i < 2; i++) {
+					fflush(_child_reader.fps[i]);
+				}
 
 				/*
-				 * If a timeout has occurred, kill the child if
-				 * it has not been done already and has not
-				 * exited
+				 * This kill is just in case the child did
+				 * something really silly like close its
+				 * stdout and stderr but then go into an
+				 * infinite loop and never exit. Normally, it
+				 * will just fail because the child is already
+				 * dead
 				 */
-				if (ret == 0 && !test->exited && !test->timed_out) {
-					kill(-test->pid, SIGKILL);
-					test->timed_out = true;
+				if (!_child_reader.timeouted) {
+					kill(-_child_reader.pid, SIGKILL);
 				}
-			}
-		}
-		collect_system_state(_child_reader.fps[0]);
-
-		for (int i = 0; i < 2; i++) {
-			fflush(_child_reader.fps[i]);
-		}
-
-		for (struct running_test *test = running_tests; test; test = test->next) {
-			time_t duration;
-			int exit_code = 0;
-
-			/*
-			 * This kill is just in case the child did something really
-			 * silly like close its stdout and stderr but then go into an
-			 * infinite loop and never exit. Normally, it will just fail
-			 * because the child is already dead
-			 */
-			if (!test->timed_out && !test->exited) {
-				kill(-test->pid, SIGKILL);
-			}
+				status = wait_child(child);
 
-			wait_child(test, 0);
+				entime = time(NULL);
+				duration = entime - sttime;
 
-			duration = test->end_time - test->start_time;
-
-			if (WIFEXITED(test->status)) {
-				exit_code = WEXITSTATUS(test->status);
-				if (exit_code) {
-					fprintf(fp, "\nERROR: Exit status is %d\n", exit_code);
+				if (status) {
+					fprintf(fp, "\nERROR: Exit status is %d\n", status);
 					rc += 1;
 				}
-			} else if (WIFSIGNALED(test->status) && !test->timed_out) {
-				int signal = WTERMSIG(test->status);
-				fprintf(fp, "\nERROR: Exited with signal %s (%d)\n", strsignal(signal), signal);
-				rc += 1;
-			}
-			fprintf(fp, "DURATION: %d\n", (int) duration);
-			if (test->timed_out) {
-				fprintf(fp, "TIMEOUT: %s\n", test->ptest_dir);
-				rc += 1;
-			}
-
-			if (opts.xml_filename)
-				xml_add_case(xh, exit_code, test->ptest_dir, test->timed_out, (int) duration);
-
-			fprintf(fp, "END: %s\n", test->ptest_dir);
-			fprintf(fp, "%s\n", get_stime(stime, GET_STIME_BUF_SIZE, test->end_time));
-		}
-
-		while (running_tests) {
-			struct running_test *test = running_tests;
-			running_tests = running_tests->next;
+				fprintf(fp, "DURATION: %d\n", (int) duration);
+				if (_child_reader.timeouted)
+					fprintf(fp, "TIMEOUT: %s\n", ptest_dir);
 
-			free(test->ptest_dir);
-			free(test);
-		}
+				if (opts.xml_filename)
+					xml_add_case(xh, status, ptest_dir, _child_reader.timeouted, (int) duration);
 
+				fprintf(fp, "END: %s\n", ptest_dir);
+				fprintf(fp, "%s\n", get_stime(stime, GET_STIME_BUF_SIZE, entime));
+			}
+			free(ptest_dir);
+		PTEST_LIST_ITERATE_END
 		fprintf(fp, "STOP: %s\n", progname);
 
 		do_close(&pipefd_stdout[0]);
-- 
2.33.0

[ptest-runner][PATCH 2/4] Recreate pipe for each test

[Joshua Watt]

The write end of the pipe has to be closed by ptest-runner to make sure
that it will get EOF when the child process is done with it. This means
that a new pipe needs to be opened for each child so that the write ends
can be passed to it.

Fixes the problem where tests would be reported with no output when
ptest-runner was told to run multiple tests because they were passed
invalid stdin/stderr pipes.

Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
---
 utils.c | 45 ++++++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/utils.c b/utils.c
index 6a6e848..c1188dd 100644
--- a/utils.c
+++ b/utils.c
@@ -61,7 +61,6 @@ static struct {
 
 	unsigned int timeout;
 	int timeouted;
-	pid_t pid;
 	int padding1;
 } _child_reader;
 
@@ -414,8 +413,8 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 	char stime[GET_STIME_BUF_SIZE];
 
 	pid_t child;
-	int pipefd_stdout[2];
-	int pipefd_stderr[2];
+	int pipefd_stdout[2] = {-1, -1};
+	int pipefd_stderr[2] = {-1, -1};
 	time_t sttime, entime;
 	time_t duration;
 	int slave;
@@ -429,28 +428,22 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 
 	do
 	{
-		if ((rc = pipe2(pipefd_stdout, 0)) == -1)
-			break;
-
-		if ((rc = pipe2(pipefd_stderr, 0)) == -1) {
-			close(pipefd_stdout[0]);
-			close(pipefd_stdout[1]);
-			break;
-		}
-
 		if (isatty(0) && ioctl(0, TIOCNOTTY) == -1) {
 			fprintf(fp, "ERROR: Unable to detach from controlling tty, %s\n", strerror(errno));
 		}
 
-		_child_reader.fds[0] = pipefd_stdout[0];
-		_child_reader.fds[1] = pipefd_stderr[0];
-		_child_reader.fps[0] = fp;
-		_child_reader.fps[1] = fp_stderr;
-		_child_reader.timeout = opts.timeout;
-		_child_reader.timeouted = 0;
 
 		fprintf(fp, "START: %s\n", progname);
 		PTEST_LIST_ITERATE_START(head, p)
+			if ((rc = pipe2(pipefd_stdout, 0)) == -1)
+				break;
+
+			if ((rc = pipe2(pipefd_stderr, 0)) == -1) {
+				close(pipefd_stdout[0]);
+				close(pipefd_stdout[1]);
+				break;
+			}
+
 			char *ptest_dir = strdup(p->run_ptest);
 			if (ptest_dir == NULL) {
 				rc = -1;
@@ -497,7 +490,12 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 				do_close(&pipefd_stdout[1]);
 				do_close(&pipefd_stderr[1]);
 
-				_child_reader.pid = child;
+				_child_reader.fds[0] = pipefd_stdout[0];
+				_child_reader.fds[1] = pipefd_stderr[0];
+				_child_reader.fps[0] = fp;
+				_child_reader.fps[1] = fp_stderr;
+				_child_reader.timeout = opts.timeout;
+				_child_reader.timeouted = 0;
 				if (setpgid(child, pgid) == -1) {
 					fprintf(fp, "ERROR: setpgid() failed, %s\n", strerror(errno));
 				}
@@ -543,7 +541,7 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 						 * the pipes until EOF to make
 						 * sure we get all the output
 						 */
-						kill(-_child_reader.pid, SIGKILL);
+						kill(-child, SIGKILL);
 						_child_reader.timeouted = 1;
 					}
 
@@ -585,7 +583,7 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 				 * dead
 				 */
 				if (!_child_reader.timeouted) {
-					kill(-_child_reader.pid, SIGKILL);
+					kill(-child, SIGKILL);
 				}
 				status = wait_child(child);
 
@@ -607,6 +605,11 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 				fprintf(fp, "%s\n", get_stime(stime, GET_STIME_BUF_SIZE, entime));
 			}
 			free(ptest_dir);
+			do_close(&pipefd_stdout[0]);
+			do_close(&pipefd_stdout[1]);
+			do_close(&pipefd_stderr[0]);
+			do_close(&pipefd_stderr[1]);
+
 		PTEST_LIST_ITERATE_END
 		fprintf(fp, "STOP: %s\n", progname);
 
-- 
2.33.0

[ptest-runner][PATCH 3/4] Report if child dies from a signal

[Joshua Watt]

Tests can exit due to a signal, which should also be reported in the
test output.

Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
---
 tests/data/signal/ptest/run-ptest | 10 ++++++++
 tests/utils.c                     | 39 +++++++++++++++++++++++++++---
 utils.c                           | 40 +++++++++++++++++--------------
 3 files changed, 68 insertions(+), 21 deletions(-)
 create mode 100755 tests/data/signal/ptest/run-ptest

diff --git a/tests/data/signal/ptest/run-ptest b/tests/data/signal/ptest/run-ptest
new file mode 100755
index 0000000..93ac65b
--- /dev/null
+++ b/tests/data/signal/ptest/run-ptest
@@ -0,0 +1,10 @@
+#!/bin/sh
+
+echo "signal" 1>&2
+echo "signal"
+
+kill -ABRT $$
+
+while true; do
+	sleep 1
+done
diff --git a/tests/utils.c b/tests/utils.c
index 19657ee..d82b90e 100644
--- a/tests/utils.c
+++ b/tests/utils.c
@@ -50,9 +50,10 @@ static char *ptests_found[] = {
 	"glibc",
 	"hang",
 	"python",
+	"signal",
 	NULL
 };
-static int ptests_found_length = 6;
+static int ptests_found_length = 7;
 static char *ptests_not_found[] = {
 	"busybox",
 	"perl",
@@ -186,6 +187,7 @@ START_TEST(test_run_ptests)
 	head = get_available_ptests(opts_directory);
 	ptest_list_remove(head, "hang", 1);
 	ptest_list_remove(head, "fail", 1);
+	ptest_list_remove(head, "signal", 1);
 
 	rc = run_ptests(head, opts, "test_run_ptests", fp_stdout, fp_stderr);
 	ck_assert(rc == 0);
@@ -215,8 +217,8 @@ search_for_timeout_and_duration(const int rp, FILE *fp_stdout)
 		found_duration = found_duration ? found_duration : find_word(line, duration_str);
 	}
 
-	ck_assert(found_timeout == true);
-	ck_assert(found_duration == true);
+	ck_assert_msg(found_timeout == true, "TIMEOUT not found");
+	ck_assert_msg(found_duration == true, "DURATION not found");
 }
 
 START_TEST(test_run_timeout_duration_ptest)
@@ -230,6 +232,36 @@ START_TEST(test_run_timeout_duration_ptest)
 }
 END_TEST
 
+static void
+search_for_signal_and_duration(const int rp, FILE *fp_stdout)
+{
+	char line_buf[PRINT_PTEST_BUF_SIZE];
+	bool found_error = false, found_duration = false;
+	char *line = NULL;
+
+	ck_assert(rp != 0);
+
+	while ((line = fgets(line_buf, PRINT_PTEST_BUF_SIZE, fp_stdout)) != NULL) {
+		// once true, stay true
+		found_error = found_error ? found_error : find_word(line, "ERROR: Exited from signal");
+		found_duration = found_duration ? found_duration : find_word(line, "DURATION");
+	}
+
+	ck_assert_msg(found_error == true, "ERROR not found");
+	ck_assert_msg(found_duration == true, "DURATION not found");
+}
+
+START_TEST(test_run_signal_ptest)
+{
+	struct ptest_list *head = get_available_ptests(opts_directory);
+	unsigned int timeout = 10;
+
+	test_ptest_expected_failure(head, timeout, "signal", search_for_signal_and_duration);
+
+	ptest_list_free_all(head);
+}
+END_TEST
+
 static void
 search_for_fail(const int rp, FILE *fp_stdout)
 {
@@ -318,6 +350,7 @@ utils_suite(void)
 	tcase_add_test(tc_core, test_filter_ptests);
 	tcase_add_test(tc_core, test_run_ptests);
 	tcase_add_test(tc_core, test_run_timeout_duration_ptest);
+	tcase_add_test(tc_core, test_run_signal_ptest);
 	tcase_add_test(tc_core, test_run_fail_ptest);
 	tcase_add_test(tc_core, test_xml_pass);
 	tcase_add_test(tc_core, test_xml_fail);
diff --git a/utils.c b/utils.c
index c1188dd..34ca2f0 100644
--- a/utils.c
+++ b/utils.c
@@ -343,18 +343,6 @@ run_child(char *run_ptest, int fd_stdout, int fd_stderr)
 	/* exit(1); not needed? */
 }
 
-static inline int
-wait_child(pid_t pid)
-{
-	int status = -1;
-
-	waitpid(pid, &status, 0);
-	if (WIFEXITED(status))
-		status = WEXITSTATUS(status);
-
-	return status;
-}
-
 /* Returns an integer file descriptor.
  * If it returns < 0, an error has occurred.
  * Otherwise, it has returned the slave pty file descriptor.
@@ -585,21 +573,37 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 				if (!_child_reader.timeouted) {
 					kill(-child, SIGKILL);
 				}
-				status = wait_child(child);
+				waitpid(child, &status, 0);
 
 				entime = time(NULL);
 				duration = entime - sttime;
 
-				if (status) {
-					fprintf(fp, "\nERROR: Exit status is %d\n", status);
-					rc += 1;
+				int exit_code = 0;
+
+				if (!_child_reader.timeouted) {
+					if (WIFEXITED(status)) {
+						exit_code = WEXITSTATUS(status);
+						if (exit_code) {
+							fprintf(fp, "\nERROR: Exit status is %d\n", exit_code);
+							rc += 1;
+						}
+					} else if (WIFSIGNALED(status)) {
+						int signal = WTERMSIG(status);
+						fprintf(fp, "\nERROR: Exited from signal %s (%d)\n", strsignal(signal), signal);
+						rc += 1;
+					} else {
+						fprintf(fp, "\nERROR: Exited for unknown reason (%d)\n", status);
+						rc += 1;
+					}
 				}
 				fprintf(fp, "DURATION: %d\n", (int) duration);
-				if (_child_reader.timeouted)
+				if (_child_reader.timeouted) {
 					fprintf(fp, "TIMEOUT: %s\n", ptest_dir);
+					rc += 1;
+				}
 
 				if (opts.xml_filename)
-					xml_add_case(xh, status, ptest_dir, _child_reader.timeouted, (int) duration);
+					xml_add_case(xh, exit_code, ptest_dir, _child_reader.timeouted, (int) duration);
 
 				fprintf(fp, "END: %s\n", ptest_dir);
 				fprintf(fp, "%s\n", get_stime(stime, GET_STIME_BUF_SIZE, entime));
-- 
2.33.0

[ptest-runner][PATCH 4/4] Change test timeout to be total elapsed time

[Joshua Watt]

Changes the way that tests time out to be the total elapsed time for the
test, not just the time between receiving output from the test. This
matches the implementation before 8259375 ("runner: Remove threads and
mutexes").

Also update the timeout test case to test for this correctly.

Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
---
 tests/data/hang/ptest/run-ptest |  1 +
 tests/utils.c                   |  2 +-
 utils.c                         | 23 ++++++++++++-----------
 3 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/tests/data/hang/ptest/run-ptest b/tests/data/hang/ptest/run-ptest
index 738041d..9031be3 100755
--- a/tests/data/hang/ptest/run-ptest
+++ b/tests/data/hang/ptest/run-ptest
@@ -3,5 +3,6 @@
 echo "hang" 1>&2
 echo "hang"
 while true; do
+	echo "hang"
 	sleep 1
 done
diff --git a/tests/utils.c b/tests/utils.c
index d82b90e..849a412 100644
--- a/tests/utils.c
+++ b/tests/utils.c
@@ -224,7 +224,7 @@ search_for_timeout_and_duration(const int rp, FILE *fp_stdout)
 START_TEST(test_run_timeout_duration_ptest)
 {
 	struct ptest_list *head = get_available_ptests(opts_directory);
-	unsigned int timeout = 1;
+	unsigned int timeout = 3;
 
 	test_ptest_expected_failure(head, timeout, "hang", search_for_timeout_and_duration);
 
diff --git a/utils.c b/utils.c
index 34ca2f0..353d6dc 100644
--- a/utils.c
+++ b/utils.c
@@ -403,7 +403,8 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 	pid_t child;
 	int pipefd_stdout[2] = {-1, -1};
 	int pipefd_stderr[2] = {-1, -1};
-	time_t sttime, entime;
+	time_t sttime, entime, now;
+	time_t timeout_deadline;
 	time_t duration;
 	int slave;
 	int pgid = -1;
@@ -489,6 +490,7 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 				}
 
 				sttime = time(NULL);
+				timeout_deadline = sttime + opts.timeout;
 				fprintf(fp, "%s\n", get_stime(stime, GET_STIME_BUF_SIZE, sttime));
 				fprintf(fp, "BEGIN: %s\n", ptest_dir);
 
@@ -519,18 +521,17 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
 					if (done) {
 						break;
 					}
-
-					int ret = poll(pfds, 2, _child_reader.timeout*1000);
-
-					if (ret == 0 && !_child_reader.timeouted) {
-						/* kill the child if we haven't
-						 * already. Note that we
-						 * continue to read data from
-						 * the pipes until EOF to make
-						 * sure we get all the output
-						 */
+					now = time(NULL);
+					if (now >= timeout_deadline) {
 						kill(-child, SIGKILL);
 						_child_reader.timeouted = 1;
+						break;
+					}
+
+					int ret = poll(pfds, 2, (timeout_deadline - now) * 1000);
+
+					if (ret == 0) {
+						continue;
 					}
 
 					for (int i = 0; i < 2; i++) {
-- 
2.33.0