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

[Thomas Huth <thuth@redhat.com>]

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?

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

 Thomas