[igt-dev] [PATCH i-g-t 1/2] runner/resultgen: Be more robust with incomplete tests

[Arkadiusz Hiler]

On Thu, Aug 16, 2018 at 02:14:15PM +0300, Petri Latvala wrote:
> If a test is incomplete and didn't have time to print that it's
> entering a subtest, the generated results will think the test binary
> does not have subtests. If that case is known, make sure to attribute
> blame correctly.
> 
> Signed-off-by: Petri Latvala <petri.latvala at intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
> ---
>  runner/resultgen.c | 41 +++++++++++++++++++++++++++++------------
>  1 file changed, 29 insertions(+), 12 deletions(-)
> 
> diff --git a/runner/resultgen.c b/runner/resultgen.c
> index 347b52a4..81e4fbda 100644
> --- a/runner/resultgen.c
> +++ b/runner/resultgen.c
> @@ -692,6 +692,8 @@ static const char *result_from_exitcode(int exitcode)
>  		return "pass";
>  	case IGT_EXIT_INVALID:
>  		return "notrun";
> +	case -1:
> +		return "incomplete";

please #define that value

bonus points for static asserts making sure that we do not clash with
IGT exit codes :-P

>  	default:
>  		return "fail";
>  	}
> @@ -712,7 +714,8 @@ static void add_subtest(struct subtests *subtests, char *subtest)
>  	subtests->names[subtests->size - 1] = subtest;
>  }
>  
> -static void fill_from_journal(int fd, char *binary,
> +static void fill_from_journal(int fd,
> +			      struct job_list_entry *entry,
>  			      struct subtests *subtests,
>  			      struct json_object *tests)
>  {
> @@ -732,7 +735,7 @@ static void fill_from_journal(int fd, char *binary,
>  			double time = 0.0;
>  			struct json_object *obj;
>  
> -			generate_piglit_name(binary, NULL, piglit_name, sizeof(piglit_name));
> +			generate_piglit_name(entry->binary, NULL, piglit_name, sizeof(piglit_name));
>  			obj = get_or_create_json_object(tests, piglit_name);
>  
>  			exitcode = atoi(line + strlen(exitline));
> @@ -752,7 +755,7 @@ static void fill_from_journal(int fd, char *binary,
>  				double time = 0.0;
>  				struct json_object *obj;
>  
> -				generate_piglit_name(binary, last_subtest, piglit_name, sizeof(piglit_name));
> +				generate_piglit_name(entry->binary, last_subtest, piglit_name, sizeof(piglit_name));
>  				obj = get_or_create_json_object(tests, piglit_name);
>  
>  				set_result(obj, "timeout");
> @@ -764,7 +767,7 @@ static void fill_from_journal(int fd, char *binary,
>  				add_runtime(obj, time);
>  
>  				/* ... and also for the binary */
> -				generate_piglit_name(binary, NULL, piglit_name, sizeof(piglit_name));
> +				generate_piglit_name(entry->binary, NULL, piglit_name, sizeof(piglit_name));
>  				obj = get_or_create_json_object(tests, piglit_name);
>  				add_runtime(obj, time);
>  			}
> @@ -774,11 +777,23 @@ static void fill_from_journal(int fd, char *binary,
>  	}
>  
>  	if (subtests->size == 0) {
> +		char *subtestname = NULL;
>  		char piglit_name[256];
>  		struct json_object *obj;
>  		const char *result = has_timeout ? "timeout" : result_from_exitcode(exitcode);
>  
> -		generate_piglit_name(binary, NULL, piglit_name, sizeof(piglit_name));
> +		/*
> +		 * If the test was killed before it printed that it's
> +		 * entering a subtest, we would incorrectly generate
> +		 * results as the binary had no subtests. If we know
> +		 * otherwise, do otherwise.
> +		 */
> +		if (entry->subtest_count == 1) {

Don't forget about multiple subtests mode, we can have more than one
subtest per entry. Changing the check to `subtest_count > 0` should do it.

With those changes:
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler at intel.com>