Re: [PATCH i-g-t v2 7/7] runner/resultgen: Add cmdline to results.json

[Petri Latvala <adrinael@adrinael.net>]

On Fri, Jan 24, 2025 at 02:20:41PM +0000, Knop, Ryszard wrote:
> On Thu, 2025-01-23 at 00:50 -0600, Lucas De Marchi wrote:
> > On Wed, Jan 22, 2025 at 09:51:42AM -0300, Gustavo Sousa wrote:
> > > Quoting Lucas De Marchi (2025-01-21 19:57:33-03:00)
> > > > For easier repro scenarios, add the cmdline to the json: one can see the
> > > > exact command executed to try to reproduce a CI failure without needing
> > > > extra files.
> > > > 
> > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > ---
> > > > runner/json_tests_data/aborted-after-a-test/reference.json | 1 +
> > > > runner/json_tests_data/aborted-on-boot/reference.json      | 1 +
> > > > runner/json_tests_data/dmesg-escapes/reference.json        | 1 +
> > > > runner/json_tests_data/dmesg-results/reference.json        | 1 +
> > > > .../dmesg-warn-level-one-piglit-style/reference.json       | 1 +
> > > > .../dmesg-warn-level-piglit-style/reference.json           | 1 +
> > > > runner/json_tests_data/dmesg-warn-level/reference.json     | 1 +
> > > > .../reference.json                                         | 1 +
> > > > .../dynamic-subtests-keep-all/reference.json               | 3 ++-
> > > > .../dynamic-subtests-keep-dynamic/reference.json           | 1 +
> > > > .../dynamic-subtests-keep-requested/reference.json         | 3 ++-
> > > > .../dynamic-subtests-keep-subtests/reference.json          | 3 ++-
> > > > runner/json_tests_data/empty-result-files/reference.json   | 1 +
> > > > runner/json_tests_data/graceful-notrun/reference.json      | 1 +
> > > > .../incomplete-before-any-subtests/reference.json          | 1 +
> > > > runner/json_tests_data/normal-run/reference.json           | 1 +
> > > > .../notrun-results-multiple-mode/reference.json            | 1 +
> > > > runner/json_tests_data/notrun-results/reference.json       | 1 +
> > > > runner/json_tests_data/piglit-style-dmesg/reference.json   | 1 +
> > > > .../json_tests_data/unprintable-characters/reference.json  | 1 +
> > > > .../warnings-with-dmesg-warns/reference.json               | 1 +
> > > > runner/json_tests_data/warnings/reference.json             | 1 +
> > > > runner/resultgen.c                                         | 7 ++++++-
> > > > 23 files changed, 31 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/runner/json_tests_data/aborted-after-a-test/reference.json b/runner/json_tests_data/aborted-after-a-test/reference.json
> > > > index 0776f7582..50ba9e6e1 100644
> > > > --- a/runner/json_tests_data/aborted-after-a-test/reference.json
> > > > +++ b/runner/json_tests_data/aborted-after-a-test/reference.json
> > > > @@ -3,6 +3,7 @@
> > > >   "results_version":10,
> > > >   "name":"normal-run",
> > > >   "uname":"Linux hostname 4.18.0-1-amd64 #1 SMP Debian 4.18.6-1 (2018-09-06) x86_64",
> > > > +  "cmdline":[],
> > > >   "time_elapsed":{
> > > >     "__type__":"TimeAttribute",
> > > >     "start":1539953735.1110389,
> > > > diff --git a/runner/json_tests_data/aborted-on-boot/reference.json b/runner/json_tests_data/aborted-on-boot/reference.json
> > > > index 75f194660..238ec6fff 100644
> > > > --- a/runner/json_tests_data/aborted-on-boot/reference.json
> > > > +++ b/runner/json_tests_data/aborted-on-boot/reference.json
> > > > @@ -3,6 +3,7 @@
> > > >   "results_version":10,
> > > >   "name":"normal-run",
> > > >   "uname":"Linux hostname 4.18.0-1-amd64 #1 SMP Debian 4.18.6-1 (2018-09-06) x86_64",
> > > > +  "cmdline":[],
> > > >   "time_elapsed":{
> > > >     "__type__":"TimeAttribute",
> > > >     "start":1539953735.1110389,
> > > > diff --git a/runner/json_tests_data/dmesg-escapes/reference.json b/runner/json_tests_data/dmesg-escapes/reference.json
> > > > index 91c573106..e74dc89e4 100644
> > > > --- a/runner/json_tests_data/dmesg-escapes/reference.json
> > > > +++ b/runner/json_tests_data/dmesg-escapes/reference.json
> > > > @@ -3,6 +3,7 @@
> > > >   "results_version":10,
> > > >   "name":"normal-run",
> > > >   "uname":"Linux hostname 4.18.0-1-amd64 #1 SMP Debian 4.18.6-1 (2018-09-06) x86_64",
> > > > +  "cmdline":[],
> > > >   "time_elapsed":{
> > > >     "__type__":"TimeAttribute",
> > > >     "start":1539953735.1110389,
> > > > diff --git a/runner/json_tests_data/dmesg-results/reference.json b/runner/json_tests_data/dmesg-results/reference.json
> > > > index e9e011853..81890f7d2 100644
> > > > --- a/runner/json_tests_data/dmesg-results/reference.json
> > > > +++ b/runner/json_tests_data/dmesg-results/reference.json
> > > > @@ -3,6 +3,7 @@
> > > >   "results_version":10,
> > > >   "name":"normal-run",
> > > >   "uname":"Linux hostname 4.18.0-1-amd64 #1 SMP Debian 4.18.6-1 (2018-09-06) x86_64",
> > > > +  "cmdline":[],
> > > >   "time_elapsed":{
> > > >     "__type__":"TimeAttribute",
> > > >     "start":1539953735.1110389,
> > > > diff --git a/runner/json_tests_data/dmesg-warn-level-one-piglit-style/reference.json b/runner/json_tests_data/dmesg-warn-level-one-piglit-style/reference.json
> > > > index 8d266cdfa..16c97d3a1 100644
> > > > --- a/runner/json_tests_data/dmesg-warn-level-one-piglit-style/reference.json
> > > > +++ b/runner/json_tests_data/dmesg-warn-level-one-piglit-style/reference.json
> > > > @@ -3,6 +3,7 @@
> > > >   "results_version":10,
> > > 
> > > Hm... Are we going to continue with 10 as the version after including
> > > another field? Are there guidelines on how results_version should be
> > > handled?
> > 
> > good question.... last time it was updated was in 2018, to be compatible
> > with piglit.
> > 
> > commit 2c793666d8c8328733f5769b16ae5858fee97f3f
> > Author: Petri Latvala <petri.latvala@intel.com>
> > Date:   Wed Dec 5 14:53:43 2018 +0200
> > 
> >      runner: Produce json with results_version=10
> >      
> >      New piglit bumped its results_version to 10, making glxinfo and pals
> >      optional in practice, not just by accident. Unfortunately reading
> >      results with newer piglit attempts to convert the results to version
> >      10, reading glxinfo and pals, and thus fails. In a hilarious summary:
> >      A commit to piglit making glxinfo optional makes it mandatory for us.
> >      
> >      v2: json unit tests confirmed to be working...
> >      
> >      Reported-by: Andi Shyti <andi.shyti@intel.com>
> >      Signed-off-by: Petri Latvala <petri.latvala@intel.com>
> >      Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> >      Cc: Andi Shyti <andi.shyti@intel.com>
> >      Tested-by: Andi Shyti <andi.shyti@intel.com>
> >      Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > 
> > Does this still work with piglit?  something I didn't test yet.
> > Thanks for noticing
> 
> In the past, there were some tools and CI runs that used piglit, but
> they are no longer around for at least 3+ years. Mesa team works with
> those now, we don't have a harness or parsing utilities that work with
> piglit et al. If it's still compatible, it's only by accident,
> considering that deqp-runner et al is under fairly active development.

IGT has for some time been able to produce results files that piglit
cannot read. For one, anything involving test result "ABORT".


-- 
Petri Latvala