[PATCH] tests/decode: Emit TAP

[PATCH] tests/decode: Emit TAP

[Richard Henderson]

We currently print FAIL for the failure of a succ_* test, but don't
return a failure exit code.  Instead, convert the script to emit
Test Anything Protocol, which gives visibility into each subtest
as well as not relying on exit codes.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tests/decode/check.sh | 36 ++++++++++++++++++++++++++----------
 tests/meson.build     |  1 +
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/tests/decode/check.sh b/tests/decode/check.sh
index 95445a0115..a3d879a099 100755
--- a/tests/decode/check.sh
+++ b/tests/decode/check.sh
@@ -4,21 +4,37 @@
 
 PYTHON=$1
 DECODETREE=$2
-E=0
+E_FILES=`echo err_*.decode`
+S_FILES=`echo succ_*.decode`
 
-# All of these tests should produce errors
-for i in err_*.decode; do
+j=0
+for i in $E_FILES $S_FILES; do
+    j=`expr $j + 1`
+done
+
+echo 1..$j
+
+j=0
+for i in $E_FILES; do
+    j=`expr $j + 1`
+    n=`basename $i .decode`
     if $PYTHON $DECODETREE $i > /dev/null 2> /dev/null; then
-        # Pass, aka failed to fail.
-        echo FAIL: $i 1>&2
-        E=1
+        # Failed to fail.
+        echo not ok $j $n
+    else
+        echo ok $j $n
     fi
 done
 
-for i in succ_*.decode; do
-    if ! $PYTHON $DECODETREE $i > /dev/null 2> /dev/null; then
-        echo FAIL:$i 1>&2
+for i in $S_FILES; do
+    j=`expr $j + 1`
+    n=`basename $i .decode`
+    if $PYTHON $DECODETREE $i > /dev/null 2> /dev/null; then
+        # Succeeded.
+        echo ok $j $n
+    else
+        echo not ok $j $n
     fi
 done
 
-exit $E
+exit 0
diff --git a/tests/meson.build b/tests/meson.build
index 8e318ec513..137ef85ab6 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -77,6 +77,7 @@ endif
 test('decodetree', sh,
      args: [ files('decode/check.sh'), config_host['PYTHON'], files('../scripts/decodetree.py') ],
      workdir: meson.current_source_dir() / 'decode',
+     protocol: 'tap', verbose: true,
      suite: 'decodetree')
 
 if 'CONFIG_TCG' in config_all
-- 
2.34.1

Re: [PATCH] tests/decode: Emit TAP

[Peter Maydell]

On Wed, 24 May 2023 at 18:38, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We currently print FAIL for the failure of a succ_* test, but don't
> return a failure exit code.  Instead, convert the script to emit
> Test Anything Protocol, which gives visibility into each subtest
> as well as not relying on exit codes.
>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tests/decode/check.sh | 36 ++++++++++++++++++++++++++----------
>  tests/meson.build     |  1 +
>  2 files changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/tests/decode/check.sh b/tests/decode/check.sh
> index 95445a0115..a3d879a099 100755
> --- a/tests/decode/check.sh
> +++ b/tests/decode/check.sh
> @@ -4,21 +4,37 @@
>
>  PYTHON=$1
>  DECODETREE=$2
> -E=0
> +E_FILES=`echo err_*.decode`
> +S_FILES=`echo succ_*.decode`

If you run shellcheck on this script it produces some
style complaints. Notably:

 * $(...) is better than `...`
 * j=$(($j + 1))  is better than j=`expr $j + 1`

At least some of its "missing quoting" complaints are
also legitimate, notably on $PYTHON and $DECODETREE.

thanks
-- PMM

Re: [PATCH] tests/decode: Emit TAP

[Richard Henderson]

On 5/25/23 03:00, Peter Maydell wrote:
> On Wed, 24 May 2023 at 18:38, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> We currently print FAIL for the failure of a succ_* test, but don't
>> return a failure exit code.  Instead, convert the script to emit
>> Test Anything Protocol, which gives visibility into each subtest
>> as well as not relying on exit codes.
>>
>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   tests/decode/check.sh | 36 ++++++++++++++++++++++++++----------
>>   tests/meson.build     |  1 +
>>   2 files changed, 27 insertions(+), 10 deletions(-)
>>
>> diff --git a/tests/decode/check.sh b/tests/decode/check.sh
>> index 95445a0115..a3d879a099 100755
>> --- a/tests/decode/check.sh
>> +++ b/tests/decode/check.sh
>> @@ -4,21 +4,37 @@
>>
>>   PYTHON=$1
>>   DECODETREE=$2
>> -E=0
>> +E_FILES=`echo err_*.decode`
>> +S_FILES=`echo succ_*.decode`
> 
> If you run shellcheck on this script it produces some
> style complaints. Notably:
> 
>   * $(...) is better than `...`
>   * j=$(($j + 1))  is better than j=`expr $j + 1`
> 
> At least some of its "missing quoting" complaints are
> also legitimate, notably on $PYTHON and $DECODETREE.

"Better" in what sense?  Also, this is /bin/sh, not /bin/bash, so I'm never certain what 
I'm really allowed to use.

I was already half tempted to convert the script to python anyway...


r~

Re: [PATCH] tests/decode: Emit TAP

[Peter Maydell]

On Thu, 25 May 2023 at 14:11, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 5/25/23 03:00, Peter Maydell wrote:
> > On Wed, 24 May 2023 at 18:38, Richard Henderson
> > <richard.henderson@linaro.org> wrote:
> >>
> >> We currently print FAIL for the failure of a succ_* test, but don't
> >> return a failure exit code.  Instead, convert the script to emit
> >> Test Anything Protocol, which gives visibility into each subtest
> >> as well as not relying on exit codes.
> >>
> >> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >> ---
> >>   tests/decode/check.sh | 36 ++++++++++++++++++++++++++----------
> >>   tests/meson.build     |  1 +
> >>   2 files changed, 27 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/tests/decode/check.sh b/tests/decode/check.sh
> >> index 95445a0115..a3d879a099 100755
> >> --- a/tests/decode/check.sh
> >> +++ b/tests/decode/check.sh
> >> @@ -4,21 +4,37 @@
> >>
> >>   PYTHON=$1
> >>   DECODETREE=$2
> >> -E=0
> >> +E_FILES=`echo err_*.decode`
> >> +S_FILES=`echo succ_*.decode`
> >
> > If you run shellcheck on this script it produces some
> > style complaints. Notably:
> >
> >   * $(...) is better than `...`
> >   * j=$(($j + 1))  is better than j=`expr $j + 1`
> >
> > At least some of its "missing quoting" complaints are
> > also legitimate, notably on $PYTHON and $DECODETREE.
>
> "Better" in what sense?  Also, this is /bin/sh, not /bin/bash, so I'm never certain what
> I'm really allowed to use.

checkpatch checks POSIX syntax if the script starts with #!/bin/sh.
(It's a pretty good tool for spotting "this thing you used
isn't actually POSIX", in fact.)

shellcheck's rationales are
https://www.shellcheck.net/wiki/SC2003 (for expr)
 -- the POSIX spec itself says "avoid expr in new scripts".
    (Also I think shell builtin arithmetic should be more efficient
    than spawning the expr binary)
https://www.shellcheck.net/wiki/SC2006 (for backticks)
 -- backticks have some awkward issues; for consistency
    I think it's better to use $() everywhere even in
    the kind of simple case where `` has no problems

configure doesn't have any backticks in it.

thanks
-- PMM

Re: [PATCH] tests/decode: Emit TAP

[Paolo Bonzini]

On 5/25/23 12:00, Peter Maydell wrote:
> At least some of its "missing quoting" complaints are
> also legitimate, notably on $PYTHON and $DECODETREE.

$PYTHON could include command line options, so I don't think it should 
be quoted.  Though we could stop insisting on using -B; either not doing 
anything at all, or adding

export PYTHONDONTWRITEBYTECODE=1

to Makefile.

Paolo