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

2019-05-09 Thread Thomas Huth
On 09/05/2019 18.59, Alex Bennée wrote:
> This attempts to clean-up the output to better match the output of the
> rest of the QEMU check system. This includes:
> 
>   - formatting as "  TESTiotest: nnn"
>   - calculating time diff at the end
>   - only dumping config on failure
> 
> Signed-off-by: Alex Bennée 
> Message-Id: <20190503143904.31211-1-alex.ben...@linaro.org>
> ---
>  tests/qemu-iotests/check | 101 +++
>  1 file changed, 61 insertions(+), 40 deletions(-)
[...]
> -cat < -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
> +if ! $pretty; then
> +_full_env_details
> +fi
>  
>  seq="check"
[...]
> @@ -852,9 +869,13 @@ do
>  #
>  if $err
>  then
> +_report_test_result $seq "FAILED $reason"
> +_full_env_details

I'd suggest to wrap the _full_env_details with a "if $pretty" statement,
otherwise the env will be printed out twice, one time at the beginning,
and one time in case of an error.

>  bad="$bad $seq"
>  n_bad=$(expr $n_bad + 1)
>  quick=false
> +else
> +_report_test_result $seq "$results"
>  fi
>  [ -f $seq.notrun ] || try=$(expr $try + 1)
>  
> 

 Thomas



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

2019-05-09 Thread Eric Blake
On 5/9/19 3:38 PM, Alex Bennée wrote:

>> Hm, this makes every iotest print two lines:
>>
>> $ ./check -T -qcow2
>> [...]
>> 001 [20:06:27] -> [20:06:27]
>> 001 0s (last 1s)
> 
> Yes - it was a compromise to ensure we printed a start and end
> timestamp but I guess we can fix it up with a bit more shell ugliness:
> 
> --8<---cut here---start->8---
> 
> Subject: [PATCH] fixup! tests/qemu-iotests: re-format output to for make
>  check-block
> 
> ---
>  tests/qemu-iotests/check | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index fb239967a32..9f083f06b46 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -726,7 +726,11 @@ _report_test_result()
>  if $pretty; then
>  echo "  TESTiotest: $1 $2"
>  else
> -echo "$1 $2"
> +if $timestamp; then
> +echo " $2"
> +else
> +echo "$1 $2"
> +fi

Why not just make $1 '' in the case where you've already printed a
timestamp, so that this code is still unconditionally echo "$1 $2"?

>  fi
>  }
> 
> @@ -793,7 +797,7 @@ do
>  $run_command >$tmp.out 2>&1)
>  fi
>  sts=$?
> -$timestamp && echo " [$(date "+%T")]"
> +$timestamp && echo -n " [$(date "+%T")]"

'echo -n' is not portable (even in bash, since you can compile a
different default for shopt xpg_echo). Better is to use 'printf %s', as
is already done in _timestamp().

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature


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

2019-05-09 Thread Alex Bennée


Max Reitz  writes:

> On 09.05.19 18:59, Alex Bennée wrote:
>> This attempts to clean-up the output to better match the output of the
>> rest of the QEMU check system. This includes:
>>
>>   - formatting as "  TESTiotest: nnn"
>>   - calculating time diff at the end
>>   - only dumping config on failure
>>
>> Signed-off-by: Alex Bennée 
>> Message-Id: <20190503143904.31211-1-alex.ben...@linaro.org>
>> ---
>>  tests/qemu-iotests/check | 101 +++
>>  1 file changed, 61 insertions(+), 40 deletions(-)
>
> Hm, this makes every iotest print two lines:
>
> $ ./check -T -qcow2
> [...]
> 001 [20:06:27] -> [20:06:27]
> 001 0s (last 1s)

Yes - it was a compromise to ensure we printed a start and end
timestamp but I guess we can fix it up with a bit more shell ugliness:

--8<---cut here---start->8---

Subject: [PATCH] fixup! tests/qemu-iotests: re-format output to for make
 check-block

---
 tests/qemu-iotests/check | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index fb239967a32..9f083f06b46 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -726,7 +726,11 @@ _report_test_result()
 if $pretty; then
 echo "  TESTiotest: $1 $2"
 else
-echo "$1 $2"
+if $timestamp; then
+echo " $2"
+else
+echo "$1 $2"
+fi
 fi
 }

@@ -793,7 +797,7 @@ do
 $run_command >$tmp.out 2>&1)
 fi
 sts=$?
-$timestamp && echo " [$(date "+%T")]"
+$timestamp && echo -n " [$(date "+%T")]"
 stop=$(_wallclock)

 if [ -f core ]
--8<---cut here---end--->8---

--
Alex Bennée



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

2019-05-09 Thread Max Reitz
On 09.05.19 18:59, Alex Bennée wrote:
> This attempts to clean-up the output to better match the output of the
> rest of the QEMU check system. This includes:
> 
>   - formatting as "  TESTiotest: nnn"
>   - calculating time diff at the end
>   - only dumping config on failure
> 
> Signed-off-by: Alex Bennée 
> Message-Id: <20190503143904.31211-1-alex.ben...@linaro.org>
> ---
>  tests/qemu-iotests/check | 101 +++
>  1 file changed, 61 insertions(+), 40 deletions(-)

Hm, this makes every iotest print two lines:

$ ./check -T -qcow2
[...]
001 [20:06:27] -> [20:06:27]
001 0s (last 1s)
002 [20:06:27] -> [20:06:28]
002 1s (last 1s)
003 [20:06:28] -> [20:06:29]
003 1s (last 1s)
004 [20:06:29] -> [20:06:29]
004 0s (last 0s)
005 [20:06:29] -> [20:06:29]
005 0s (last 0s)
[..]

Which looks rather weird to me.

Max



signature.asc
Description: OpenPGP digital signature