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

[Petri Latvala]

On Thu, Aug 16, 2018 at 02:42:10PM +0300, Arkadiusz Hiler wrote:
> 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>


Thanks, pushed with changes.


-- 
Petri Latvala