[PATCH i-g-t 15/28] State machine goes brr

Petri Latvala petri.latvala at intel.com
Mon Mar 22 11:34:07 UTC 2021


---
 runner/resultgen.c | 662 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 514 insertions(+), 148 deletions(-)

diff --git a/runner/resultgen.c b/runner/resultgen.c
index 5bede2e6..705b2749 100644
--- a/runner/resultgen.c
+++ b/runner/resultgen.c
@@ -1228,46 +1228,59 @@ static void fill_from_journal(int fd,
 	fclose(f);
 }
 
+typedef enum comms_state {
+	STATE_INITIAL = 0,
+	STATE_AFTER_EXEC,
+	STATE_SUBTEST_STARTED,
+	STATE_DYNAMIC_SUBTEST_STARTED,
+	STATE_BETWEEN_DYNAMIC_SUBTESTS,
+	STATE_BETWEEN_SUBTESTS,
+	STATE_EXITED,
+	STATE_ERROR
+} comms_state_t;
+
 struct comms_context
 {
+	comms_state_t state;
+
+	struct json_object *binaryruntimeobj;
 	struct json_object *current_test;
+	struct json_object *current_dynamic_subtest;
 	char *current_subtest_name;
+	char *current_dynamic_subtest_name;
+
 	char *outbuf, *errbuf;
 	size_t outbuflen, errbuflen;
-	ssize_t commonoutbufmarker, commonerrbufmarker;
+	size_t outidx, nextoutidx;
+	size_t erridx, nexterridx;
+	size_t dynoutidx, nextdynoutidx;
+	size_t dynerridx, nextdynerridx;
+
 	char *igt_version;
+
 	char *subtestresult;
+	char *dynamicsubtestresult;
+
 	char *cmdline;
 	int exitcode;
-};
 
-static void comms_init_context(struct comms_context *context)
-{
-	memset(context, 0, sizeof(*context));
-
-	context->commonoutbufmarker = -1;
-	context->commonerrbufmarker = -1;
-}
+	struct subtest_list *subtests;
+	struct subtest *subtest;
+	struct results *results;
+	struct job_list_entry *entry;
+	const char *binary;
+};
 
 static void comms_free_context(struct comms_context *context)
 {
-	
-}
-
-static void comms_append_log(struct comms_context *context,
-			     union runnerpacket_read_helper helper)
-{
-	char **textbuf;
-	size_t *textlen;
-
-	if (helper.log.stream == STDOUT_FILENO) {
-		textbuf = &context->outbuf;
-		textlen = &context->outbuflen;
-	} else {
-		textbuf = &context->errbuf;
-		textlen = &context->errbuflen;
-	}
-	append_line(textbuf, textlen, helper.log.text);
+	free(context->current_subtest_name);
+	free(context->current_dynamic_subtest_name);
+	free(context->outbuf);
+	free(context->errbuf);
+	free(context->igt_version);
+	free(context->subtestresult);
+	free(context->dynamicsubtestresult);
+	free(context->cmdline);
 }
 
 static void comms_inject_subtest_start_log(struct comms_context *context,
@@ -1296,13 +1309,10 @@ static void comms_inject_subtest_end_log(struct comms_context *context,
 
 static void comms_finish_subtest(struct comms_context *context)
 {
-	size_t commonoutlen = 0;
-	size_t commonerrlen = 0;
-
 	json_object_object_add(context->current_test, "out",
-			       new_escaped_json_string(context->outbuf, context->outbuflen));
+			       new_escaped_json_string(context->outbuf + context->outidx, context->outbuflen - context->outidx));
 	json_object_object_add(context->current_test, "err",
-			       new_escaped_json_string(context->errbuf, context->errbuflen));
+			       new_escaped_json_string(context->errbuf + context->outidx, context->errbuflen - context->erridx));
 
 	if (context->igt_version)
 		add_igt_version(context->current_test, context->igt_version, strlen(context->igt_version));
@@ -1313,38 +1323,472 @@ static void comms_finish_subtest(struct comms_context *context)
 
 	free(context->subtestresult);
 	context->subtestresult = NULL;
+	context->current_test = NULL;
+
+	context->outidx = context->nextoutidx;
+	context->erridx = context->nexterridx;
+}
+
+static void comms_finish_dynamic_subtest(struct comms_context *context)
+{
+	json_object_object_add(context->current_dynamic_subtest, "out",
+			       new_escaped_json_string(context->outbuf + context->dynoutidx, context->outbuflen - context->dynoutidx));
+	json_object_object_add(context->current_dynamic_subtest, "err",
+			       new_escaped_json_string(context->errbuf + context->dynerridx, context->errbuflen - context->dynerridx));
+
+	if (context->igt_version)
+		add_igt_version(context->current_dynamic_subtest, context->igt_version, strlen(context->igt_version));
+
+	if (context->dynamicsubtestresult == NULL)
+		context->dynamicsubtestresult = strdup("incomplete");
+	set_result(context->current_dynamic_subtest, context->dynamicsubtestresult);
 
-	/* If we have commonbuf markers, we have overlapping logs that need to be also added to the next subtest */
-	if (context->commonoutbufmarker >= 0) {
-		commonoutlen = context->outbuflen - context->commonoutbufmarker;
-		memmove(context->outbuf, context->outbuf + context->commonoutbufmarker, commonoutlen);
+	free(context->dynamicsubtestresult);
+	context->dynamicsubtestresult = NULL;
+	context->current_dynamic_subtest = NULL;
+
+	context->dynoutidx = context->nextdynoutidx;
+	context->dynerridx = context->nextdynerridx;
+}
+
+static void comms_add_new_subtest(struct comms_context *context,
+				  const char *subtestname)
+{
+	char piglit_name[256];
+
+	add_subtest(context->subtests, strdup(subtestname));
+	context->subtest = &context->subtests->subs[context->subtests->size - 1];
+	generate_piglit_name(context->binary, subtestname, piglit_name, sizeof(piglit_name));
+	context->current_test = get_or_create_json_object(context->results->tests, piglit_name);
+	free(context->current_subtest_name);
+	context->current_subtest_name = strdup(subtestname);
+}
+
+static void comms_add_new_dynamic_subtest(struct comms_context *context,
+					  const char *dynamic_name)
+{
+	char piglit_name[256];
+	char dynamic_piglit_name[256];
+
+	add_dynamic_subtest(context->subtest, strdup(dynamic_name));
+	generate_piglit_name(context->binary, context->current_subtest_name, piglit_name, sizeof(piglit_name));
+	generate_piglit_name_for_dynamic(piglit_name, dynamic_name, dynamic_piglit_name, sizeof(dynamic_piglit_name));
+	context->current_dynamic_subtest = get_or_create_json_object(context->results->tests, dynamic_piglit_name);
+	free(context->current_dynamic_subtest_name);
+	context->current_dynamic_subtest_name = strdup(dynamic_name);
+}
+
+static void comms_handle_log(struct comms_context *context,
+			     union runnerpacket_read_helper helper)
+{
+	char **textbuf;
+	size_t *textlen;
+
+	if (context->state == STATE_ERROR)
+		return;
+
+	if (helper.log.stream == STDOUT_FILENO) {
+		textbuf = &context->outbuf;
+		textlen = &context->outbuflen;
+	} else {
+		textbuf = &context->errbuf;
+		textlen = &context->errbuflen;
 	}
-	if (context->commonerrbufmarker >= 0) {
-		commonerrlen = context->errbuflen - context->commonerrbufmarker;
-		memmove(context->errbuf, context->errbuf + context->commonerrbufmarker, commonerrlen);
+	append_line(textbuf, textlen, helper.log.text);
+}
+
+static void comms_handle_exec(struct comms_context *context,
+			      union runnerpacket_read_helper helper)
+{
+	switch (context->state) {
+	case STATE_INITIAL:
+		break;
+
+	case STATE_AFTER_EXEC:
+		/*
+		 * Resume after an exec that didn't involve any
+		 * subtests. Resumes can only happen for tests with
+		 * subtests, so while we might have logs already
+		 * collected, we have nowhere to put them. The joblist
+		 * doesn't help, because the ordering is up to the
+		 * test.
+		 */
+		printf("Warning: Need to discard %zd bytes of logs, no subtest data\n", context->outbuflen + context->errbuflen);
+		context->outbuflen = context->errbuflen = 0;
+		context->outidx = context->erridx = 0;
+		context->nextoutidx = context->nexterridx = 0;
+		break;
+
+	case STATE_SUBTEST_STARTED:
+	case STATE_DYNAMIC_SUBTEST_STARTED:
+	case STATE_BETWEEN_DYNAMIC_SUBTESTS:
+	case STATE_BETWEEN_SUBTESTS:
+	case STATE_EXITED:
+		/* A resume exec, so we're already collecting data. */
+		assert(context->current_test != NULL);
+		comms_finish_subtest(context);
+		break;
+	case STATE_ERROR:
+		return;
+	default:
+		assert(false); /* unreachable */
 	}
 
-	context->outbuflen = commonoutlen;
-	context->errbuflen = commonerrlen;
-	context->commonoutbufmarker = -1;
-	context->commonerrbufmarker = -1;
+	free(context->cmdline);
+	context->cmdline = strdup(helper.exec.cmdline);
+
+	context->state = STATE_AFTER_EXEC;
+}
+
+static void comms_handle_exit(struct comms_context *context,
+			      union runnerpacket_read_helper helper)
+{
+	char piglit_name[256];
+
+	if (context->state == STATE_ERROR)
+		return;
+
+	if (context->state == STATE_AFTER_EXEC) {
+		/*
+		 * Exit after exec, so we didn't get any
+		 * subtests. Check if there's supposed to be any,
+		 * otherwise stuff logs into the binary's result.
+		 */
+
+		char *subtestname = NULL;
+
+		if (context->entry->subtest_count > 0) {
+			subtestname = context->entry->subtests[0];
+			add_subtest(context->subtests, strdup(subtestname));
+		}
+		generate_piglit_name(context->binary, subtestname, piglit_name, sizeof(piglit_name));
+		context->current_test = get_or_create_json_object(context->results->tests, piglit_name);
+
+		/* Get result from exitcode unless we have an override already */
+		if (context->subtestresult == NULL)
+			context->subtestresult = strdup(result_from_exitcode(helper.exit.exitcode));
+	} else if (helper.exit.exitcode == IGT_EXIT_ABORT || helper.exit.exitcode == GRACEFUL_EXITCODE) {
+		/*
+		 * If we did get subtests, we need to assign the
+		 * special exitcode results to the last subtest,
+		 * normal and dynamic
+		 */
+		const char *result = helper.exit.exitcode == IGT_EXIT_ABORT ? "abort" : "notrun";
+
+		free(context->subtestresult);
+		context->subtestresult = strdup(result);
+		free(context->dynamicsubtestresult);
+		context->dynamicsubtestresult = strdup(result);
+	}
+
+	context->exitcode = helper.exit.exitcode;
+	add_runtime(context->binaryruntimeobj, strtod(helper.exit.timeused, NULL));
+
+	context->state = STATE_EXITED;
+}
+
+static void comms_handle_subtest_start(struct comms_context *context,
+				       union runnerpacket_read_helper helper)
+{
+	char errmsg[512];
+
+	switch (context->state) {
+	case STATE_INITIAL:
+	case STATE_EXITED:
+		/* Subtest starts when we're not even running? (Before exec or after exit) */
+		fprintf(stderr, "Error: Unexpected subtest start (binary wasn't running)\n");
+		context->state = STATE_ERROR;
+		return;
+	case STATE_SUBTEST_STARTED:
+	case STATE_DYNAMIC_SUBTEST_STARTED:
+	case STATE_BETWEEN_DYNAMIC_SUBTESTS:
+		/*
+		 * Subtest starts when the previous one was still
+		 * running. Text-based parsing would figure that a
+		 * resume happened, but we know the real deal with
+		 * socket comms.
+		 */
+		snprintf(errmsg, sizeof(errmsg),
+			 "\nrunner: Subtest %s already running when subtest %s starts. This is a test bug.\n",
+			 context->current_subtest_name,
+			 helper.subteststart.name);
+		append_line(&context->errbuf, &context->errbuflen, errmsg);
+
+		if (context->state == STATE_DYNAMIC_SUBTEST_STARTED ||
+		    context->state == STATE_BETWEEN_DYNAMIC_SUBTESTS)
+			comms_finish_dynamic_subtest(context);
+
+		/* fallthrough */
+	case STATE_BETWEEN_SUBTESTS:
+		/* Already collecting for a subtest, finish it up */
+		if (context->current_dynamic_subtest)
+			comms_finish_dynamic_subtest(context);
+
+		comms_finish_subtest(context);
+
+		/* fallthrough */
+	case STATE_AFTER_EXEC:
+		comms_add_new_subtest(context, helper.subteststart.name);
+
+		/* Subtest starting message is not in logs with socket comms, inject it manually */
+		comms_inject_subtest_start_log(context, STARTING_SUBTEST, helper.subteststart.name);
+
+		break;
+	case STATE_ERROR:
+		return;
+	default:
+		assert(false); /* unreachable */
+	}
+
+	context->state = STATE_SUBTEST_STARTED;
+}
+
+static void comms_handle_subtest_result(struct comms_context *context,
+					union runnerpacket_read_helper helper)
+{
+	char errmsg[512];
+
+	switch (context->state) {
+	case STATE_INITIAL:
+	case STATE_EXITED:
+		/* Subtest result when we're not even running? (Before exec or after exit) */
+		fprintf(stderr, "Error: Unexpected subtest result (binary wasn't running)\n");
+		context->state = STATE_ERROR;
+		return;
+	case STATE_DYNAMIC_SUBTEST_STARTED:
+		/*
+		 * Subtest result when dynamic subtest is still
+		 * running. Text-based parsing would consider that an
+		 * incomplete, we're able to inject a warning.
+		 */
+		snprintf(errmsg, sizeof(errmsg),
+			 "\nrunner: Dynamic subtest %s still running when subtest %s ended. This is a test bug.\n",
+			 context->current_dynamic_subtest_name,
+			 helper.subtestresult.name);
+		append_line(&context->errbuf, &context->errbuflen, errmsg);
+		comms_finish_dynamic_subtest(context);
+		break;
+	case STATE_BETWEEN_SUBTESTS:
+		/* Subtest result without starting it, and we're already collecting logs for a previous test */
+		comms_finish_subtest(context);
+		comms_add_new_subtest(context, helper.subtestresult.name);
+		break;
+	case STATE_AFTER_EXEC:
+		/* Subtest result without starting it, so comes from a fixture. We're not yet collecting logs for anything. */
+		comms_add_new_subtest(context, helper.subtestresult.name);
+		break;
+	case STATE_SUBTEST_STARTED:
+	case STATE_BETWEEN_DYNAMIC_SUBTESTS:
+		/* Normal flow */
+		break;
+	case STATE_ERROR:
+		return;
+	default:
+		assert(false); /* unreachable */
+	}
+
+	comms_inject_subtest_end_log(context,
+				     SUBTEST_RESULT,
+				     helper.subtestresult.name,
+				     helper.subtestresult.result,
+				     helper.subtestresult.timeused);
+
+	/* Next subtest, if any, will begin its logs right after that result line */
+	context->nextoutidx = context->outbuflen;
+	context->nexterridx = context->errbuflen;
+
+	/*
+	 * Only store the actual result from the packet if we don't
+	 * already have one. If we do, it's from an override.
+	 */
+	if (context->subtestresult == NULL) {
+		const char *mappedresult;
+
+		parse_result_string(helper.subtestresult.result,
+				    strlen(helper.subtestresult.result),
+				    &mappedresult, NULL);
+		context->subtestresult = strdup(mappedresult);
+	}
+
+	context->state = STATE_BETWEEN_SUBTESTS;
+}
+
+static void comms_handle_dynamic_subtest_start(struct comms_context *context,
+						union runnerpacket_read_helper helper)
+{
+	char errmsg[512];
+
+	switch (context->state) {
+	case STATE_INITIAL:
+	case STATE_EXITED:
+		/* Dynamic subtest starts when we're not even running? (Before exec or after exit) */
+		fprintf(stderr, "Error: Unexpected dynamic subtest start (binary wasn't running)\n");
+		context->state = STATE_ERROR;
+		return;
+	case STATE_AFTER_EXEC:
+		/* Binary was running but a subtest wasn't. We don't know where to inject an error message. */
+		fprintf(stderr, "Error: Unexpected dynamic subtest start (subtest wasn't running)\n");
+		context->state = STATE_ERROR;
+		return;
+	case STATE_BETWEEN_SUBTESTS:
+		/*
+		 * Dynamic subtest starts when a subtest is not
+		 * running. We can't know which subtest this dynamic
+		 * subtest was supposed to be in. But we can inject a
+		 * warn into the previous subtest.
+		 */
+		snprintf(errmsg, sizeof(errmsg),
+			 "\nrunner: Dynamic subtest %s started when not inside a subtest. This is a test bug.\n",
+			 helper.dynamicsubteststart.name);
+		append_line(&context->errbuf, &context->errbuflen, errmsg);
+
+		/* Leave the state as is and hope for the best */
+		return;
+	case STATE_DYNAMIC_SUBTEST_STARTED:
+		snprintf(errmsg, sizeof(errmsg),
+			 "\nrunner: Dynamic subtest %s already running when dynamic subtest %s starts. This is a test bug.\n",
+			 context->current_dynamic_subtest_name,
+			 helper.dynamicsubteststart.name);
+		append_line(&context->errbuf, &context->errbuflen, errmsg);
+
+		/* fallthrough */
+	case STATE_BETWEEN_DYNAMIC_SUBTESTS:
+		comms_finish_dynamic_subtest(context);
+		/* fallthrough */
+	case STATE_SUBTEST_STARTED:
+		comms_add_new_dynamic_subtest(context, helper.dynamicsubteststart.name);
+
+		/* Dynamic subtest starting message is not in logs with socket comms, inject it manually */
+		comms_inject_subtest_start_log(context, STARTING_DYNAMIC_SUBTEST, helper.dynamicsubteststart.name);
+
+		break;
+	case STATE_ERROR:
+		return;
+	default:
+		assert(false); /* unreachable */
+	}
+
+	context->state = STATE_DYNAMIC_SUBTEST_STARTED;
+}
+
+static void comms_handle_dynamic_subtest_result(struct comms_context *context,
+						union runnerpacket_read_helper helper)
+{
+	char errmsg[512];
+
+	switch (context->state) {
+	case STATE_INITIAL:
+	case STATE_EXITED:
+		/* Dynamic subtest result when we're not even running? (Before exec or after exit) */
+		fprintf(stderr, "Error: Unexpected dynamic subtest result (binary wasn't running)\n");
+		context->state = STATE_ERROR;
+		return;
+	case STATE_AFTER_EXEC:
+		/* Binary was running but a subtest wasn't. We don't know where to inject an error message. */
+		fprintf(stderr, "Error: Unexpected dynamic subtest result (subtest wasn't running)\n");
+		context->state = STATE_ERROR;
+		return;
+	case STATE_BETWEEN_SUBTESTS:
+		/*
+		 * Dynamic subtest result when a subtest is not
+		 * running. We can't know which subtest this dynamic
+		 * subtest was supposed to be in. But we can inject a
+		 * warn into the previous subtest.
+		 */
+		snprintf(errmsg, sizeof(errmsg),
+			 "\nrunner: Dynamic subtest %s result when not inside a subtest. This is a test bug.\n",
+			 helper.dynamicsubtestresult.name);
+		append_line(&context->errbuf, &context->errbuflen, errmsg);
+
+		/* Leave the state as is and hope for the best */
+		return;
+	case STATE_BETWEEN_DYNAMIC_SUBTESTS:
+		/*
+		 * Result without starting. There's no
+		 * skip_subtests_henceforth equivalent for dynamic
+		 * subtests so this shouldn't happen, but we can
+		 * handle it nevertheless.
+		 */
+		comms_finish_dynamic_subtest(context);
+		/* fallthrough */
+	case STATE_SUBTEST_STARTED:
+		/* Result without starting, but we aren't collecting for a dynamic subtest yet */
+		comms_add_new_dynamic_subtest(context, helper.dynamicsubtestresult.name);
+		break;
+	case STATE_DYNAMIC_SUBTEST_STARTED:
+		/* Normal flow */
+		break;
+	case STATE_ERROR:
+		return;
+	default:
+		assert(false); /* unreachable */
+	}
+
+	comms_inject_subtest_end_log(context,
+				     DYNAMIC_SUBTEST_RESULT,
+				     helper.dynamicsubtestresult.name,
+				     helper.dynamicsubtestresult.result,
+				     helper.dynamicsubtestresult.timeused);
+
+	/* Next dynamic subtest, if any, will begin its logs right after that result line */
+	context->nextdynoutidx = context->outbuflen;
+	context->nextdynerridx = context->errbuflen;
+
+	/*
+	 * Only store the actual result from the packet if we don't
+	 * already have one. If we do, it's from an override.
+	 */
+	if (context->dynamicsubtestresult == NULL) {
+		const char *mappedresult;
+
+		parse_result_string(helper.dynamicsubtestresult.result,
+				    strlen(helper.dynamicsubtestresult.result),
+				    &mappedresult, NULL);
+		context->dynamicsubtestresult = strdup(mappedresult);
+	}
+
+	context->state = STATE_BETWEEN_DYNAMIC_SUBTESTS;
+}
+
+static void comms_handle_versionstring(struct comms_context *context,
+				       union runnerpacket_read_helper helper)
+{
+	if (context->state == STATE_ERROR)
+		return;
+
+	free(context->igt_version);
+	context->igt_version = strdup(helper.versionstring.text);
+}
+
+static void comms_handle_result_override(struct comms_context *context,
+					 union runnerpacket_read_helper helper)
+{
+	if (context->state == STATE_ERROR)
+		return;
+
+	if (context->current_dynamic_subtest) {
+		free(context->dynamicsubtestresult);
+		context->dynamicsubtestresult = strdup(helper.resultoverride.result);
+	}
+
+	free(context->subtestresult);
+	context->subtestresult = strdup(helper.resultoverride.result);
 }
 
 static const int COMMSPARSE_SUCCESS = 0;
 static const int COMMSPARSE_ERROR = -1;
 static const int COMMSPARSE_EMPTY = 1;
 
-static int fill_from_comms(int fd, const char *binary,
+static int fill_from_comms(int fd,
+			   struct job_list_entry *entry,
 			   struct subtest_list *subtests,
 			   struct results *results)
 {
-	struct comms_context context;
-	struct json_object *obj;
+	struct comms_context context = {};
+	char piglit_name[256];
 	struct stat statbuf;
 	char *buf, *bufend, *p;
 	int ret = COMMSPARSE_EMPTY;
-	char piglit_name[256];
-	bool subtest_has_started = false;
 
 	if (fd < 0)
 		return COMMSPARSE_EMPTY;
@@ -1362,10 +1806,12 @@ static int fill_from_comms(int fd, const char *binary,
 	bufend = buf + statbuf.st_size;
 	p = buf;
 
-	comms_init_context(&context);
-
-	generate_piglit_name(binary, NULL, piglit_name, sizeof(piglit_name));
-	obj = get_or_create_json_object(results->runtimes, piglit_name);
+	context.entry = entry;
+	context.binary = entry->binary;
+	generate_piglit_name(entry->binary, NULL, piglit_name, sizeof(piglit_name));
+	context.binaryruntimeobj = get_or_create_json_object(results->runtimes, piglit_name);
+	context.results = results;
+	context.subtests = subtests;
 
 	while (p != NULL && p != bufend) {
 		const struct runnerpacket *packet;
@@ -1413,129 +1859,49 @@ static int fill_from_comms(int fd, const char *binary,
 
 		switch (helper.type) {
 		case PACKETTYPE_INVALID:
-			fprintf(stderr, "Error parsing runnerpacket (type=%"PRIu32")\n", packet->type);
-			munmap(buf, statbuf.st_size);
-			return COMMSPARSE_ERROR;
-
+			printf("Warning: Unknown packet type %"PRIu32", skipping\n", packet->type);
+			break;
 		case PACKETTYPE_LOG:
-			comms_append_log(&context, helper);
+			comms_handle_log(&context, helper);
 			break;
-
 		case PACKETTYPE_EXEC:
-			if (context.current_test != NULL)
-				comms_finish_subtest(&context);
-
-			context.cmdline = strdup(helper.exec.cmdline);
+			comms_handle_exec(&context, helper);
 			break;
-
 		case PACKETTYPE_EXIT:
-			context.exitcode = helper.exit.exitcode;
-			add_runtime(obj, strtod(helper.exit.timeused, NULL));
+			comms_handle_exit(&context, helper);
 			break;
-
 		case PACKETTYPE_SUBTEST_START:
-			if (context.current_test != NULL) {
-				/* Already collecting for a subtest, finish it up */
-				comms_finish_subtest(&context);
-			}
-
-			add_subtest(subtests, strdup(helper.subteststart.name));
-			generate_piglit_name(binary, helper.subteststart.name, piglit_name, sizeof(piglit_name));
-			context.current_test = get_or_create_json_object(results->tests, piglit_name);
-			free(context.current_subtest_name);
-			context.current_subtest_name = strdup(helper.subteststart.name);
-
-			/* Subtest starting message is not in logs with socket comms, inject it manually here */
-			comms_inject_subtest_start_log(&context, STARTING_SUBTEST, helper.subteststart.name);
-
-			subtest_has_started = true;
-
+			comms_handle_subtest_start(&context, helper);
 			break;
-
 		case PACKETTYPE_SUBTEST_RESULT:
-			if (!subtest_has_started) {
-				/* Result without start, a skip/fail from fixture */
-				if (context.current_test != NULL &&
-				    context.current_subtest_name != NULL &&
-				    strcmp(helper.subtestresult.name, context.current_subtest_name) != 0)
-					comms_finish_subtest(&context);
-
-				add_subtest(subtests, strdup(helper.subtestresult.name));
-				generate_piglit_name(binary, helper.subtestresult.name, piglit_name, sizeof(piglit_name));
-				context.current_test = get_or_create_json_object(results->tests, piglit_name);
-
-				free(context.current_subtest_name);
-				context.current_subtest_name = strdup(helper.subtestresult.name);
-
-			}
-
-			comms_inject_subtest_end_log(&context,
-						     SUBTEST_RESULT,
-						     helper.subtestresult.name,
-						     helper.subtestresult.result,
-						     helper.subtestresult.timeused);
-			comms_inject_subtest_end_log(&context,
-						     SUBTEST_RESULT,
-						     helper.subtestresult.name,
-						     helper.subtestresult.result,
-						     helper.subtestresult.timeused);
-
-			/*
-			 * Only store the actual result from the
-			 * packet if we don't already have one. If we
-			 * do, it's from an override.
-			 */
-			if (context.subtestresult == NULL) {
-				const char *mappedresult;
-
-				parse_result_string(helper.subtestresult.result,
-						    strlen(helper.subtestresult.result),
-						    &mappedresult, NULL);
-				context.subtestresult = strdup(mappedresult);
-			}
-
-			subtest_has_started = false;
-
-			/* Drop the marker here for the log lines shared between this test and the next */
-			context.commonoutbufmarker = context.outbuflen;
-			context.commonerrbufmarker = context.errbuflen;
+			comms_handle_subtest_result(&context, helper);
 			break;
-
 		case PACKETTYPE_DYNAMIC_SUBTEST_START:
+			comms_handle_dynamic_subtest_start(&context, helper);
 			break;
-
 		case PACKETTYPE_DYNAMIC_SUBTEST_RESULT:
+			comms_handle_dynamic_subtest_result(&context, helper);
 			break;
-
 		case PACKETTYPE_VERSIONSTRING:
-			free(context.igt_version);
-			context.igt_version = strdup(helper.versionstring.text);
+			comms_handle_versionstring(&context, helper);
 			break;
-
 		case PACKETTYPE_RESULT_OVERRIDE:
-			{
-				const char *mappedresult;
-
-				parse_result_string(helper.resultoverride.result,
-						    strlen(helper.resultoverride.result),
-						    &mappedresult, NULL);
-				free(context.subtestresult);
-				context.subtestresult = strdup(mappedresult);
-			}
-
+			comms_handle_result_override(&context, helper);
 			break;
-
 		default:
 			printf("Warning: Unknown packet type %"PRIu32"\n", helper.type);
 			break;
 		}
 	}
 
-	comms_finish_subtest(&context);
+	if (context.current_dynamic_subtest != NULL)
+		comms_finish_dynamic_subtest(&context);
+	if (context.current_test != NULL)
+		comms_finish_subtest(&context);
 	comms_free_context(&context);
 	munmap(buf, statbuf.st_size);
 
-	return ret;
+	return context.state == STATE_ERROR ? COMMSPARSE_ERROR : ret;
 }
 
 static void prune_subtests_with_dynamic_subtests(const char *binary,
@@ -1787,7 +2153,7 @@ static bool parse_test_directory(int dirfd,
 
 	/*
 	 * Get test output from socket comms if it exists, otherwise parse stdout/stderr */
-	commsparsed = fill_from_comms(fds[_F_SOCKET], entry->binary, &subtests, results);
+	commsparsed = fill_from_comms(fds[_F_SOCKET], entry, &subtests, results);
 	if (commsparsed == COMMSPARSE_ERROR) {
 		fprintf(stderr, "Error parsing output files (comms.txt)\n");
 		status = false;
-- 
2.29.2



More information about the Intel-gfx-trybot mailing list