Re: [PATCH 2/2] test: For missing static checkers, skip rather than failing tests

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4606 bytes --]

On Thu, Oct 09, 2025 at 09:29:02PM +0200, Stefano Brivio wrote:
> On Thu,  9 Oct 2025 14:43:58 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > We run a bunch of static checkers as part of our testsuite.  That's useful,
> > but it means that if a user doesn't have one of them installed, it fails
> > the entire testsuite.  Alter our scripts to skip the test, rather than
> > failing outright if the checker tool is not installed.
> > 
> > This requires exeter v0.4.4 or later.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  test/build/static_checkers.sh | 17 +++++++++++++----
> >  test/lib/exeter               | 10 +++++++++-
> >  2 files changed, 22 insertions(+), 5 deletions(-)
> > 
> > diff --git a/test/build/static_checkers.sh b/test/build/static_checkers.sh
> > index 228b99ae..caccd183 100755
> > --- a/test/build/static_checkers.sh
> > +++ b/test/build/static_checkers.sh
> > @@ -15,16 +15,25 @@
> >  
> >  . $(dirname $0)/../exeter/sh/exeter.sh
> >  
> > -exeter_register cppcheck make -C .. cppcheck
> > +do_check() {
> 
> Nit: everywhere else in the test suite, we have kerneldoc-style
> comments (see lib/test).

Good point, added.

> > +	checker="$1"
> > +	shift
> > +	if ! which "${checker}"; then
> 
> I think we should hide standard output here, just like we do for the
> *tools directives in test_one_line(), because if the check fails, it's
> a failure we already handled.

Good idea.  I hid both stdout - it just generates noise on success -
and stderr - the error is already handled on failure.

> 
> > +		exeter_skip "${checker} not available"
> > +	fi
> > +	make "${@}" "${checker}"
> > +}
> > +
> > +exeter_register cppcheck do_check cppcheck -C ..
> >  exeter_set_description cppcheck "passt sources pass cppcheck"
> >  
> > -exeter_register clang_tidy make -C .. clang-tidy
> > +exeter_register clang_tidy do_check clang-tody -C ..
> >  exeter_set_description clang_tidy "passt sources pass clang-tidy"
> >  
> > -exeter_register flake8 make flake8
> > +exeter_register flake8 do_check flake8
> >  exeter_set_description flake8 "passt tests in Python pass flake8"
> >  
> > -exeter_register mypy make mypy
> > +exeter_register mypy do_check mypy
> >  exeter_set_description mypy "passt tests in Python pass mypy --strict"
> >  
> >  exeter_main "$@"
> > diff --git a/test/lib/exeter b/test/lib/exeter
> > index 530c6909..6ed92a16 100644
> > --- a/test/lib/exeter
> > +++ b/test/lib/exeter
> > @@ -49,7 +49,15 @@ exeter() {
> >  	for __testid in $($EXETOOL list -- "$@"); do
> >  		__desc="$($EXETOOL desc -- "$@" -- "${__testid}")"
> >  		status_test_start "${__desc}"
> > -		context_run host "$* '${__testid}'" && status_test_ok || status_test_fail
> > +		status=0
> > +		context_run host "$* '${__testid}'" || status="$?"
> 
> It would be nice to keep all variable names enclosed in curly brackets,
> for consistency, but also to avoid surprises.

Done.

> > +		if [ "$status" = 0 ]; then
> > +			status_test_ok
> > +		elif [ "$status" = 77 ]; then
> 
> I couldn't quite find out where 77 comes from. It's not specified in
> POSIX.1-2024 2.8.2 "Exit Status for Commands":
> 
>   https://pubs.opengroup.org/onlinepubs/9799919799/utilities/V3_chap02.html#tag_19_08_02
> 
> and my version of which(1) just returns 1. Is this something from Bash?

No, it's exeter_skip that generates code 77.

That choice is made on what seems to be a loose convention amongst
some existing test tools.  Meson uses it, for example:

https://mesonbuild.com/Unit-tests.html#skipped-tests-and-hard-errors

I believe they got it from some GNU tools, like automake:

https://www.gnu.org/software/automake/manual/automake.html#Scripts_002dbased-Testsuites-1

Based on those examples, I wrote it into the exeter protocol.

https://gitlab.com/dgibson/exeter/-/blob/main/PROTOCOL.md?ref_type=heads#exit-codes

BATS and Avocado recognize it by default.  For BATS, I handled that by
putting explicit handling in the .bats files generated by exetool.  I
didn't originally handle Avocado, but your question prompted me to
figure it out, and I just committed a change which handles it (by
configuring Avocado via the job file to recognize this code to mean
skip).

In any case, it's not perfect, but seemed as good a convention as any
to pick.

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]