| Message ID | 20230524173815.1148653-1-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | tests/decode: Emit TAP | expand |
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
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~
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
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
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
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(-)