[PATCH i-g-t v3 10/10] runner/resultgen: Add cmdline to results.json

[Kamil Konieczny]

Hi Lucas,
On 2025-01-30 at 13:17:11 -0600, Lucas De Marchi wrote:
> On Thu, Jan 30, 2025 at 08:00:55PM +0100, Kamil Konieczny wrote:
> > Hi Lucas,
> > On 2025-01-30 at 09:21:49 -0800, Lucas De Marchi wrote:
> > > 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.
> > > 
> > > Adding cmdline to the results.json doesn't need a version upgrade:
> > > piglit can still parse the file.
> > 
> > Why not just printing this into stdout before executing test loop?
> > The same way which is already used by igt_facts, so it will go
> > into runnerNN.txt log and will not be duplicated in each and every
> > results.json
> 
> igt_facts print things that is happening between the tests. It's
> monitoring the execution.
> 
> I don't understand the push back here when we already add things
> like version, uname, execution time, etc. You could use the same argument and say "look at the runner.log"
> to see the execution time.
> 
> I want to be able to see things rendered in the final html at
> https://intel-gfx-ci.01.org/tree/intel-xe/bat-all.html,
> make things discoverable and also be able to download the .json, process
> it and come up with the answer to "what do I have to do to reproduce
> this error locally?".  A script `repro-igt.py <results.json|url>` would
> be very handy and made possible with improvements like this.
> It's a huge benefit for reproducing and fixing bugs. I don't think the
> extra few bytes really matter.
> 

Well, what I wrote is not a blocker, I get your point so

Acked-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>

> > 
> > Adding also Ewelina to Cc
> > Cc: Ewelina Musial <ewelina.musial at intel.com>
> > 
> > Digression: for repro it could be helpfull to have also git logs
> > and kernel config but this also is one-time info for whole
> > bat/shard run, so again no need to duplicate in every results.
> 
> they are already there
> 
> IGT-Version: 1.30-gfafef52e0 (x86_64) (Linux: 6.13.0-xe+ x86_64)
> 		   ^^^^^^^^^

IGT is there but not the kernel :( But that is for another discussion.

Regards,
Kamil

> 
> The kernel one is not since we stopped appending the version,
> but it would be good to bring it back.
> 
> Lucas De Marchi
> 
> > 
> > Regards,
> > Kamil
> > 
> > > 
> > > Tested-by: Peter Senna Tschudin <peter.senna at linux.intel.com>
> > > Reviewed-by: Peter Senna Tschudin <peter.senna at linux.intel.com>
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> > > ---
> > ...cut...
> > 
> > > index a2b79da9e..2c483fdf5 100644
> > > --- a/runner/json_tests_data/warnings/reference.json
> > > +++ b/runner/json_tests_data/warnings/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/resultgen.c b/runner/resultgen.c
> > > index 87847bf5b..0d3a569cf 100644
> > > --- a/runner/resultgen.c
> > > +++ b/runner/resultgen.c
> > > @@ -2281,7 +2281,7 @@ struct json_object *generate_results_json(int dirfd)
> > >  {
> > >  	struct settings settings;
> > >  	struct job_list job_list;
> > > -	struct json_object *obj, *elapsed;
> > > +	struct json_object *obj, *elapsed, *arr;
> > >  	struct results results;
> > >  	int testdirfd, fd;
> > >  	size_t i;
> > > @@ -2319,6 +2319,11 @@ struct json_object *generate_results_json(int dirfd)
> > >  		close(fd);
> > >  	}
> > > 
> > > +	arr = json_object_new_array();
> > > +	for (i = 0; i < settings.cmdline.argc; i++)
> > > +		json_object_array_add(arr, json_object_new_string(settings.cmdline.argv[i]));
> > > +	json_object_object_add(obj, "cmdline", arr);
> > > +
> > >  	elapsed = json_object_new_object();
> > >  	json_object_object_add(elapsed, "__type__", json_object_new_string("TimeAttribute"));
> > >  	if ((fd = openat(dirfd, "starttime.txt", O_RDONLY)) >= 0) {
> > > --
> > > 2.48.0
> > >