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

[Lucas De Marchi]

On Fri, Jan 31, 2025 at 01:16:19PM +0100, Kamil Konieczny wrote:
>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>

thanks

>
>> >
>> > 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.

yeah... CI used to build the kernel with a LOCALVERSION env var set to
"-g${short_hash}", which simulates what's done with CONFIG_LOCALVERSION_AUTO

Ryszard, can we bring a similar behavior back? I think the main reason
to avoid CONFIG_LOCALVERSION_AUTO in xe/ci repository was that for
local/dev builds, it's an annoyance when you are updating the module:
you don't want to keep changing the commit hash since then module will
fail to load. However, for using in a CI environment I think it makes
sense. Thoughts?

Lucas De Marchi

>
>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
>> > >