[PATCH 0/2] Fixes to exeter test integration

[PATCH 0/2] Fixes to exeter test integration

[David Gibson]

In advance of the tunbridge work, here are some further small fixes to
the test logic, specifically how we interface with exeter tests.

David Gibson (2):
  test: Add some missing quoting in exeter runner
  test: For missing static checkers, skip rather than failing tests

 test/build/static_checkers.sh | 17 +++++++++++++----
 test/lib/exeter               | 12 ++++++++++--
 2 files changed, 23 insertions(+), 6 deletions(-)

-- 
2.51.0

[PATCH 1/2] test: Add some missing quoting in exeter runner

[David Gibson]

exeter() quoted ${__testid}, but in some places we use it there's an
extra level of shell, which needs another layer of quoting.  This breaks
if testids include ';', which is quite common in exeter tests created as
a composition/pipeline of two functions.  Add the required extra quoting.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 test/lib/exeter | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/lib/exeter b/test/lib/exeter
index 3b19beaa..530c6909 100644
--- a/test/lib/exeter
+++ b/test/lib/exeter
@@ -47,9 +47,9 @@ exeter() {
 	[ ${CI} -eq 1 ] && video_link "${1}"
 
 	for __testid in $($EXETOOL list -- "$@"); do
-		__desc="$($EXETOOL desc -- "$@" -- ${__testid})"
+		__desc="$($EXETOOL desc -- "$@" -- "${__testid}")"
 		status_test_start "${__desc}"
-		context_run host "$@" "${__testid}" && status_test_ok || status_test_fail
+		context_run host "$* '${__testid}'" && status_test_ok || status_test_fail
 	done
 
 	cd ..
-- 
2.51.0

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

[David Gibson]

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() {
+	checker="$1"
+	shift
+	if ! which "${checker}"; then
+		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="$?"
+		if [ "$status" = 0 ]; then
+			status_test_ok
+		elif [ "$status" = 77 ]; then
+			status_test_skip
+		else
+			status_test_fail
+		fi
 	done
 
 	cd ..
-- 
2.51.0

Re: [PATCH 1/2] test: Add some missing quoting in exeter runner

[Stefano Brivio]

On Thu,  9 Oct 2025 14:43:57 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> exeter() quoted ${__testid}, but in some places we use it there's an
> extra level of shell, which needs another layer of quoting.  This breaks
> if testids include ';', which is quite common in exeter tests created as
> a composition/pipeline of two functions.  Add the required extra quoting.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  test/lib/exeter | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/test/lib/exeter b/test/lib/exeter
> index 3b19beaa..530c6909 100644
> --- a/test/lib/exeter
> +++ b/test/lib/exeter
> @@ -47,9 +47,9 @@ exeter() {
>  	[ ${CI} -eq 1 ] && video_link "${1}"
>  
>  	for __testid in $($EXETOOL list -- "$@"); do
> -		__desc="$($EXETOOL desc -- "$@" -- ${__testid})"
> +		__desc="$($EXETOOL desc -- "$@" -- "${__testid}")"
>  		status_test_start "${__desc}"
> -		context_run host "$@" "${__testid}" && status_test_ok || status_test_fail
> +		context_run host "$* '${__testid}'" && status_test_ok || status_test_fail

Nit: for consistency, given that @ and * are variable names, I would
enclose them in curly brackets as well, while at it.

-- 
Stefano

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

[Stefano Brivio]

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

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

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

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

> +			status_test_skip
> +		else
> +			status_test_fail
> +		fi
>  	done
>  
>  	cd ..

-- 
Stefano

Re: [PATCH 1/2] test: Add some missing quoting in exeter runner

[David Gibson]

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

On Thu, Oct 09, 2025 at 09:28:56PM +0200, Stefano Brivio wrote:
> On Thu,  9 Oct 2025 14:43:57 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > exeter() quoted ${__testid}, but in some places we use it there's an
> > extra level of shell, which needs another layer of quoting.  This breaks
> > if testids include ';', which is quite common in exeter tests created as
> > a composition/pipeline of two functions.  Add the required extra quoting.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  test/lib/exeter | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/test/lib/exeter b/test/lib/exeter
> > index 3b19beaa..530c6909 100644
> > --- a/test/lib/exeter
> > +++ b/test/lib/exeter
> > @@ -47,9 +47,9 @@ exeter() {
> >  	[ ${CI} -eq 1 ] && video_link "${1}"
> >  
> >  	for __testid in $($EXETOOL list -- "$@"); do
> > -		__desc="$($EXETOOL desc -- "$@" -- ${__testid})"
> > +		__desc="$($EXETOOL desc -- "$@" -- "${__testid}")"
> >  		status_test_start "${__desc}"
> > -		context_run host "$@" "${__testid}" && status_test_ok || status_test_fail
> > +		context_run host "$* '${__testid}'" && status_test_ok || status_test_fail
> 
> Nit: for consistency, given that @ and * are variable names, I would
> enclose them in curly brackets as well, while at it.

Good point, done.

-- 
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 --]

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

[David Gibson]

[-- 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 --]