* [PATCH 0/2] Fixes to exeter test integration @ 2025-10-09 3:43 David Gibson 2025-10-09 3:43 ` [PATCH 1/2] test: Add some missing quoting in exeter runner David Gibson 2025-10-09 3:43 ` [PATCH 2/2] test: For missing static checkers, skip rather than failing tests David Gibson 0 siblings, 2 replies; 7+ messages in thread From: David Gibson @ 2025-10-09 3:43 UTC (permalink / raw) To: Stefano Brivio, passt-dev; +Cc: 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] test: Add some missing quoting in exeter runner 2025-10-09 3:43 [PATCH 0/2] Fixes to exeter test integration David Gibson @ 2025-10-09 3:43 ` David Gibson 2025-10-09 19:28 ` Stefano Brivio 2025-10-09 3:43 ` [PATCH 2/2] test: For missing static checkers, skip rather than failing tests David Gibson 1 sibling, 1 reply; 7+ messages in thread From: David Gibson @ 2025-10-09 3:43 UTC (permalink / raw) To: Stefano Brivio, passt-dev; +Cc: 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] test: Add some missing quoting in exeter runner 2025-10-09 3:43 ` [PATCH 1/2] test: Add some missing quoting in exeter runner David Gibson @ 2025-10-09 19:28 ` Stefano Brivio 2025-10-10 2:28 ` David Gibson 0 siblings, 1 reply; 7+ messages in thread From: Stefano Brivio @ 2025-10-09 19:28 UTC (permalink / raw) To: David Gibson; +Cc: passt-dev 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] test: Add some missing quoting in exeter runner 2025-10-09 19:28 ` Stefano Brivio @ 2025-10-10 2:28 ` David Gibson 0 siblings, 0 replies; 7+ messages in thread From: David Gibson @ 2025-10-10 2:28 UTC (permalink / raw) To: Stefano Brivio; +Cc: passt-dev [-- 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 --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] test: For missing static checkers, skip rather than failing tests 2025-10-09 3:43 [PATCH 0/2] Fixes to exeter test integration David Gibson 2025-10-09 3:43 ` [PATCH 1/2] test: Add some missing quoting in exeter runner David Gibson @ 2025-10-09 3:43 ` David Gibson 2025-10-09 19:29 ` Stefano Brivio 1 sibling, 1 reply; 7+ messages in thread From: David Gibson @ 2025-10-09 3:43 UTC (permalink / raw) To: Stefano Brivio, passt-dev; +Cc: 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] test: For missing static checkers, skip rather than failing tests 2025-10-09 3:43 ` [PATCH 2/2] test: For missing static checkers, skip rather than failing tests David Gibson @ 2025-10-09 19:29 ` Stefano Brivio 2025-10-10 3:11 ` David Gibson 0 siblings, 1 reply; 7+ messages in thread From: Stefano Brivio @ 2025-10-09 19:29 UTC (permalink / raw) To: David Gibson; +Cc: passt-dev 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] test: For missing static checkers, skip rather than failing tests 2025-10-09 19:29 ` Stefano Brivio @ 2025-10-10 3:11 ` David Gibson 0 siblings, 0 replies; 7+ messages in thread From: David Gibson @ 2025-10-10 3:11 UTC (permalink / raw) To: Stefano Brivio; +Cc: passt-dev [-- 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 --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-10-10 3:40 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-10-09 3:43 [PATCH 0/2] Fixes to exeter test integration David Gibson 2025-10-09 3:43 ` [PATCH 1/2] test: Add some missing quoting in exeter runner David Gibson 2025-10-09 19:28 ` Stefano Brivio 2025-10-10 2:28 ` David Gibson 2025-10-09 3:43 ` [PATCH 2/2] test: For missing static checkers, skip rather than failing tests David Gibson 2025-10-09 19:29 ` Stefano Brivio 2025-10-10 3:11 ` David Gibson
Code repositories for project(s) associated with this public inbox https://passt.top/passt This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for IMAP folder(s).