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

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

[Petri Latvala]

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@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@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";
 	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) {
+			subtestname = entry->subtests[0];
+			add_subtest(subtests, strdup(subtestname));
+		}
+
+		generate_piglit_name(entry->binary, subtestname, piglit_name, sizeof(piglit_name));
 		obj = get_or_create_json_object(tests, piglit_name);
 		set_result(obj, result);
 	}
@@ -835,7 +850,9 @@ static void override_results(char *binary,
 	}
 }
 
-static bool parse_test_directory(int dirfd, char *binary, struct json_object *tests)
+static bool parse_test_directory(int dirfd,
+				 struct job_list_entry *entry,
+				 struct json_object *tests)
 {
 	int fds[_F_LAST];
 	struct subtests subtests = {};
@@ -849,16 +866,16 @@ static bool parse_test_directory(int dirfd, char *binary, struct json_object *te
 	 * fill_from_journal fills the subtests struct and adds
 	 * timeout results where applicable.
 	 */
-	fill_from_journal(fds[_F_JOURNAL], binary, &subtests, tests);
+	fill_from_journal(fds[_F_JOURNAL], entry, &subtests, tests);
 
-	if (!fill_from_output(fds[_F_OUT], binary, "out", &subtests, tests) ||
-	    !fill_from_output(fds[_F_ERR], binary, "err", &subtests, tests) ||
-	    !fill_from_dmesg(fds[_F_DMESG], binary, &subtests, tests)) {
+	if (!fill_from_output(fds[_F_OUT], entry->binary, "out", &subtests, tests) ||
+	    !fill_from_output(fds[_F_ERR], entry->binary, "err", &subtests, tests) ||
+	    !fill_from_dmesg(fds[_F_DMESG], entry->binary, &subtests, tests)) {
 		fprintf(stderr, "Error parsing output files\n");
 		return false;
 	}
 
-	override_results(binary, &subtests, tests);
+	override_results(entry->binary, &subtests, tests);
 
 	close_outputs(fds);
 
@@ -940,7 +957,7 @@ bool generate_results(int dirfd)
 			break;
 		}
 
-		if (!parse_test_directory(testdirfd, job_list.entries[i].binary, tests)) {
+		if (!parse_test_directory(testdirfd, &job_list.entries[i], tests)) {
 			close(resultsfd);
 			return false;
 		}
-- 
2.14.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t 2/2] runner/resultgen: Implement the totals field

[Petri Latvala]

The totals field in the results json lists the total amount of
particular test results, both overall and by binary.

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
 runner/resultgen.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 86 insertions(+), 4 deletions(-)

diff --git a/runner/resultgen.c b/runner/resultgen.c
index 81e4fbda..7ac2c8d3 100644
--- a/runner/resultgen.c
+++ b/runner/resultgen.c
@@ -850,9 +850,89 @@ static void override_results(char *binary,
 	}
 }
 
+static struct json_object *get_totals_object(struct json_object *totals,
+					     const char *key)
+{
+	struct json_object *obj = NULL;
+
+	if (json_object_object_get_ex(totals, key, &obj))
+		return obj;
+
+	obj = json_object_new_object();
+	json_object_object_add(totals, key, obj);
+
+	json_object_object_add(obj, "crash", json_object_new_int(0));
+	json_object_object_add(obj, "pass", json_object_new_int(0));
+	json_object_object_add(obj, "dmesg-fail", json_object_new_int(0));
+	json_object_object_add(obj, "dmesg-warn", json_object_new_int(0));
+	json_object_object_add(obj, "skip", json_object_new_int(0));
+	json_object_object_add(obj, "incomplete", json_object_new_int(0));
+	json_object_object_add(obj, "timeout", json_object_new_int(0));
+	json_object_object_add(obj, "notrun", json_object_new_int(0));
+	json_object_object_add(obj, "fail", json_object_new_int(0));
+	json_object_object_add(obj, "warn", json_object_new_int(0));
+
+	return obj;
+}
+
+static void add_result_to_totals(struct json_object *totals,
+				 const char *result)
+{
+	json_object *numobj = NULL;
+	int old;
+
+	if (!json_object_object_get_ex(totals, result, &numobj)) {
+		fprintf(stderr, "Warning: Totals object without count for %s\n", result);
+		return;
+	}
+
+	old = json_object_get_int(numobj);
+	json_object_object_add(totals, result, json_object_new_int(old + 1));
+}
+
+static void add_to_totals(char *binary,
+			  struct subtests *subtests,
+			  struct json_object *tests,
+			  struct json_object *totals)
+{
+	struct json_object *test, *resultobj, *roottotal, *binarytotal;
+	char piglit_name[256];
+	const char *result;
+	size_t i;
+
+	generate_piglit_name(binary, NULL, piglit_name, sizeof(piglit_name));
+	roottotal = get_totals_object(totals, "");
+	binarytotal = get_totals_object(totals, piglit_name);
+
+	if (subtests->size == 0) {
+		test = get_or_create_json_object(tests, piglit_name);
+		if (!json_object_object_get_ex(test, "result", &resultobj)) {
+			fprintf(stderr, "Warning: No results set for %s\n", piglit_name);
+			return;
+		}
+		result = json_object_get_string(resultobj);
+		add_result_to_totals(roottotal, result);
+		add_result_to_totals(binarytotal, result);
+		return;
+	}
+
+	for (i = 0; i < subtests->size; i++) {
+		generate_piglit_name(binary, subtests->names[i], piglit_name, sizeof(piglit_name));
+		test = get_or_create_json_object(tests, piglit_name);
+		if (!json_object_object_get_ex(test, "result", &resultobj)) {
+			fprintf(stderr, "Warning: No results set for %s\n", piglit_name);
+			return;
+		}
+		result = json_object_get_string(resultobj);
+		add_result_to_totals(roottotal, result);
+		add_result_to_totals(binarytotal, result);
+	}
+}
+
 static bool parse_test_directory(int dirfd,
 				 struct job_list_entry *entry,
-				 struct json_object *tests)
+				 struct json_object *tests,
+				 struct json_object *totals)
 {
 	int fds[_F_LAST];
 	struct subtests subtests = {};
@@ -876,6 +956,7 @@ static bool parse_test_directory(int dirfd,
 	}
 
 	override_results(entry->binary, &subtests, tests);
+	add_to_totals(entry->binary, &subtests, tests, totals);
 
 	close_outputs(fds);
 
@@ -886,7 +967,7 @@ bool generate_results(int dirfd)
 {
 	struct settings settings;
 	struct job_list job_list;
-	struct json_object *obj, *tests;
+	struct json_object *obj, *tests, *totals;
 	int resultsfd, testdirfd, unamefd;
 	const char *json_string;
 	size_t i;
@@ -942,11 +1023,12 @@ bool generate_results(int dirfd)
 	 * - lspci
 	 * - options
 	 * - time_elapsed
-	 * - totals
 	 */
 
 	tests = json_object_new_object();
 	json_object_object_add(obj, "tests", tests);
+	totals = json_object_new_object();
+	json_object_object_add(obj, "totals", totals);
 
 	for (i = 0; i < job_list.size; i++) {
 		char name[16];
@@ -957,7 +1039,7 @@ bool generate_results(int dirfd)
 			break;
 		}
 
-		if (!parse_test_directory(testdirfd, &job_list.entries[i], tests)) {
+		if (!parse_test_directory(testdirfd, &job_list.entries[i], tests, totals)) {
 			close(resultsfd);
 			return false;
 		}
-- 
2.14.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [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@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@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@intel.com>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/2] runner/resultgen: Be more robust with incomplete tests

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/2] runner/resultgen: Be more robust with incomplete tests
URL   : https://patchwork.freedesktop.org/series/48311/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4677 -> IGTPW_1721 =

== Summary - WARNING ==

  Minor unknown changes coming with IGTPW_1721 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1721, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/48311/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1721:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_gttfill@basic:
      fi-pnv-d510:        SKIP -> PASS

    
== Known issues ==

  Here are the changes found in IGTPW_1721 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    {igt@amdgpu/amd_basic@userptr}:
      {fi-kbl-8809g}:     PASS -> INCOMPLETE (fdo#107402)

    igt@drv_module_reload@basic-reload-inject:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106725, fdo#106248) +1

    igt@gem_exec_suspend@basic-s3:
      fi-skl-guc:         PASS -> INCOMPLETE (fdo#106693, fdo#104108)

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    igt@kms_frontbuffer_tracking@basic:
      {fi-byt-clapper}:   FAIL (fdo#103167) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    
    ==== Warnings ====

    {igt@kms_psr@primary_page_flip}:
      fi-cnl-psr:         DMESG-WARN (fdo#107372) -> DMESG-FAIL (fdo#107372)

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#106248 https://bugs.freedesktop.org/show_bug.cgi?id=106248
  fdo#106693 https://bugs.freedesktop.org/show_bug.cgi?id=106693
  fdo#106725 https://bugs.freedesktop.org/show_bug.cgi?id=106725
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372
  fdo#107402 https://bugs.freedesktop.org/show_bug.cgi?id=107402


== Participating hosts (54 -> 49) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * IGT: IGT_4601 -> IGTPW_1721

  CI_DRM_4677: 1af9e170b6469a64c82f5a4961a2be2f0fc1ff0a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1721: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1721/
  IGT_4601: 0b5235db8d4c647a23cafe344c099d3699c8927e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1721/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/2] runner/resultgen: Implement the totals field

[Arkadiusz Hiler]

On Thu, Aug 16, 2018 at 02:14:16PM +0300, Petri Latvala wrote:
> The totals field in the results json lists the total amount of
> particular test results, both overall and by binary.
> 
> Signed-off-by: Petri Latvala <petri.latvala@intel.com>
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,1/2] runner/resultgen: Be more robust with incomplete tests

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/2] runner/resultgen: Be more robust with incomplete tests
URL   : https://patchwork.freedesktop.org/series/48311/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4601_full -> IGTPW_1721_full =

== Summary - WARNING ==

  Minor unknown changes coming with IGTPW_1721_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1721_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/48311/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1721_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_tiled_blits@interruptible:
      shard-glk:          SKIP -> PASS +4

    
== Known issues ==

  Here are the changes found in IGTPW_1721_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_suspend@shrink:
      shard-glk:          PASS -> FAIL (fdo#106886)

    igt@gem_exec_schedule@pi-ringfull-bsd1:
      shard-kbl:          NOTRUN -> FAIL (fdo#103158) +2

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#105363)

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#105363, fdo#102887)

    igt@prime_vgem@fence-wait-blt:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411)

    igt@testdisplay:
      shard-glk:          PASS -> INCOMPLETE (fdo#103359, fdo#107093, k.org#198133)

    
    ==== Possible fixes ====

    igt@drv_suspend@shrink:
      shard-kbl:          INCOMPLETE (fdo#103665, fdo#106886) -> PASS

    igt@gem_partial_pwrite_pread@write:
      shard-snb:          INCOMPLETE (fdo#105411) -> PASS

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-kbl:          INCOMPLETE (fdo#103665, fdo#106023) -> PASS

    igt@gem_render_linear_blits@basic:
      shard-kbl:          INCOMPLETE (fdo#103665) -> PASS

    igt@kms_available_modes_crc@available_mode_test_crc:
      shard-snb:          FAIL (fdo#106641) -> PASS

    igt@kms_rotation_crc@primary-rotation-180:
      shard-snb:          FAIL (fdo#103925) -> PASS

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS

    
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103158 https://bugs.freedesktop.org/show_bug.cgi?id=103158
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#107093 https://bugs.freedesktop.org/show_bug.cgi?id=107093
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * IGT: IGT_4601 -> IGTPW_1721
    * Linux: CI_DRM_4676 -> CI_DRM_4677

  CI_DRM_4676: 8171ee8227a2633ffb5808841f08cc1a3bfaffbb @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4677: 1af9e170b6469a64c82f5a4961a2be2f0fc1ff0a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1721: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1721/
  IGT_4601: 0b5235db8d4c647a23cafe344c099d3699c8927e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1721/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [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@intel.com>
> > Cc: Arkadiusz Hiler <arkadiusz.hiler@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@intel.com>


Thanks, pushed with changes.


-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev