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

[Lucas De Marchi]

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.

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

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