> On Apr 27, 2022, at 3:38 PM, Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Tue, Apr 26, 2022 at 9:14 PM Mykola Lysenko <mykolal@xxxxxx> wrote:
>>
>> Implement per subtest log collection for both parallel
>> and sequential test execution. This allows granular
>> per-subtest error output in the 'All error logs' section.
>> Add subtest log transfer into the protocol during the
>> parallel test execution.
>>
>> Move all test log printing logic into dump_test_log
>> function. One exception is the output of test names when
>> verbose printing is enabled. Move test name/result
>> printing into separate functions to avoid repetition.
>>
>> Print all successful subtest results in the log. Print
>> only failed test logs when test does not have subtests.
>> Or only failed subtests' logs when test has subtests.
>>
>> Disable 'All error logs' output when verbose mode is
>> enabled. This functionality was already broken and is
>> causing confusion.
>>
>> Signed-off-by: Mykola Lysenko <mykolal@xxxxxx>
>> ---
>
> Works great! I've dropped the before test/subtest duplicated output of
> test/subtest name, as it seems unnecessary in practice. I dropped few
> lines of code that do that locally, as you suggested.
>
> I also noticed a small memory leak, see comment below, please send a follow up.
Thanks a lot Andrii for the review!
I have sent a follow-up patch
>
>> tools/testing/selftests/bpf/test_progs.c | 640 +++++++++++++++++------
>> tools/testing/selftests/bpf/test_progs.h | 35 +-
>> 2 files changed, 499 insertions(+), 176 deletions(-)
>>
>
> [...]
>
>> +
>> +static int dispatch_thread_read_log(int sock_fd, char **log_buf, size_t *log_cnt)
>> +{
>> + FILE *log_fp = NULL;
>> +
>> + log_fp = open_memstream(log_buf, log_cnt);
>> + if (!log_fp)
>> + return 1;
>> +
>> + while (true) {
>> + struct msg msg;
>> +
>> + if (read_prog_test_msg(sock_fd, &msg, MSG_TEST_LOG))
>
> leaking log_fp here?
>
>> + return 1;
>> +
>> + fprintf(log_fp, "%s", msg.test_log.log_buf);
>> + if (msg.test_log.is_last)
>> + break;
>> + }
>> + fclose(log_fp);
>> + log_fp = NULL;
>> + return 0;
>> +}
>> +
>
> [...]
