All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t] runner: Treat dmesg warnings as pure warnings
@ 2018-11-13 17:25 Chris Wilson
  2018-11-13 18:40 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Chris Wilson @ 2018-11-13 17:25 UTC (permalink / raw)
  To: igt-dev; +Cc: Martin Peres, Petri Latvala

I have whinged on for ages about the dmesg-warnings being an expected
part of kernel testing (where else is the kernel meant to log its
errors?) and should be treated the same as our stderr for the test. That
is if a test fails, it fails and does not need to be conflated with
whether or not there was a dmesg warning (just as the test saying why it
failed on stderr does not need flagging), and that a passing test with a
dmesg warning is simply a warn.

The effect is that we simply remove the "dmesg-" flagging from results
names, as the err/dmesg output is simply collated for the error report
already.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Martin Peres <martin.peres@linux.intel.com>
---
 runner/resultgen.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/runner/resultgen.c b/runner/resultgen.c
index a62e400e..394c904f 100644
--- a/runner/resultgen.c
+++ b/runner/resultgen.c
@@ -466,8 +466,8 @@ static bool fill_from_output(int fd, const char *binary, const char *key,
 
 /*
  * This regexp controls the kmsg handling. All kernel log records that
- * have log level of warning or higher convert the result to
- * dmesg-warn/dmesg-fail unless they match this regexp.
+ * have log level of warning or higher convert a passing result to
+ * warn unless they match this regexp.
  *
  * TODO: Move this to external files, i915-suppressions.txt,
  * general-suppressions.txt et al.
@@ -816,14 +816,11 @@ static void override_result_single(struct json_object *obj)
 {
 	const char *errtext = NULL, *result = NULL;
 	struct json_object *textobj;
-	bool dmesgwarns = false;
 
 	if (json_object_object_get_ex(obj, "err", &textobj))
 		errtext = json_object_get_string(textobj);
 	if (json_object_object_get_ex(obj, "result", &textobj))
 		result = json_object_get_string(textobj);
-	if (json_object_object_get_ex(obj, "dmesg-warnings", &textobj))
-		dmesgwarns = true;
 
 	if (!strcmp(result, "pass") &&
 	    count_lines(errtext, errtext + strlen(errtext)) > 2) {
@@ -831,13 +828,9 @@ static void override_result_single(struct json_object *obj)
 		result = "warn";
 	}
 
-	if (dmesgwarns) {
-		if (!strcmp(result, "pass") || !strcmp(result, "warn")) {
-			set_result(obj, "dmesg-warn");
-		} else if (!strcmp(result, "fail")) {
-			set_result(obj, "dmesg-fail");
-		}
-	}
+	if (json_object_object_get_ex(obj, "dmesg-warnings", &textobj) &&
+	    !strcmp(result, "pass"))
+		set_result(obj, "warn");
 }
 
 static void override_results(char *binary,
-- 
2.19.1

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [igt-dev] ✗ Fi.CI.BAT: failure for runner: Treat dmesg warnings as pure warnings
  2018-11-13 17:25 [igt-dev] [PATCH i-g-t] runner: Treat dmesg warnings as pure warnings Chris Wilson
@ 2018-11-13 18:40 ` Patchwork
  2018-11-14 11:56 ` [igt-dev] [PATCH i-g-t] " Petri Latvala
  2018-11-14 13:04 ` [igt-dev] ✗ Fi.CI.BAT: failure for runner: Treat dmesg warnings as pure warnings (rev2) Patchwork
  2 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2018-11-13 18:40 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev

== Series Details ==

Series: runner: Treat dmesg warnings as pure warnings
URL   : https://patchwork.freedesktop.org/series/52437/
State : failure

== Summary ==

IGT patchset build failed on latest successful build
cab148ca3ec904a94d0cd43476cf7e1f8663f906 tests/i915/query: more accurate available/fused topology printout

(runner_json_test:25234) igt_core-INFO:   #6 ../runner/runner_json_tests.c:40 compare_objects()
(runner_json_test:25234) igt_core-INFO:   #7 ../runner/runner_json_tests.c:114 compare()
(runner_json_test:25234) igt_core-INFO:   #8 ../runner/runner_json_tests.c:146 run_results_and_compare()
(runner_json_test:25234) igt_core-INFO:   #9 ../runner/runner_json_tests.c:167 __real_main159()
(runner_json_test:25234) igt_core-INFO:   #10 ../runner/runner_json_tests.c:159 main()
(runner_json_test:25234) igt_core-INFO:   #11 ../csu/libc-start.c:344 __libc_start_main()
(runner_json_test:25234) igt_core-INFO:   #12 [_start+0x2a]
****  END  ****
Warning: Cannot open result directory 1
(runner_json_test:25234) CRITICAL: Test assertion failure function compare, file ../runner/runner_json_tests.c:109:
(runner_json_test:25234) CRITICAL: Failed assertion: !strcmp(json_object_get_string(one), json_object_get_string(two))
Subtest dmesg-results failed.
**** DEBUG ****
(runner_json_test:25234) DEBUG: Root object
(runner_json_test:25234) DEBUG: Key __type__
(runner_json_test:25234) DEBUG: Key results_version
(runner_json_test:25234) DEBUG: Key name
(runner_json_test:25234) DEBUG: Key uname
(runner_json_test:25234) DEBUG: Key time_elapsed
(runner_json_test:25234) DEBUG: Key __type__
(runner_json_test:25234) DEBUG: Key start
(runner_json_test:25234) DEBUG: Key end
(runner_json_test:25234) DEBUG: Key tests
(runner_json_test:25234) DEBUG: Key igt@successtest@first-subtest
(runner_json_test:25234) DEBUG: Key out
(runner_json_test:25234) DEBUG: Key igt-version
(runner_json_test:25234) DEBUG: Key result
(runner_json_test:25234) CRITICAL: Test assertion failure function compare, file ../runner/runner_json_tests.c:109:
(runner_json_test:25234) CRITICAL: Failed assertion: !strcmp(json_object_get_string(one), json_object_get_string(two))
(runner_json_test:25234) igt_core-INFO: Stack trace:
(runner_json_test:25234) igt_core-INFO:   #0 ../lib/igt_core.c:1467 __igt_fail_assert()
(runner_json_test:25234) igt_core-INFO:   #1 ../runner/runner_json_tests.c:112 compare()
(runner_json_test:25234) igt_core-INFO:   #2 ../runner/runner_json_tests.c:40 compare_objects()
(runner_json_test:25234) igt_core-INFO:   #3 ../runner/runner_json_tests.c:114 compare()
(runner_json_test:25234) igt_core-INFO:   #4 ../runner/runner_json_tests.c:40 compare_objects()
(runner_json_test:25234) igt_core-INFO:   #5 ../runner/runner_json_tests.c:114 compare()
(runner_json_test:25234) igt_core-INFO:   #6 ../runner/runner_json_tests.c:40 compare_objects()
(runner_json_test:25234) igt_core-INFO:   #7 ../runner/runner_json_tests.c:114 compare()
(runner_json_test:25234) igt_core-INFO:   #8 ../runner/runner_json_tests.c:146 run_results_and_compare()
(runner_json_test:25234) igt_core-INFO:   #9 ../runner/runner_json_tests.c:167 __real_main159()
(runner_json_test:25234) igt_core-INFO:   #10 ../runner/runner_json_tests.c:159 main()
(runner_json_test:25234) igt_core-INFO:   #11 ../csu/libc-start.c:344 __libc_start_main()
(runner_json_test:25234) igt_core-INFO:   #12 [_start+0x2a]
****  END  ****
-------

Full log written to /home/cidrm/igt-gpu-tools/build/meson-logs/testlog.txt
FAILED: meson-test 
/usr/bin/python3 -u /usr/bin/meson test --no-rebuild --print-errorlogs
ninja: build stopped: subcommand failed.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] runner: Treat dmesg warnings as pure warnings
  2018-11-13 17:25 [igt-dev] [PATCH i-g-t] runner: Treat dmesg warnings as pure warnings Chris Wilson
  2018-11-13 18:40 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
@ 2018-11-14 11:56 ` Petri Latvala
  2018-11-14 13:25   ` Ville Syrjälä
  2018-11-14 13:04 ` [igt-dev] ✗ Fi.CI.BAT: failure for runner: Treat dmesg warnings as pure warnings (rev2) Patchwork
  2 siblings, 1 reply; 14+ messages in thread
From: Petri Latvala @ 2018-11-14 11:56 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev, Martin Peres

On Tue, Nov 13, 2018 at 05:25:47PM +0000, Chris Wilson wrote:
> I have whinged on for ages about the dmesg-warnings being an expected
> part of kernel testing (where else is the kernel meant to log its
> errors?) and should be treated the same as our stderr for the test. That
> is if a test fails, it fails and does not need to be conflated with
> whether or not there was a dmesg warning (just as the test saying why it
> failed on stderr does not need flagging), and that a passing test with a
> dmesg warning is simply a warn.
> 
> The effect is that we simply remove the "dmesg-" flagging from results
> names, as the err/dmesg output is simply collated for the error report
> already.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Martin Peres <martin.peres@linux.intel.com>


Yep, now's a good a time as any to remove this cargo cult if ever.

Some acks from kernel people in general would be nice.

TODO:
 Martin: cibuglog filters ready for this
 Tomi et al: Visualization ready for this (tooltips etc)

Runner's tests need to be adapted, so squash this in:


-- >8 --
Subject: [PATCH i-g-t 1/1] Adapt unit tests for removing dmesg-warn

Signed-off-by: Petri Latvala <petri.latvala@intel.com>
---
 .../json_tests_data/dmesg-results/README.txt  |   3 +-
 .../dmesg-results/reference.json              |  34 ++--
 .../warnings-with-dmesg-warns/0/dmesg.txt     |   6 -
 .../warnings-with-dmesg-warns/0/err.txt       |   3 -
 .../warnings-with-dmesg-warns/0/journal.txt   |   2 -
 .../warnings-with-dmesg-warns/0/out.txt       |   3 -
 .../warnings-with-dmesg-warns/1/dmesg.txt     |   5 -
 .../warnings-with-dmesg-warns/1/err.txt       |   2 -
 .../warnings-with-dmesg-warns/1/journal.txt   |   2 -
 .../warnings-with-dmesg-warns/1/out.txt       |   3 -
 .../warnings-with-dmesg-warns/2/dmesg.txt     |   4 -
 .../warnings-with-dmesg-warns/2/err.txt       |   0
 .../warnings-with-dmesg-warns/2/journal.txt   |   1 -
 .../warnings-with-dmesg-warns/2/out.txt       |   2 -
 .../warnings-with-dmesg-warns/3/dmesg.txt     |   4 -
 .../warnings-with-dmesg-warns/3/err.txt       |   1 -
 .../warnings-with-dmesg-warns/3/journal.txt   |   2 -
 .../warnings-with-dmesg-warns/3/out.txt       |   6 -
 .../warnings-with-dmesg-warns/4/dmesg.txt     |   4 -
 .../warnings-with-dmesg-warns/4/err.txt       |   1 -
 .../warnings-with-dmesg-warns/4/journal.txt   |   2 -
 .../warnings-with-dmesg-warns/4/out.txt       |   6 -
 .../warnings-with-dmesg-warns/README.txt      |   3 -
 .../warnings-with-dmesg-warns/endtime.txt     |   1 -
 .../warnings-with-dmesg-warns/joblist.txt     |   5 -
 .../warnings-with-dmesg-warns/metadata.txt    |  12 --
 .../warnings-with-dmesg-warns/reference.json  | 159 ------------------
 .../warnings-with-dmesg-warns/starttime.txt   |   1 -
 .../warnings-with-dmesg-warns/uname.txt       |   1 -
 runner/runner_json_tests.c                    |   1 -
 30 files changed, 18 insertions(+), 261 deletions(-)
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/0/dmesg.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/0/err.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/0/journal.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/0/out.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/1/dmesg.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/1/err.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/1/journal.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/1/out.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/2/dmesg.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/2/err.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/2/journal.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/2/out.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/3/dmesg.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/3/err.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/3/journal.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/3/out.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/4/dmesg.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/4/err.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/4/journal.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/4/out.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/README.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/endtime.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/joblist.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/metadata.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/reference.json
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/starttime.txt
 delete mode 100644 runner/json_tests_data/warnings-with-dmesg-warns/uname.txt

diff --git a/runner/json_tests_data/dmesg-results/README.txt b/runner/json_tests_data/dmesg-results/README.txt
index d928a526..e66ee611 100644
--- a/runner/json_tests_data/dmesg-results/README.txt
+++ b/runner/json_tests_data/dmesg-results/README.txt
@@ -1,3 +1,2 @@
 Certain types of kernel log messages cause the result to change from
-'pass' to 'dmesg-warn', and from 'fail' to 'dmesg-fail'. 'skip' should
-stay 'skip'.
+'pass' to 'warn'. 'skip' should stay 'skip'.
diff --git a/runner/json_tests_data/dmesg-results/reference.json b/runner/json_tests_data/dmesg-results/reference.json
index 811b63eb..bf96217e 100644
--- a/runner/json_tests_data/dmesg-results/reference.json
+++ b/runner/json_tests_data/dmesg-results/reference.json
@@ -12,7 +12,7 @@
     "igt@successtest@first-subtest":{
       "out":"Starting subtest: first-subtest\nSubtest first-subtest: SUCCESS (0.000s)\n",
       "igt-version":"IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)",
-      "result":"dmesg-warn",
+      "result":"warn",
       "time":{
         "__type__":"TimeAttribute",
         "start":0,
@@ -25,7 +25,7 @@
     "igt@successtest@second-subtest":{
       "out":"Starting subtest: second-subtest\nSubtest second-subtest: FAIL (0.000s)\n",
       "igt-version":"IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)",
-      "result":"dmesg-fail",
+      "result":"fail",
       "time":{
         "__type__":"TimeAttribute",
         "start":0,
@@ -41,7 +41,7 @@
         "start":0,
         "end":0.01
       },
-      "result":"dmesg-warn",
+      "result":"warn",
       "out":"IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)\nSUCCESS (0.000s)\n",
       "igt-version":"IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)",
       "err":"",
@@ -77,50 +77,50 @@
     "":{
       "crash":0,
       "pass":0,
-      "dmesg-fail":1,
-      "dmesg-warn":2,
+      "dmesg-fail":0,
+      "dmesg-warn":0,
       "skip":2,
       "incomplete":0,
       "timeout":0,
       "notrun":0,
-      "fail":0,
-      "warn":0
+      "fail":1,
+      "warn":2
     },
     "root":{
       "crash":0,
       "pass":0,
-      "dmesg-fail":1,
-      "dmesg-warn":2,
+      "dmesg-fail":0,
+      "dmesg-warn":0,
       "skip":2,
       "incomplete":0,
       "timeout":0,
       "notrun":0,
-      "fail":0,
-      "warn":0
+      "fail":1,
+      "warn":2
     },
     "igt@successtest":{
       "crash":0,
       "pass":0,
-      "dmesg-fail":1,
-      "dmesg-warn":1,
+      "dmesg-fail":0,
+      "dmesg-warn":0,
       "skip":0,
       "incomplete":0,
       "timeout":0,
       "notrun":0,
-      "fail":0,
-      "warn":0
+      "fail":1,
+      "warn":1
     },
     "igt@no-subtests":{
       "crash":0,
       "pass":0,
       "dmesg-fail":0,
-      "dmesg-warn":1,
+      "dmesg-warn":0,
       "skip":0,
       "incomplete":0,
       "timeout":0,
       "notrun":0,
       "fail":0,
-      "warn":0
+      "warn":1
     },
     "igt@skippers":{
       "crash":0,
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/0/dmesg.txt b/runner/json_tests_data/warnings-with-dmesg-warns/0/dmesg.txt
deleted file mode 100644
index e50f75db..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/0/dmesg.txt
+++ /dev/null
@@ -1,6 +0,0 @@
-6,951,3216186095083,-;Console: switching to colour dummy device 80x25
-14,952,3216186095097,-;[IGT] successtest: executing
-14,953,3216186101115,-;[IGT] successtest: starting subtest first-subtest
-3,954,3216186101159,-;Warning from kernel
-14,955,3216186101160,-;[IGT] successtest: exiting, ret=0
-6,956,3216186101299,-;Console: switching to colour frame buffer device 240x75
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/0/err.txt b/runner/json_tests_data/warnings-with-dmesg-warns/0/err.txt
deleted file mode 100644
index e18c00e9..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/0/err.txt
+++ /dev/null
@@ -1,3 +0,0 @@
-Starting subtest: first-subtest
-This is a warning line
-Subtest first-subtest: SUCCESS (0.000s)
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/0/journal.txt b/runner/json_tests_data/warnings-with-dmesg-warns/0/journal.txt
deleted file mode 100644
index 86a30e07..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/0/journal.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-first-subtest
-exit:0 (0.014s)
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/0/out.txt b/runner/json_tests_data/warnings-with-dmesg-warns/0/out.txt
deleted file mode 100644
index 5946bf31..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/0/out.txt
+++ /dev/null
@@ -1,3 +0,0 @@
-IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)
-Starting subtest: first-subtest
-Subtest first-subtest: SUCCESS (0.000s)
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/1/dmesg.txt b/runner/json_tests_data/warnings-with-dmesg-warns/1/dmesg.txt
deleted file mode 100644
index 59b7cee3..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/1/dmesg.txt
+++ /dev/null
@@ -1,5 +0,0 @@
-6,956,3216186111837,-;Console: switching to colour dummy device 80x25
-14,957,3216186111851,-;[IGT] successtest: executing
-14,958,3216186114762,-;[IGT] successtest: starting subtest second-subtest
-14,959,3216186114814,-;[IGT] successtest: exiting, ret=0
-6,960,3216186114933,-;Console: switching to colour frame buffer device 240x75
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/1/err.txt b/runner/json_tests_data/warnings-with-dmesg-warns/1/err.txt
deleted file mode 100644
index 57be9f7b..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/1/err.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-Starting subtest: second-subtest
-Subtest second-subtest: SUCCESS (0.000s)
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/1/journal.txt b/runner/json_tests_data/warnings-with-dmesg-warns/1/journal.txt
deleted file mode 100644
index 99f57815..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/1/journal.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-second-subtest
-exit:0 (0.013s)
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/1/out.txt b/runner/json_tests_data/warnings-with-dmesg-warns/1/out.txt
deleted file mode 100644
index 24b37244..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/1/out.txt
+++ /dev/null
@@ -1,3 +0,0 @@
-IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)
-Starting subtest: second-subtest
-Subtest second-subtest: SUCCESS (0.000s)
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/2/dmesg.txt b/runner/json_tests_data/warnings-with-dmesg-warns/2/dmesg.txt
deleted file mode 100644
index 998b4797..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/2/dmesg.txt
+++ /dev/null
@@ -1,4 +0,0 @@
-6,961,3216186123400,-;Console: switching to colour dummy device 80x25
-14,962,3216186123414,-;[IGT] no-subtests: executing
-14,963,3216186125204,-;[IGT] no-subtests: exiting, ret=0
-6,964,3216186125374,-;Console: switching to colour frame buffer device 240x75
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/2/err.txt b/runner/json_tests_data/warnings-with-dmesg-warns/2/err.txt
deleted file mode 100644
index e69de29b..00000000
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/2/journal.txt b/runner/json_tests_data/warnings-with-dmesg-warns/2/journal.txt
deleted file mode 100644
index 7151877f..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/2/journal.txt
+++ /dev/null
@@ -1 +0,0 @@
-exit:0 (0.010s)
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/2/out.txt b/runner/json_tests_data/warnings-with-dmesg-warns/2/out.txt
deleted file mode 100644
index 695b67c2..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/2/out.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)
-SUCCESS (0.000s)
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/3/dmesg.txt b/runner/json_tests_data/warnings-with-dmesg-warns/3/dmesg.txt
deleted file mode 100644
index 21e75031..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/3/dmesg.txt
+++ /dev/null
@@ -1,4 +0,0 @@
-6,965,3216186135188,-;Console: switching to colour dummy device 80x25
-14,966,3216186135212,-;[IGT] skippers: executing
-14,967,3216186137075,-;[IGT] skippers: exiting, ret=77
-6,968,3216186137206,-;Console: switching to colour frame buffer device 240x75
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/3/err.txt b/runner/json_tests_data/warnings-with-dmesg-warns/3/err.txt
deleted file mode 100644
index 59b73d09..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/3/err.txt
+++ /dev/null
@@ -1 +0,0 @@
-Subtest skip-one: SKIP
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/3/journal.txt b/runner/json_tests_data/warnings-with-dmesg-warns/3/journal.txt
deleted file mode 100644
index afab8ba6..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/3/journal.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-skip-one
-exit:77 (0.011s)
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/3/out.txt b/runner/json_tests_data/warnings-with-dmesg-warns/3/out.txt
deleted file mode 100644
index 96284e78..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/3/out.txt
+++ /dev/null
@@ -1,6 +0,0 @@
-IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)
-Test requirement not met in function __real_main3, file ../runner/testdata/skippers.c:6:
-Test requirement: false
-Skipping from fixture
-Last errno: 2, No such file or directory
-Subtest skip-one: SKIP
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/4/dmesg.txt b/runner/json_tests_data/warnings-with-dmesg-warns/4/dmesg.txt
deleted file mode 100644
index 737bc692..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/4/dmesg.txt
+++ /dev/null
@@ -1,4 +0,0 @@
-6,969,3216186145899,-;Console: switching to colour dummy device 80x25
-14,970,3216186145912,-;[IGT] skippers: executing
-14,971,3216186147754,-;[IGT] skippers: exiting, ret=77
-6,972,3216186147894,-;Console: switching to colour frame buffer device 240x75
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/4/err.txt b/runner/json_tests_data/warnings-with-dmesg-warns/4/err.txt
deleted file mode 100644
index 2251da1e..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/4/err.txt
+++ /dev/null
@@ -1 +0,0 @@
-Subtest skip-two: SKIP
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/4/journal.txt b/runner/json_tests_data/warnings-with-dmesg-warns/4/journal.txt
deleted file mode 100644
index a9dba132..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/4/journal.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-skip-two
-exit:77 (0.010s)
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/4/out.txt b/runner/json_tests_data/warnings-with-dmesg-warns/4/out.txt
deleted file mode 100644
index 2024db8f..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/4/out.txt
+++ /dev/null
@@ -1,6 +0,0 @@
-IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)
-Test requirement not met in function __real_main3, file ../runner/testdata/skippers.c:6:
-Test requirement: false
-Skipping from fixture
-Last errno: 2, No such file or directory
-Subtest skip-two: SKIP
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/README.txt b/runner/json_tests_data/warnings-with-dmesg-warns/README.txt
deleted file mode 100644
index 7ef9ee73..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/README.txt
+++ /dev/null
@@ -1,3 +0,0 @@
-A test that has output in stderr should result in 'warn' instead of
-'pass'. But if there is a message in the kernel logs that would make
-the result 'dmesg-warn' instead, that takes priority over 'warn'.
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/endtime.txt b/runner/json_tests_data/warnings-with-dmesg-warns/endtime.txt
deleted file mode 100644
index 635f6ae9..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/endtime.txt
+++ /dev/null
@@ -1 +0,0 @@
-1539953735.172373
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/joblist.txt b/runner/json_tests_data/warnings-with-dmesg-warns/joblist.txt
deleted file mode 100644
index 31ef8413..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/joblist.txt
+++ /dev/null
@@ -1,5 +0,0 @@
-successtest first-subtest
-successtest second-subtest
-no-subtests
-skippers skip-one
-skippers skip-two
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/metadata.txt b/runner/json_tests_data/warnings-with-dmesg-warns/metadata.txt
deleted file mode 100644
index 1316560d..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/metadata.txt
+++ /dev/null
@@ -1,12 +0,0 @@
-abort_on_error : 0
-name : normal-run
-dry_run : 0
-sync : 0
-log_level : 0
-overwrite : 0
-multiple_mode : 0
-inactivity_timeout : 0
-use_watchdog : 0
-piglit_style_dmesg : 0
-test_root : /path/does/not/exist
-results_path : /path/does/not/exist
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/reference.json b/runner/json_tests_data/warnings-with-dmesg-warns/reference.json
deleted file mode 100644
index 9e027efb..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/reference.json
+++ /dev/null
@@ -1,159 +0,0 @@
-{
-  "__type__":"TestrunResult",
-  "results_version":9,
-  "name":"normal-run",
-  "uname":"Linux hostname 4.18.0-1-amd64 #1 SMP Debian 4.18.6-1 (2018-09-06) x86_64",
-  "time_elapsed":{
-    "__type__":"TimeAttribute",
-    "start":1539953735.1110389,
-    "end":1539953735.1723731
-  },
-  "tests":{
-    "igt@successtest@first-subtest":{
-      "out":"Starting subtest: first-subtest\nSubtest first-subtest: SUCCESS (0.000s)\n",
-      "igt-version":"IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)",
-      "result":"dmesg-warn",
-      "time":{
-        "__type__":"TimeAttribute",
-        "start":0,
-        "end":0
-      },
-      "err":"Starting subtest: first-subtest\nThis is a warning line\nSubtest first-subtest: SUCCESS (0.000s)\n",
-	"dmesg":"<6> [3216186.095083] Console: switching to colour dummy device 80x25\n<6> [3216186.095097] [IGT] successtest: executing\n<6> [3216186.101115] [IGT] successtest: starting subtest first-subtest\n<3> [3216186.101159] Warning from kernel\n<6> [3216186.101160] [IGT] successtest: exiting, ret=0\n<6> [3216186.101299] Console: switching to colour frame buffer device 240x75\n",
-	"dmesg-warnings":"<3> [3216186.101159] Warning from kernel\n"
-    },
-    "igt@successtest@second-subtest":{
-      "out":"Starting subtest: second-subtest\nSubtest second-subtest: SUCCESS (0.000s)\n",
-      "igt-version":"IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)",
-      "result":"pass",
-      "time":{
-        "__type__":"TimeAttribute",
-        "start":0,
-        "end":0
-      },
-      "err":"Starting subtest: second-subtest\nSubtest second-subtest: SUCCESS (0.000s)\n",
-      "dmesg":"<6> [3216186.111837] Console: switching to colour dummy device 80x25\n<6> [3216186.111851] [IGT] successtest: executing\n<6> [3216186.114762] [IGT] successtest: starting subtest second-subtest\n<6> [3216186.114814] [IGT] successtest: exiting, ret=0\n<6> [3216186.114933] Console: switching to colour frame buffer device 240x75\n"
-    },
-    "igt@no-subtests":{
-      "time":{
-        "__type__":"TimeAttribute",
-        "start":0,
-        "end":0.01
-      },
-      "result":"pass",
-      "out":"IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)\nSUCCESS (0.000s)\n",
-      "igt-version":"IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)",
-      "err":"",
-      "dmesg":"<6> [3216186.123400] Console: switching to colour dummy device 80x25\n<6> [3216186.123414] [IGT] no-subtests: executing\n<6> [3216186.125204] [IGT] no-subtests: exiting, ret=0\n<6> [3216186.125374] Console: switching to colour frame buffer device 240x75\n"
-    },
-    "igt@skippers@skip-one":{
-      "out":"Test requirement not met in function __real_main3, file ..\/runner\/testdata\/skippers.c:6:\nTest requirement: false\nSkipping from fixture\nLast errno: 2, No such file or directory\nSubtest skip-one: SKIP\n",
-      "igt-version":"IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)",
-      "result":"skip",
-      "time":{
-        "__type__":"TimeAttribute",
-        "start":0,
-        "end":0
-      },
-      "err":"Subtest skip-one: SKIP\n",
-      "dmesg":"<6> [3216186.135188] Console: switching to colour dummy device 80x25\n<6> [3216186.135212] [IGT] skippers: executing\n<6> [3216186.137075] [IGT] skippers: exiting, ret=77\n<6> [3216186.137206] Console: switching to colour frame buffer device 240x75\n"
-    },
-    "igt@skippers@skip-two":{
-      "out":"Test requirement not met in function __real_main3, file ..\/runner\/testdata\/skippers.c:6:\nTest requirement: false\nSkipping from fixture\nLast errno: 2, No such file or directory\nSubtest skip-two: SKIP\n",
-      "igt-version":"IGT-Version: 1.23-g0c763bfd (x86_64) (Linux: 4.18.0-1-amd64 x86_64)",
-      "result":"skip",
-      "time":{
-        "__type__":"TimeAttribute",
-        "start":0,
-        "end":0
-      },
-      "err":"Subtest skip-two: SKIP\n",
-      "dmesg":"<6> [3216186.145899] Console: switching to colour dummy device 80x25\n<6> [3216186.145912] [IGT] skippers: executing\n<6> [3216186.147754] [IGT] skippers: exiting, ret=77\n<6> [3216186.147894] Console: switching to colour frame buffer device 240x75\n"
-    }
-  },
-  "totals":{
-    "":{
-      "crash":0,
-      "pass":2,
-      "dmesg-fail":0,
-      "dmesg-warn":1,
-      "skip":2,
-      "incomplete":0,
-      "timeout":0,
-      "notrun":0,
-      "fail":0,
-      "warn":0
-    },
-    "root":{
-      "crash":0,
-      "pass":2,
-      "dmesg-fail":0,
-      "dmesg-warn":1,
-      "skip":2,
-      "incomplete":0,
-      "timeout":0,
-      "notrun":0,
-      "fail":0,
-      "warn":0
-    },
-    "igt@successtest":{
-      "crash":0,
-      "pass":1,
-      "dmesg-fail":0,
-      "dmesg-warn":1,
-      "skip":0,
-      "incomplete":0,
-      "timeout":0,
-      "notrun":0,
-      "fail":0,
-      "warn":0
-    },
-    "igt@no-subtests":{
-      "crash":0,
-      "pass":1,
-      "dmesg-fail":0,
-      "dmesg-warn":0,
-      "skip":0,
-      "incomplete":0,
-      "timeout":0,
-      "notrun":0,
-      "fail":0,
-      "warn":0
-    },
-    "igt@skippers":{
-      "crash":0,
-      "pass":0,
-      "dmesg-fail":0,
-      "dmesg-warn":0,
-      "skip":2,
-      "incomplete":0,
-      "timeout":0,
-      "notrun":0,
-      "fail":0,
-      "warn":0
-    }
-  },
-  "runtimes":{
-    "igt@successtest":{
-      "time":{
-        "__type__":"TimeAttribute",
-        "start":0,
-        "end":0.027
-      }
-    },
-    "igt@no-subtests":{
-      "time":{
-        "__type__":"TimeAttribute",
-        "start":0,
-        "end":0.01
-      }
-    },
-    "igt@skippers":{
-      "time":{
-        "__type__":"TimeAttribute",
-        "start":0,
-        "end":0.020999999999999998
-      }
-    }
-  }
-}
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/starttime.txt b/runner/json_tests_data/warnings-with-dmesg-warns/starttime.txt
deleted file mode 100644
index ae038f18..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/starttime.txt
+++ /dev/null
@@ -1 +0,0 @@
-1539953735.111039
diff --git a/runner/json_tests_data/warnings-with-dmesg-warns/uname.txt b/runner/json_tests_data/warnings-with-dmesg-warns/uname.txt
deleted file mode 100644
index a7aef6f7..00000000
--- a/runner/json_tests_data/warnings-with-dmesg-warns/uname.txt
+++ /dev/null
@@ -1 +0,0 @@
-Linux hostname 4.18.0-1-amd64 #1 SMP Debian 4.18.6-1 (2018-09-06) x86_64
diff --git a/runner/runner_json_tests.c b/runner/runner_json_tests.c
index 758700d4..7f8469e4 100644
--- a/runner/runner_json_tests.c
+++ b/runner/runner_json_tests.c
@@ -150,7 +150,6 @@ static void run_results_and_compare(int dirfd, const char *dirname)
 static const char *dirnames[] = {
 	"normal-run",
 	"warnings",
-	"warnings-with-dmesg-warns",
 	"piglit-style-dmesg",
 	"incomplete-before-any-subtests",
 	"dmesg-results",
-- 
2.18.0
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [igt-dev] ✗ Fi.CI.BAT: failure for runner: Treat dmesg warnings as pure warnings (rev2)
  2018-11-13 17:25 [igt-dev] [PATCH i-g-t] runner: Treat dmesg warnings as pure warnings Chris Wilson
  2018-11-13 18:40 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
  2018-11-14 11:56 ` [igt-dev] [PATCH i-g-t] " Petri Latvala
@ 2018-11-14 13:04 ` Patchwork
  2 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2018-11-14 13:04 UTC (permalink / raw)
  To: Petri Latvala; +Cc: igt-dev

== Series Details ==

Series: runner: Treat dmesg warnings as pure warnings (rev2)
URL   : https://patchwork.freedesktop.org/series/52437/
State : failure

== Summary ==

IGT patchset build failed on latest successful build
cab148ca3ec904a94d0cd43476cf7e1f8663f906 tests/i915/query: more accurate available/fused topology printout

  #7 ../runner/runner_json_tests.c:114 compare()
  #8 ../runner/runner_json_tests.c:146 run_results_and_compare()
  #9 ../runner/runner_json_tests.c:166 __real_main158()
  #10 ../runner/runner_json_tests.c:158 main()
  #11 ../csu/libc-start.c:344 __libc_start_main()
  #12 [_start+0x2a]
Subtest dmesg-results: FAIL (0.004s)
--- stderr ---
Warning: Cannot open result directory 1
(runner_json_test:35925) CRITICAL: Test assertion failure function compare, file ../runner/runner_json_tests.c:109:
(runner_json_test:35925) CRITICAL: Failed assertion: !strcmp(json_object_get_string(one), json_object_get_string(two))
Subtest dmesg-results failed.
**** DEBUG ****
(runner_json_test:35925) DEBUG: Root object
(runner_json_test:35925) DEBUG: Key __type__
(runner_json_test:35925) DEBUG: Key results_version
(runner_json_test:35925) DEBUG: Key name
(runner_json_test:35925) DEBUG: Key uname
(runner_json_test:35925) DEBUG: Key time_elapsed
(runner_json_test:35925) DEBUG: Key __type__
(runner_json_test:35925) DEBUG: Key start
(runner_json_test:35925) DEBUG: Key end
(runner_json_test:35925) DEBUG: Key tests
(runner_json_test:35925) DEBUG: Key igt@successtest@first-subtest
(runner_json_test:35925) DEBUG: Key out
(runner_json_test:35925) DEBUG: Key igt-version
(runner_json_test:35925) DEBUG: Key result
(runner_json_test:35925) CRITICAL: Test assertion failure function compare, file ../runner/runner_json_tests.c:109:
(runner_json_test:35925) CRITICAL: Failed assertion: !strcmp(json_object_get_string(one), json_object_get_string(two))
(runner_json_test:35925) igt_core-INFO: Stack trace:
(runner_json_test:35925) igt_core-INFO:   #0 ../lib/igt_core.c:1467 __igt_fail_assert()
(runner_json_test:35925) igt_core-INFO:   #1 ../runner/runner_json_tests.c:112 compare()
(runner_json_test:35925) igt_core-INFO:   #2 ../runner/runner_json_tests.c:40 compare_objects()
(runner_json_test:35925) igt_core-INFO:   #3 ../runner/runner_json_tests.c:114 compare()
(runner_json_test:35925) igt_core-INFO:   #4 ../runner/runner_json_tests.c:40 compare_objects()
(runner_json_test:35925) igt_core-INFO:   #5 ../runner/runner_json_tests.c:114 compare()
(runner_json_test:35925) igt_core-INFO:   #6 ../runner/runner_json_tests.c:40 compare_objects()
(runner_json_test:35925) igt_core-INFO:   #7 ../runner/runner_json_tests.c:114 compare()
(runner_json_test:35925) igt_core-INFO:   #8 ../runner/runner_json_tests.c:146 run_results_and_compare()
(runner_json_test:35925) igt_core-INFO:   #9 ../runner/runner_json_tests.c:166 __real_main158()
(runner_json_test:35925) igt_core-INFO:   #10 ../runner/runner_json_tests.c:158 main()
(runner_json_test:35925) igt_core-INFO:   #11 ../csu/libc-start.c:344 __libc_start_main()
(runner_json_test:35925) igt_core-INFO:   #12 [_start+0x2a]
****  END  ****
-------

Full log written to /home/cidrm/igt-gpu-tools/build/meson-logs/testlog.txt
FAILED: meson-test 
/usr/bin/python3 -u /usr/bin/meson test --no-rebuild --print-errorlogs
ninja: build stopped: subcommand failed.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] runner: Treat dmesg warnings as pure warnings
  2018-11-14 11:56 ` [igt-dev] [PATCH i-g-t] " Petri Latvala
@ 2018-11-14 13:25   ` Ville Syrjälä
  2018-11-14 13:34     ` Chris Wilson
  0 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2018-11-14 13:25 UTC (permalink / raw)
  To: Chris Wilson, igt-dev, Martin Peres

On Wed, Nov 14, 2018 at 01:56:53PM +0200, Petri Latvala wrote:
> On Tue, Nov 13, 2018 at 05:25:47PM +0000, Chris Wilson wrote:
> > I have whinged on for ages about the dmesg-warnings being an expected
> > part of kernel testing (where else is the kernel meant to log its
> > errors?) and should be treated the same as our stderr for the test. That
> > is if a test fails, it fails and does not need to be conflated with
> > whether or not there was a dmesg warning (just as the test saying why it
> > failed on stderr does not need flagging), and that a passing test with a
> > dmesg warning is simply a warn.
> > 
> > The effect is that we simply remove the "dmesg-" flagging from results
> > names, as the err/dmesg output is simply collated for the error report
> > already.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Petri Latvala <petri.latvala@intel.com>
> > Cc: Martin Peres <martin.peres@linux.intel.com>
> 
> 
> Yep, now's a good a time as any to remove this cargo cult if ever.
> 
> Some acks from kernel people in general would be nice.
> 
> TODO:
>  Martin: cibuglog filters ready for this
>  Tomi et al: Visualization ready for this (tooltips etc)

Can I still see from the color/etc. whether there was some dmesg spew or
not? I often glance at those and would prefer to not have to go through
all the fails to find them.

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] runner: Treat dmesg warnings as pure warnings
  2018-11-14 13:25   ` Ville Syrjälä
@ 2018-11-14 13:34     ` Chris Wilson
  2018-11-14 13:53       ` Ville Syrjälä
  0 siblings, 1 reply; 14+ messages in thread
From: Chris Wilson @ 2018-11-14 13:34 UTC (permalink / raw)
  To: Martin Peres, Ville Syrjälä, igt-dev

Quoting Ville Syrjälä (2018-11-14 13:25:39)
> On Wed, Nov 14, 2018 at 01:56:53PM +0200, Petri Latvala wrote:
> > On Tue, Nov 13, 2018 at 05:25:47PM +0000, Chris Wilson wrote:
> > > I have whinged on for ages about the dmesg-warnings being an expected
> > > part of kernel testing (where else is the kernel meant to log its
> > > errors?) and should be treated the same as our stderr for the test. That
> > > is if a test fails, it fails and does not need to be conflated with
> > > whether or not there was a dmesg warning (just as the test saying why it
> > > failed on stderr does not need flagging), and that a passing test with a
> > > dmesg warning is simply a warn.
> > > 
> > > The effect is that we simply remove the "dmesg-" flagging from results
> > > names, as the err/dmesg output is simply collated for the error report
> > > already.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > Cc: Martin Peres <martin.peres@linux.intel.com>
> > 
> > 
> > Yep, now's a good a time as any to remove this cargo cult if ever.
> > 
> > Some acks from kernel people in general would be nice.
> > 
> > TODO:
> >  Martin: cibuglog filters ready for this
> >  Tomi et al: Visualization ready for this (tooltips etc)
> 
> Can I still see from the color/etc. whether there was some dmesg spew or
> not? I often glance at those and would prefer to not have to go through
> all the fails to find them.

It was never shown in the colour:

	warn/dmesg-warn == orange
	fail/dmesg-fail == red

If there's a warning in dmesg it'll either be orange if it didn't result
in a test failure, or red if it did impact on user behaviour.

Also coupled with Petri's runner changes, if it's a severe warning
(coupled with a taint), then the CI runner will terminate and reboot.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] runner: Treat dmesg warnings as pure warnings
  2018-11-14 13:34     ` Chris Wilson
@ 2018-11-14 13:53       ` Ville Syrjälä
  2018-11-21  8:07         ` Joonas Lahtinen
  0 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2018-11-14 13:53 UTC (permalink / raw)
  To: Chris Wilson; +Cc: igt-dev, Martin Peres

On Wed, Nov 14, 2018 at 01:34:10PM +0000, Chris Wilson wrote:
> Quoting Ville Syrjälä (2018-11-14 13:25:39)
> > On Wed, Nov 14, 2018 at 01:56:53PM +0200, Petri Latvala wrote:
> > > On Tue, Nov 13, 2018 at 05:25:47PM +0000, Chris Wilson wrote:
> > > > I have whinged on for ages about the dmesg-warnings being an expected
> > > > part of kernel testing (where else is the kernel meant to log its
> > > > errors?) and should be treated the same as our stderr for the test. That
> > > > is if a test fails, it fails and does not need to be conflated with
> > > > whether or not there was a dmesg warning (just as the test saying why it
> > > > failed on stderr does not need flagging), and that a passing test with a
> > > > dmesg warning is simply a warn.
> > > > 
> > > > The effect is that we simply remove the "dmesg-" flagging from results
> > > > names, as the err/dmesg output is simply collated for the error report
> > > > already.
> > > > 
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > Cc: Martin Peres <martin.peres@linux.intel.com>
> > > 
> > > 
> > > Yep, now's a good a time as any to remove this cargo cult if ever.
> > > 
> > > Some acks from kernel people in general would be nice.
> > > 
> > > TODO:
> > >  Martin: cibuglog filters ready for this
> > >  Tomi et al: Visualization ready for this (tooltips etc)
> > 
> > Can I still see from the color/etc. whether there was some dmesg spew or
> > not? I often glance at those and would prefer to not have to go through
> > all the fails to find them.
> 
> It was never shown in the colour:
> 
> 	warn/dmesg-warn == orange
> 	fail/dmesg-fail == red

Indeed. Which makes me wonder whether I've been overlooking some
dmesg noise. Either that or it's just my memory playing tricks
and telling me that I've mostly been looking at the orange stuff.

So let's consider this a feature request then. I would like to see
from the overview which tests produced dmesg noise, regardless of
whether the test otherwise succeeded or failed.

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] runner: Treat dmesg warnings as pure warnings
  2018-11-14 13:53       ` Ville Syrjälä
@ 2018-11-21  8:07         ` Joonas Lahtinen
  2018-11-21  9:18           ` Chris Wilson
  0 siblings, 1 reply; 14+ messages in thread
From: Joonas Lahtinen @ 2018-11-21  8:07 UTC (permalink / raw)
  To: Chris Wilson, Ville Syrjälä; +Cc: igt-dev, Martin Peres

Quoting Ville Syrjälä (2018-11-14 15:53:26)
> On Wed, Nov 14, 2018 at 01:34:10PM +0000, Chris Wilson wrote:
> > Quoting Ville Syrjälä (2018-11-14 13:25:39)
> > > On Wed, Nov 14, 2018 at 01:56:53PM +0200, Petri Latvala wrote:
> > > > On Tue, Nov 13, 2018 at 05:25:47PM +0000, Chris Wilson wrote:
> > > > > I have whinged on for ages about the dmesg-warnings being an expected
> > > > > part of kernel testing (where else is the kernel meant to log its
> > > > > errors?) and should be treated the same as our stderr for the test. That
> > > > > is if a test fails, it fails and does not need to be conflated with
> > > > > whether or not there was a dmesg warning (just as the test saying why it
> > > > > failed on stderr does not need flagging), and that a passing test with a
> > > > > dmesg warning is simply a warn.
> > > > > 
> > > > > The effect is that we simply remove the "dmesg-" flagging from results
> > > > > names, as the err/dmesg output is simply collated for the error report
> > > > > already.
> > > > > 
> > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > > Cc: Martin Peres <martin.peres@linux.intel.com>
> > > > 
> > > > 
> > > > Yep, now's a good a time as any to remove this cargo cult if ever.
> > > > 
> > > > Some acks from kernel people in general would be nice.
> > > > 
> > > > TODO:
> > > >  Martin: cibuglog filters ready for this
> > > >  Tomi et al: Visualization ready for this (tooltips etc)
> > > 
> > > Can I still see from the color/etc. whether there was some dmesg spew or
> > > not? I often glance at those and would prefer to not have to go through
> > > all the fails to find them.
> > 
> > It was never shown in the colour:
> > 
> >       warn/dmesg-warn == orange
> >       fail/dmesg-fail == red
> 
> Indeed. Which makes me wonder whether I've been overlooking some
> dmesg noise. Either that or it's just my memory playing tricks
> and telling me that I've mostly been looking at the orange stuff.
> 
> So let's consider this a feature request then. I would like to see
> from the overview which tests produced dmesg noise, regardless of
> whether the test otherwise succeeded or failed.

I think this is a fair change.

And for documentation purposes, what I proposed in IRC:

My suggestion was to add letter "D" to the matrix when there is dmesg
noise in the specific subtest (Similarly as there's letter T in
timeout).

Regards, Joonas

> 
> -- 
> Ville Syrjälä
> Intel
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] runner: Treat dmesg warnings as pure warnings
  2018-11-21  8:07         ` Joonas Lahtinen
@ 2018-11-21  9:18           ` Chris Wilson
  2018-11-21 10:05             ` Joonas Lahtinen
  0 siblings, 1 reply; 14+ messages in thread
From: Chris Wilson @ 2018-11-21  9:18 UTC (permalink / raw)
  To: Ville Syrjälä, Joonas Lahtinen; +Cc: igt-dev, Martin Peres

Quoting Joonas Lahtinen (2018-11-21 08:07:30)
> Quoting Ville Syrjälä (2018-11-14 15:53:26)
> > On Wed, Nov 14, 2018 at 01:34:10PM +0000, Chris Wilson wrote:
> > > Quoting Ville Syrjälä (2018-11-14 13:25:39)
> > > > On Wed, Nov 14, 2018 at 01:56:53PM +0200, Petri Latvala wrote:
> > > > > On Tue, Nov 13, 2018 at 05:25:47PM +0000, Chris Wilson wrote:
> > > > > > I have whinged on for ages about the dmesg-warnings being an expected
> > > > > > part of kernel testing (where else is the kernel meant to log its
> > > > > > errors?) and should be treated the same as our stderr for the test. That
> > > > > > is if a test fails, it fails and does not need to be conflated with
> > > > > > whether or not there was a dmesg warning (just as the test saying why it
> > > > > > failed on stderr does not need flagging), and that a passing test with a
> > > > > > dmesg warning is simply a warn.
> > > > > > 
> > > > > > The effect is that we simply remove the "dmesg-" flagging from results
> > > > > > names, as the err/dmesg output is simply collated for the error report
> > > > > > already.
> > > > > > 
> > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > > > Cc: Martin Peres <martin.peres@linux.intel.com>
> > > > > 
> > > > > 
> > > > > Yep, now's a good a time as any to remove this cargo cult if ever.
> > > > > 
> > > > > Some acks from kernel people in general would be nice.
> > > > > 
> > > > > TODO:
> > > > >  Martin: cibuglog filters ready for this
> > > > >  Tomi et al: Visualization ready for this (tooltips etc)
> > > > 
> > > > Can I still see from the color/etc. whether there was some dmesg spew or
> > > > not? I often glance at those and would prefer to not have to go through
> > > > all the fails to find them.
> > > 
> > > It was never shown in the colour:
> > > 
> > >       warn/dmesg-warn == orange
> > >       fail/dmesg-fail == red
> > 
> > Indeed. Which makes me wonder whether I've been overlooking some
> > dmesg noise. Either that or it's just my memory playing tricks
> > and telling me that I've mostly been looking at the orange stuff.
> > 
> > So let's consider this a feature request then. I would like to see
> > from the overview which tests produced dmesg noise, regardless of
> > whether the test otherwise succeeded or failed.
> 
> I think this is a fair change.
> 
> And for documentation purposes, what I proposed in IRC:
> 
> My suggestion was to add letter "D" to the matrix when there is dmesg
> noise in the specific subtest (Similarly as there's letter T in
> timeout).

I just don't understand what is "dmesg noise". dmesg is where I log my
errors, there is no other suitable log.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] runner: Treat dmesg warnings as pure warnings
  2018-11-21  9:18           ` Chris Wilson
@ 2018-11-21 10:05             ` Joonas Lahtinen
  2018-11-22  9:41               ` Daniel Vetter
  0 siblings, 1 reply; 14+ messages in thread
From: Joonas Lahtinen @ 2018-11-21 10:05 UTC (permalink / raw)
  To: Ville Syrjälä, Chris Wilson; +Cc: igt-dev, Martin Peres

Quoting Chris Wilson (2018-11-21 11:18:43)
> Quoting Joonas Lahtinen (2018-11-21 08:07:30)
> > Quoting Ville Syrjälä (2018-11-14 15:53:26)
> > > On Wed, Nov 14, 2018 at 01:34:10PM +0000, Chris Wilson wrote:
> > > > Quoting Ville Syrjälä (2018-11-14 13:25:39)
> > > > > On Wed, Nov 14, 2018 at 01:56:53PM +0200, Petri Latvala wrote:
> > > > > > On Tue, Nov 13, 2018 at 05:25:47PM +0000, Chris Wilson wrote:
> > > > > > > I have whinged on for ages about the dmesg-warnings being an expected
> > > > > > > part of kernel testing (where else is the kernel meant to log its
> > > > > > > errors?) and should be treated the same as our stderr for the test. That
> > > > > > > is if a test fails, it fails and does not need to be conflated with
> > > > > > > whether or not there was a dmesg warning (just as the test saying why it
> > > > > > > failed on stderr does not need flagging), and that a passing test with a
> > > > > > > dmesg warning is simply a warn.
> > > > > > > 
> > > > > > > The effect is that we simply remove the "dmesg-" flagging from results
> > > > > > > names, as the err/dmesg output is simply collated for the error report
> > > > > > > already.
> > > > > > > 
> > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > > > > Cc: Martin Peres <martin.peres@linux.intel.com>
> > > > > > 
> > > > > > 
> > > > > > Yep, now's a good a time as any to remove this cargo cult if ever.
> > > > > > 
> > > > > > Some acks from kernel people in general would be nice.
> > > > > > 
> > > > > > TODO:
> > > > > >  Martin: cibuglog filters ready for this
> > > > > >  Tomi et al: Visualization ready for this (tooltips etc)
> > > > > 
> > > > > Can I still see from the color/etc. whether there was some dmesg spew or
> > > > > not? I often glance at those and would prefer to not have to go through
> > > > > all the fails to find them.
> > > > 
> > > > It was never shown in the colour:
> > > > 
> > > >       warn/dmesg-warn == orange
> > > >       fail/dmesg-fail == red
> > > 
> > > Indeed. Which makes me wonder whether I've been overlooking some
> > > dmesg noise. Either that or it's just my memory playing tricks
> > > and telling me that I've mostly been looking at the orange stuff.
> > > 
> > > So let's consider this a feature request then. I would like to see
> > > from the overview which tests produced dmesg noise, regardless of
> > > whether the test otherwise succeeded or failed.
> > 
> > I think this is a fair change.
> > 
> > And for documentation purposes, what I proposed in IRC:
> > 
> > My suggestion was to add letter "D" to the matrix when there is dmesg
> > noise in the specific subtest (Similarly as there's letter T in
> > timeout).
> 
> I just don't understand what is "dmesg noise". dmesg is where I log my
> errors, there is no other suitable log.

My take on this is that it provides a quick differentation of:

"The IGT test fails, functionality lost"

"The IGT test passes, functionality is there, but makes kernel spew
error (recoverably)"

Both are errors, but there's informational value in seeing the
difference, at least to me.

Regards, Joonas

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] runner: Treat dmesg warnings as pure warnings
  2018-11-21 10:05             ` Joonas Lahtinen
@ 2018-11-22  9:41               ` Daniel Vetter
  2018-11-22  9:42                 ` Daniel Vetter
  2018-11-22 12:59                 ` Joonas Lahtinen
  0 siblings, 2 replies; 14+ messages in thread
From: Daniel Vetter @ 2018-11-22  9:41 UTC (permalink / raw)
  To: Joonas Lahtinen; +Cc: igt-dev, Martin Peres

On Wed, Nov 21, 2018 at 12:05:45PM +0200, Joonas Lahtinen wrote:
> Quoting Chris Wilson (2018-11-21 11:18:43)
> > Quoting Joonas Lahtinen (2018-11-21 08:07:30)
> > > Quoting Ville Syrjälä (2018-11-14 15:53:26)
> > > > On Wed, Nov 14, 2018 at 01:34:10PM +0000, Chris Wilson wrote:
> > > > > Quoting Ville Syrjälä (2018-11-14 13:25:39)
> > > > > > On Wed, Nov 14, 2018 at 01:56:53PM +0200, Petri Latvala wrote:
> > > > > > > On Tue, Nov 13, 2018 at 05:25:47PM +0000, Chris Wilson wrote:
> > > > > > > > I have whinged on for ages about the dmesg-warnings being an expected
> > > > > > > > part of kernel testing (where else is the kernel meant to log its
> > > > > > > > errors?) and should be treated the same as our stderr for the test. That
> > > > > > > > is if a test fails, it fails and does not need to be conflated with
> > > > > > > > whether or not there was a dmesg warning (just as the test saying why it
> > > > > > > > failed on stderr does not need flagging), and that a passing test with a
> > > > > > > > dmesg warning is simply a warn.
> > > > > > > > 
> > > > > > > > The effect is that we simply remove the "dmesg-" flagging from results
> > > > > > > > names, as the err/dmesg output is simply collated for the error report
> > > > > > > > already.
> > > > > > > > 
> > > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > > > > > Cc: Martin Peres <martin.peres@linux.intel.com>
> > > > > > > 
> > > > > > > 
> > > > > > > Yep, now's a good a time as any to remove this cargo cult if ever.
> > > > > > > 
> > > > > > > Some acks from kernel people in general would be nice.
> > > > > > > 
> > > > > > > TODO:
> > > > > > >  Martin: cibuglog filters ready for this
> > > > > > >  Tomi et al: Visualization ready for this (tooltips etc)
> > > > > > 
> > > > > > Can I still see from the color/etc. whether there was some dmesg spew or
> > > > > > not? I often glance at those and would prefer to not have to go through
> > > > > > all the fails to find them.
> > > > > 
> > > > > It was never shown in the colour:
> > > > > 
> > > > >       warn/dmesg-warn == orange
> > > > >       fail/dmesg-fail == red
> > > > 
> > > > Indeed. Which makes me wonder whether I've been overlooking some
> > > > dmesg noise. Either that or it's just my memory playing tricks
> > > > and telling me that I've mostly been looking at the orange stuff.
> > > > 
> > > > So let's consider this a feature request then. I would like to see
> > > > from the overview which tests produced dmesg noise, regardless of
> > > > whether the test otherwise succeeded or failed.
> > > 
> > > I think this is a fair change.
> > > 
> > > And for documentation purposes, what I proposed in IRC:
> > > 
> > > My suggestion was to add letter "D" to the matrix when there is dmesg
> > > noise in the specific subtest (Similarly as there's letter T in
> > > timeout).
> > 
> > I just don't understand what is "dmesg noise". dmesg is where I log my
> > errors, there is no other suitable log.
> 
> My take on this is that it provides a quick differentation of:
> 
> "The IGT test fails, functionality lost"
> 
> "The IGT test passes, functionality is there, but makes kernel spew
> error (recoverably)"
> 
> Both are errors, but there's informational value in seeing the
> difference, at least to me.

There's lots of igt tests that entirely rely on the in-kernel checks to
validate stuff. dmesg output in that case means "test failed,
functionality lost".

I'm kinda leaning towards just marking anything with dmesg warn-and-worse
as "fail". And restrict warn/orange to igt_warn and friends. If that's too
much we can think about a DRM_WARN and using warning level in syslog to
result in "warn", and error and worse levels in "fail".

Kernel on fire, but hey look the test passed is not a conclusion we should
ever support imo. Given that Joonas has it, I'd say we need to be more
clear here that dmesg = stuff failed hard.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] runner: Treat dmesg warnings as pure warnings
  2018-11-22  9:41               ` Daniel Vetter
@ 2018-11-22  9:42                 ` Daniel Vetter
  2018-11-22 12:59                 ` Joonas Lahtinen
  1 sibling, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2018-11-22  9:42 UTC (permalink / raw)
  To: Joonas Lahtinen; +Cc: igt-dev, Martin Peres

On Thu, Nov 22, 2018 at 10:41:26AM +0100, Daniel Vetter wrote:
> On Wed, Nov 21, 2018 at 12:05:45PM +0200, Joonas Lahtinen wrote:
> > Quoting Chris Wilson (2018-11-21 11:18:43)
> > > Quoting Joonas Lahtinen (2018-11-21 08:07:30)
> > > > Quoting Ville Syrjälä (2018-11-14 15:53:26)
> > > > > On Wed, Nov 14, 2018 at 01:34:10PM +0000, Chris Wilson wrote:
> > > > > > Quoting Ville Syrjälä (2018-11-14 13:25:39)
> > > > > > > On Wed, Nov 14, 2018 at 01:56:53PM +0200, Petri Latvala wrote:
> > > > > > > > On Tue, Nov 13, 2018 at 05:25:47PM +0000, Chris Wilson wrote:
> > > > > > > > > I have whinged on for ages about the dmesg-warnings being an expected
> > > > > > > > > part of kernel testing (where else is the kernel meant to log its
> > > > > > > > > errors?) and should be treated the same as our stderr for the test. That
> > > > > > > > > is if a test fails, it fails and does not need to be conflated with
> > > > > > > > > whether or not there was a dmesg warning (just as the test saying why it
> > > > > > > > > failed on stderr does not need flagging), and that a passing test with a
> > > > > > > > > dmesg warning is simply a warn.
> > > > > > > > > 
> > > > > > > > > The effect is that we simply remove the "dmesg-" flagging from results
> > > > > > > > > names, as the err/dmesg output is simply collated for the error report
> > > > > > > > > already.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > > > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > > > > > > Cc: Martin Peres <martin.peres@linux.intel.com>
> > > > > > > > 
> > > > > > > > 
> > > > > > > > Yep, now's a good a time as any to remove this cargo cult if ever.
> > > > > > > > 
> > > > > > > > Some acks from kernel people in general would be nice.
> > > > > > > > 
> > > > > > > > TODO:
> > > > > > > >  Martin: cibuglog filters ready for this
> > > > > > > >  Tomi et al: Visualization ready for this (tooltips etc)
> > > > > > > 
> > > > > > > Can I still see from the color/etc. whether there was some dmesg spew or
> > > > > > > not? I often glance at those and would prefer to not have to go through
> > > > > > > all the fails to find them.
> > > > > > 
> > > > > > It was never shown in the colour:
> > > > > > 
> > > > > >       warn/dmesg-warn == orange
> > > > > >       fail/dmesg-fail == red
> > > > > 
> > > > > Indeed. Which makes me wonder whether I've been overlooking some
> > > > > dmesg noise. Either that or it's just my memory playing tricks
> > > > > and telling me that I've mostly been looking at the orange stuff.
> > > > > 
> > > > > So let's consider this a feature request then. I would like to see
> > > > > from the overview which tests produced dmesg noise, regardless of
> > > > > whether the test otherwise succeeded or failed.
> > > > 
> > > > I think this is a fair change.
> > > > 
> > > > And for documentation purposes, what I proposed in IRC:
> > > > 
> > > > My suggestion was to add letter "D" to the matrix when there is dmesg
> > > > noise in the specific subtest (Similarly as there's letter T in
> > > > timeout).
> > > 
> > > I just don't understand what is "dmesg noise". dmesg is where I log my
> > > errors, there is no other suitable log.
> > 
> > My take on this is that it provides a quick differentation of:
> > 
> > "The IGT test fails, functionality lost"
> > 
> > "The IGT test passes, functionality is there, but makes kernel spew
> > error (recoverably)"
> > 
> > Both are errors, but there's informational value in seeing the
> > difference, at least to me.
> 
> There's lots of igt tests that entirely rely on the in-kernel checks to
> validate stuff. dmesg output in that case means "test failed,
> functionality lost".
> 
> I'm kinda leaning towards just marking anything with dmesg warn-and-worse
> as "fail". And restrict warn/orange to igt_warn and friends. If that's too
> much we can think about a DRM_WARN and using warning level in syslog to
> result in "warn", and error and worse levels in "fail".
> 
> Kernel on fire, but hey look the test passed is not a conclusion we should
> ever support imo. Given that Joonas has it, I'd say we need to be more
> clear here that dmesg = stuff failed hard.

This would also match what we do in userspace testing in mesa, where debug
builds with assert() enabled also fail hard. And warning is reserved only
for stderr output, i.e. a conscious decision that this failure should only
be reported where no one ever really looks. At least not users :-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] runner: Treat dmesg warnings as pure warnings
  2018-11-22  9:41               ` Daniel Vetter
  2018-11-22  9:42                 ` Daniel Vetter
@ 2018-11-22 12:59                 ` Joonas Lahtinen
  2018-11-22 14:26                   ` Daniel Vetter
  1 sibling, 1 reply; 14+ messages in thread
From: Joonas Lahtinen @ 2018-11-22 12:59 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: igt-dev, Martin Peres

Quoting Daniel Vetter (2018-11-22 11:41:26)
> On Wed, Nov 21, 2018 at 12:05:45PM +0200, Joonas Lahtinen wrote:
> > Quoting Chris Wilson (2018-11-21 11:18:43)
> > > Quoting Joonas Lahtinen (2018-11-21 08:07:30)
> > > > Quoting Ville Syrjälä (2018-11-14 15:53:26)
> > > > > On Wed, Nov 14, 2018 at 01:34:10PM +0000, Chris Wilson wrote:
> > > > > > Quoting Ville Syrjälä (2018-11-14 13:25:39)
> > > > > > > On Wed, Nov 14, 2018 at 01:56:53PM +0200, Petri Latvala wrote:
> > > > > > > > On Tue, Nov 13, 2018 at 05:25:47PM +0000, Chris Wilson wrote:
> > > > > > > > > I have whinged on for ages about the dmesg-warnings being an expected
> > > > > > > > > part of kernel testing (where else is the kernel meant to log its
> > > > > > > > > errors?) and should be treated the same as our stderr for the test. That
> > > > > > > > > is if a test fails, it fails and does not need to be conflated with
> > > > > > > > > whether or not there was a dmesg warning (just as the test saying why it
> > > > > > > > > failed on stderr does not need flagging), and that a passing test with a
> > > > > > > > > dmesg warning is simply a warn.
> > > > > > > > > 
> > > > > > > > > The effect is that we simply remove the "dmesg-" flagging from results
> > > > > > > > > names, as the err/dmesg output is simply collated for the error report
> > > > > > > > > already.
> > > > > > > > > 
> > > > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > > > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > > > > > > Cc: Martin Peres <martin.peres@linux.intel.com>
> > > > > > > > 
> > > > > > > > 
> > > > > > > > Yep, now's a good a time as any to remove this cargo cult if ever.
> > > > > > > > 
> > > > > > > > Some acks from kernel people in general would be nice.
> > > > > > > > 
> > > > > > > > TODO:
> > > > > > > >  Martin: cibuglog filters ready for this
> > > > > > > >  Tomi et al: Visualization ready for this (tooltips etc)
> > > > > > > 
> > > > > > > Can I still see from the color/etc. whether there was some dmesg spew or
> > > > > > > not? I often glance at those and would prefer to not have to go through
> > > > > > > all the fails to find them.
> > > > > > 
> > > > > > It was never shown in the colour:
> > > > > > 
> > > > > >       warn/dmesg-warn == orange
> > > > > >       fail/dmesg-fail == red
> > > > > 
> > > > > Indeed. Which makes me wonder whether I've been overlooking some
> > > > > dmesg noise. Either that or it's just my memory playing tricks
> > > > > and telling me that I've mostly been looking at the orange stuff.
> > > > > 
> > > > > So let's consider this a feature request then. I would like to see
> > > > > from the overview which tests produced dmesg noise, regardless of
> > > > > whether the test otherwise succeeded or failed.
> > > > 
> > > > I think this is a fair change.
> > > > 
> > > > And for documentation purposes, what I proposed in IRC:
> > > > 
> > > > My suggestion was to add letter "D" to the matrix when there is dmesg
> > > > noise in the specific subtest (Similarly as there's letter T in
> > > > timeout).
> > > 
> > > I just don't understand what is "dmesg noise". dmesg is where I log my
> > > errors, there is no other suitable log.
> > 
> > My take on this is that it provides a quick differentation of:
> > 
> > "The IGT test fails, functionality lost"
> > 
> > "The IGT test passes, functionality is there, but makes kernel spew
> > error (recoverably)"
> > 
> > Both are errors, but there's informational value in seeing the
> > difference, at least to me.
> 
> There's lots of igt tests that entirely rely on the in-kernel checks to
> validate stuff. dmesg output in that case means "test failed,
> functionality lost".
> 
> I'm kinda leaning towards just marking anything with dmesg warn-and-worse
> as "fail". And restrict warn/orange to igt_warn and friends. If that's too
> much we can think about a DRM_WARN and using warning level in syslog to
> result in "warn", and error and worse levels in "fail".
> 
> Kernel on fire, but hey look the test passed is not a conclusion we should
> ever support imo. Given that Joonas has it, I'd say we need to be more
> clear here that dmesg = stuff failed hard.

Well, such tests sound like a candidate to be moved to kernel selftests,
if they look to just make sure kernel doesn't trip over itself. Maybe
helps to identify such tests, where only expected failure mode is kernel
message.

Because there for sure is a difference between having WARNs in your dmesg
log while the program continues to work vs. program crashing/not working,
when it comes to severity.

Regards, Joonas

> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [igt-dev] [PATCH i-g-t] runner: Treat dmesg warnings as pure warnings
  2018-11-22 12:59                 ` Joonas Lahtinen
@ 2018-11-22 14:26                   ` Daniel Vetter
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2018-11-22 14:26 UTC (permalink / raw)
  To: Joonas Lahtinen; +Cc: igt-dev, Martin Peres, Daniel Vetter

On Thu, Nov 22, 2018 at 02:59:35PM +0200, Joonas Lahtinen wrote:
> Quoting Daniel Vetter (2018-11-22 11:41:26)
> > On Wed, Nov 21, 2018 at 12:05:45PM +0200, Joonas Lahtinen wrote:
> > > Quoting Chris Wilson (2018-11-21 11:18:43)
> > > > Quoting Joonas Lahtinen (2018-11-21 08:07:30)
> > > > > Quoting Ville Syrjälä (2018-11-14 15:53:26)
> > > > > > On Wed, Nov 14, 2018 at 01:34:10PM +0000, Chris Wilson wrote:
> > > > > > > Quoting Ville Syrjälä (2018-11-14 13:25:39)
> > > > > > > > On Wed, Nov 14, 2018 at 01:56:53PM +0200, Petri Latvala wrote:
> > > > > > > > > On Tue, Nov 13, 2018 at 05:25:47PM +0000, Chris Wilson wrote:
> > > > > > > > > > I have whinged on for ages about the dmesg-warnings being an expected
> > > > > > > > > > part of kernel testing (where else is the kernel meant to log its
> > > > > > > > > > errors?) and should be treated the same as our stderr for the test. That
> > > > > > > > > > is if a test fails, it fails and does not need to be conflated with
> > > > > > > > > > whether or not there was a dmesg warning (just as the test saying why it
> > > > > > > > > > failed on stderr does not need flagging), and that a passing test with a
> > > > > > > > > > dmesg warning is simply a warn.
> > > > > > > > > > 
> > > > > > > > > > The effect is that we simply remove the "dmesg-" flagging from results
> > > > > > > > > > names, as the err/dmesg output is simply collated for the error report
> > > > > > > > > > already.
> > > > > > > > > > 
> > > > > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > > > > > > Cc: Petri Latvala <petri.latvala@intel.com>
> > > > > > > > > > Cc: Martin Peres <martin.peres@linux.intel.com>
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > Yep, now's a good a time as any to remove this cargo cult if ever.
> > > > > > > > > 
> > > > > > > > > Some acks from kernel people in general would be nice.
> > > > > > > > > 
> > > > > > > > > TODO:
> > > > > > > > >  Martin: cibuglog filters ready for this
> > > > > > > > >  Tomi et al: Visualization ready for this (tooltips etc)
> > > > > > > > 
> > > > > > > > Can I still see from the color/etc. whether there was some dmesg spew or
> > > > > > > > not? I often glance at those and would prefer to not have to go through
> > > > > > > > all the fails to find them.
> > > > > > > 
> > > > > > > It was never shown in the colour:
> > > > > > > 
> > > > > > >       warn/dmesg-warn == orange
> > > > > > >       fail/dmesg-fail == red
> > > > > > 
> > > > > > Indeed. Which makes me wonder whether I've been overlooking some
> > > > > > dmesg noise. Either that or it's just my memory playing tricks
> > > > > > and telling me that I've mostly been looking at the orange stuff.
> > > > > > 
> > > > > > So let's consider this a feature request then. I would like to see
> > > > > > from the overview which tests produced dmesg noise, regardless of
> > > > > > whether the test otherwise succeeded or failed.
> > > > > 
> > > > > I think this is a fair change.
> > > > > 
> > > > > And for documentation purposes, what I proposed in IRC:
> > > > > 
> > > > > My suggestion was to add letter "D" to the matrix when there is dmesg
> > > > > noise in the specific subtest (Similarly as there's letter T in
> > > > > timeout).
> > > > 
> > > > I just don't understand what is "dmesg noise". dmesg is where I log my
> > > > errors, there is no other suitable log.
> > > 
> > > My take on this is that it provides a quick differentation of:
> > > 
> > > "The IGT test fails, functionality lost"
> > > 
> > > "The IGT test passes, functionality is there, but makes kernel spew
> > > error (recoverably)"
> > > 
> > > Both are errors, but there's informational value in seeing the
> > > difference, at least to me.
> > 
> > There's lots of igt tests that entirely rely on the in-kernel checks to
> > validate stuff. dmesg output in that case means "test failed,
> > functionality lost".
> > 
> > I'm kinda leaning towards just marking anything with dmesg warn-and-worse
> > as "fail". And restrict warn/orange to igt_warn and friends. If that's too
> > much we can think about a DRM_WARN and using warning level in syslog to
> > result in "warn", and error and worse levels in "fail".
> > 
> > Kernel on fire, but hey look the test passed is not a conclusion we should
> > ever support imo. Given that Joonas has it, I'd say we need to be more
> > clear here that dmesg = stuff failed hard.
> 
> Well, such tests sound like a candidate to be moved to kernel selftests,
> if they look to just make sure kernel doesn't trip over itself. Maybe
> helps to identify such tests, where only expected failure mode is kernel
> message.
> 
> Because there for sure is a difference between having WARNs in your dmesg
> log while the program continues to work vs. program crashing/not working,
> when it comes to severity.

lockdep warnings, modeset state checker, GEM_BUG_ON, the list is _very_
long. You're essentially proposing that we move pretty much all of igt
into the kernel, that doesn't make sense.

Most of these really are nothing else than assert() calls in a different
place. If you test userspace, then if you hit an assert() in one of the
libraries you're testing, you're also not declaring those bugs to be just
warnings instead of full failures.

We could just convert them all to BUG_ON, which would more closely
resemble what we'd do in userspace. But that makes debugging unecesarily
painful. But "it's easier to debug if the kernel keeps chugging along" is
the only reason we're trying to keep going, not that these failures aren't
fairly severe issues. And I think this is the same case Chris made earlier
in this thread: WARN_ON() == assert() for the kernel, more or less. And
these WARNING backtraces are the clear majority of the dmesg failures we have.

Hence why I think if we're going to simplify all this, we should map dmesg
failures to an overall "fail" status. And then maybe later on figure out
how we could wire up the stderr output/"warn" level for the kernel too
(e.g. through a DRM_WARNING() set of macros, and corresponding log
levels).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-11-22 14:26 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-13 17:25 [igt-dev] [PATCH i-g-t] runner: Treat dmesg warnings as pure warnings Chris Wilson
2018-11-13 18:40 ` [igt-dev] ✗ Fi.CI.BAT: failure for " Patchwork
2018-11-14 11:56 ` [igt-dev] [PATCH i-g-t] " Petri Latvala
2018-11-14 13:25   ` Ville Syrjälä
2018-11-14 13:34     ` Chris Wilson
2018-11-14 13:53       ` Ville Syrjälä
2018-11-21  8:07         ` Joonas Lahtinen
2018-11-21  9:18           ` Chris Wilson
2018-11-21 10:05             ` Joonas Lahtinen
2018-11-22  9:41               ` Daniel Vetter
2018-11-22  9:42                 ` Daniel Vetter
2018-11-22 12:59                 ` Joonas Lahtinen
2018-11-22 14:26                   ` Daniel Vetter
2018-11-14 13:04 ` [igt-dev] ✗ Fi.CI.BAT: failure for runner: Treat dmesg warnings as pure warnings (rev2) Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.