Re: [Qemu-devel] [PATCH v3] tests/qemu-iotests: re-format output to for make check-block

["Alex Bennée" <alex.bennee@linaro.org>]

Thomas Huth <thuth@redhat.com> writes:

> On 10/05/2019 12.29, Alex Bennée wrote:
>> This attempts to clean-up the output to better match the output of the
>> rest of the QEMU check system when called with -pretty. This includes:
>>
>>   - formatting as "  TEST    iotest: nnn"
>>   - calculating time diff at the end
>>   - only dumping config on failure (when -pretty enabled)
>>
>> The existing output is mostly preserved although the dumping of the
>> old time at the start "Ns ..." was removed to keep the logic simple.
>> The timestamp mode can still be used to see which tests are "hanging".
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Message-Id: <20190503143904.31211-1-alex.bennee@linaro.org>
>>
>> ---
>> v3
>>   - revert echo to printf
>>   - add _report_test_start
>> ---
>>  tests/qemu-iotests/check | 101 ++++++++++++++++++++++++++-------------
>>  1 file changed, 68 insertions(+), 33 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
>> index 922c5d1d3d3..ac481f905bf 100755
>> --- a/tests/qemu-iotests/check
>> +++ b/tests/qemu-iotests/check
>> @@ -27,6 +27,7 @@ bad=""
>>  notrun=""
>>  casenotrun=""
>>  interrupt=true
>> +pretty=false
>>
>>  # by default don't output timestamps
>>  timestamp=${TIMESTAMP:=false}
>> @@ -88,6 +89,22 @@ _full_platform_details()
>>      echo "$os/$platform $host $kernel"
>>  }
>>
>> +_full_env_details()
>> +{
>> +    cat <<EOF
>> +QEMU          -- "$QEMU_PROG" $QEMU_OPTIONS
>> +QEMU_IMG      -- "$QEMU_IMG_PROG" $QEMU_IMG_OPTIONS
>> +QEMU_IO       -- "$QEMU_IO_PROG" $QEMU_IO_OPTIONS
>> +QEMU_NBD      -- "$QEMU_NBD_PROG" $QEMU_NBD_OPTIONS
>> +IMGFMT        -- $FULL_IMGFMT_DETAILS
>> +IMGPROTO      -- $IMGPROTO
>> +PLATFORM      -- $FULL_HOST_DETAILS
>> +TEST_DIR      -- $TEST_DIR
>> +SOCKET_SCM_HELPER -- $SOCKET_SCM_HELPER
>> +
>> +EOF
>> +}
>> +
>>  # $1 = prog to look for
>>  set_prog_path()
>>  {
>> @@ -256,6 +273,7 @@ other options
>>      -o options          -o options to pass to qemu-img create/convert
>>      -T                  output timestamps
>>      -c mode             cache mode
>> +    -pretty             pretty print output for make check
>
> "pretty" is likely just a matter of taste ... so maybe this should be
> named differently instead? "--makecheck" ? Or "--one-shot" ?
>
>>  testlist options
>>      -g group[,group...]        include tests from these groups
>> @@ -403,7 +421,10 @@ testlist options
>>                  command -v xxdiff >/dev/null 2>&1 && diff=xxdiff
>>              fi
>>              ;;
>> -
>> +        -pretty)   # pretty print output
>> +            pretty=true
>> +            xpand=false
>> +            ;;
>>          -n)        # show me, don't do it
>>              showme=true
>>              xpand=false
>> @@ -704,23 +725,30 @@ END        { if (NR > 0) {
>>
>>  trap "_wrapup; exit \$status" 0 1 2 3 15
>>
>> +# Report the test start and results, optionally pretty printing for make
>> +# args: $seq
>> +_report_test_start()
>> +{
>> +    if $pretty; then
>> +        printf "  TEST    iotest: %s" "$1"
>
> Could you maybe change the "iotest:" into "iotest-$IMGFMT:" ? ... so
> that when you run "make check SPEED=slow" you also see which kind of
> format is currently under test?

Sure I can do that.

>
> And this currently also does not play very nicely when running "make -j8
> check" in parallel:
>
>   [...]
>   TEST    iotest: 001  TEST    check-qtest-alpha: tests/qmp-test
>   TEST    check-qtest-alpha: tests/qmp-cmd-test
>   TEST    check-qtest-aarch64: tests/boot-serial-test
>   TEST    check-qtest-aarch64: tests/migration-test
>   TEST    check-qtest-arm: tests/tmp105-test
>   TEST    check-unit: tests/check-qnum
>   TEST    check-unit: tests/check-qstring
>   TEST    check-unit: tests/check-qlist
>   TEST    check-unit: tests/check-qnull
>  2s (last 2s)
>   TEST    iotest: 002  TEST    check-qtest-arm: tests/pca9552-test
>   TEST    check-unit: tests/check-qobject
>   TEST    check-qtest-cris: tests/qmp-test
>   [...]
>
> I think the "make check" mode should only print out one time for each
> test, preferable at the end, like the other tests (like qtests) are
> doing it...?

*sigh* and this is of course why deferred everything to the end in the
last revision. Should we just assume the -pretty/-make whatever is
incompatible with -T for the timestamp mode?

>
>  Thomas


--
Alex Bennée