On Tue, Aug 19, 2025 at 04:27:38PM +0200, Stefano Brivio wrote:

On Thu, 7 Aug 2025 21:32:35 +1000 David Gibson wrote:

...

Introduce some trivial testcases based on the exeter library. These run passt and pasta with --help and --version options. Extend our test scripts to run these tests.

Signed-off-by: David Gibson --- test/.gitignore | 1 + test/Makefile | 5 ++++- test/lib/exeter | 46 +++++++++++++++++++++++++++++++++++++++++++++ test/run | 9 ++++++++- test/smoke/smoke.sh | 27 ++++++++++++++++++++++++++ 5 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 test/lib/exeter create mode 100755 test/smoke/smoke.sh

diff --git a/test/.gitignore b/test/.gitignore index 3573444f..cf48b885 100644 --- a/test/.gitignore +++ b/test/.gitignore @@ -11,3 +11,4 @@ nstool rampstream guest-key guest-key.pub +/exeter/ diff --git a/test/Makefile b/test/Makefile index bf63db87..332f3f3e 100644 --- a/test/Makefile +++ b/test/Makefile @@ -50,7 +50,7 @@ UBUNTU_NEW_IMGS = xenial-server-cloudimg-powerpc-disk1.img \ jammy-server-cloudimg-s390x.img UBUNTU_IMGS = $(UBUNTU_OLD_IMGS) $(UBUNTU_NEW_IMGS)

-DOWNLOAD_ASSETS = mbuto podman \ +DOWNLOAD_ASSETS = exeter mbuto podman \ $(DEBIAN_IMGS) $(FEDORA_IMGS) $(OPENSUSE_IMGS) $(UBUNTU_IMGS)

Pre-existing, but this broke a while after you switched the image preparation to Makefile targets, so you can't really expect people to issue 'make' at the moment.

Sorry, I'm not quite sure what the problem is you're getting at.

I ran 'make exeter' to get further. It would be nice to update test/README.md (or fix the Makefile... or both).

Hrm.. "make check" already depends on "make assets" which should (now) include exeter. I'm not sure why that's not working for you.

...

TESTDATA_ASSETS = small.bin big.bin medium.bin \ rampstream @@ -70,6 +70,9 @@ assets: $(ASSETS) pull-%: % git -C $* pull

+exeter: + git clone https://gitlab.com/dgibson/exeter.git + mbuto: git clone git://mbuto.sh/mbuto

diff --git a/test/lib/exeter b/test/lib/exeter new file mode 100644 index 00000000..4f7bcff9 --- /dev/null +++ b/test/lib/exeter @@ -0,0 +1,46 @@ +#!/bin/sh +# +# SPDX-License-Identifier: GPL-2.0-or-later +# +# PASST - Plug A Simple Socket Transport +# for qemu/UNIX domain socket mode +# +# PASTA - Pack A Subtle Tap Abstraction +# for network namespace/tap device mode +# +# test/lib/exeter - Run exeter tests within the rest of passt's tests +# +# Copyright Red Hat +# Author: David Gibson + +exeter() {

It would be nice to have the usual arguments / function description here.

Good point, added.

...

+ STATESETUP="${STATEBASE}/$1" + mkdir -p "${STATESETUP}" + + context_setup_host host + layout_host

Indentation here was done with tabs, this adds spaces instead.

Oops. For some reason I'd added emacs configuration to explicitly *disable* tab indent mode for shell mode in the passt tree. Weird, and now reversed.

...

+ + cd test + + __ntests=$("$@" --list | wc -l) + if [ $? != 0 ]; then + info "Failed to get exeter manifest for $@" + pause_continue \ + "Press any key to pause test session" \ + "Resuming in " \ + "Paused, press any key to continue" \ + 5 + return + fi + + status_file_start "Exeter tests: $*" ${__ntests} + + for __testid in $("$@" --list); do + status_test_start "${__testid}" + context_run host "$@" "${__testid}" && status_test_ok || status_test_fail + done + + cd .. + + teardown_context_watch ${PANE_HOST} host +} diff --git a/test/run b/test/run index f73c3119..9e183682 100755 --- a/test/run +++ b/test/run @@ -53,6 +53,7 @@ COMMIT="$(git log --oneline --no-decorate -1)" . lib/layout_ugly . lib/test . lib/video +. lib/exeter

# cleanup() - Remove temporary files cleanup() { @@ -67,6 +68,8 @@ run() { perf_init [ ${CI} -eq 1 ] && video_start ci

+ exeter smoke/smoke.sh + setup build test build/all test build/cppcheck @@ -223,6 +226,10 @@ run_selected() {

__setup= for __test; do + if "test/${__test}" --list; then + exeter "${__test}" + continue + fi # HACK: the migrate tests need the setup repeated for # each test if [ "${__test%%/*}" != "${__setup}" -o \ @@ -234,7 +241,7 @@ run_selected() {

test "${__test}" done - teardown "${__setup}" + [ -n "${__setup}" ] && teardown "${__setup}"

log "PASS: ${STATUS_PASS}, FAIL: ${STATUS_FAIL}, SKIPPED: ${STATUS_SKIPPED}"

diff --git a/test/smoke/smoke.sh b/test/smoke/smoke.sh new file mode 100755 index 00000000..26a98d31 --- /dev/null +++ b/test/smoke/smoke.sh @@ -0,0 +1,27 @@ +#! /bin/sh

...this is desirable, I think (and, for consistency, I'd use "#!/bin/sh") as Bash is not universally available, but:

So.. I believe "#! " (with the space) is actually the traditional / correct version. I did once encounter a situation where it mattered, although to be fair, that might have been with 25 year old SunOS or something equally obscure. Nonetheless I've habitually included the space ever since (see find-arm64-firmware.sh, prepare-distro-img.sh and rapstream-check.sh).

...

+# +# SPDX-License-Identifier: GPL-2.0-or-later +# +# PASST - Plug A Simple Socket Transport +# for qemu/UNIX domain socket mode +# +# PASTA - Pack A Subtle Tap Abstraction +# for network namespace/tap device mode +# +# test/smoke/smoke.sh - Basic snoke tests

smoke

Lol, oops.

...

+# +# Copyright Red Hat +# Author: David Gibson + +source $(dirname $0)/../exeter/sh/exeter.sh

this is specific to Bash, so what I get with dash is:

smoke/smoke.sh: 16: source: not found smoke/smoke.sh: 21: exeter_register: not found smoke/smoke.sh: 22: exeter_register: not found smoke/smoke.sh: 24: exeter_register: not found smoke/smoke.sh: 25: exeter_register: not found smoke/smoke.sh: 27: exeter_main: not found ...

Drat. I'm pretty sure I found that before, but I must have accidentally discarded with some fat-fingered gitting. Ok, fixed now. -- 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

On Wed, Aug 20, 2025 at 11:52:18AM +0200, Stefano Brivio wrote:

On Wed, 20 Aug 2025 12:55:58 +1000 David Gibson wrote:

...

On Tue, Aug 19, 2025 at 04:27:38PM +0200, Stefano Brivio wrote:

...

On Thu, 7 Aug 2025 21:32:35 +1000 David Gibson wrote:

...

Introduce some trivial testcases based on the exeter library. These run passt and pasta with --help and --version options. Extend our test scripts to run these tests.

Signed-off-by: David Gibson --- test/.gitignore | 1 + test/Makefile | 5 ++++- test/lib/exeter | 46 +++++++++++++++++++++++++++++++++++++++++++++ test/run | 9 ++++++++- test/smoke/smoke.sh | 27 ++++++++++++++++++++++++++ 5 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 test/lib/exeter create mode 100755 test/smoke/smoke.sh

diff --git a/test/.gitignore b/test/.gitignore index 3573444f..cf48b885 100644 --- a/test/.gitignore +++ b/test/.gitignore @@ -11,3 +11,4 @@ nstool rampstream guest-key guest-key.pub +/exeter/ diff --git a/test/Makefile b/test/Makefile index bf63db87..332f3f3e 100644 --- a/test/Makefile +++ b/test/Makefile @@ -50,7 +50,7 @@ UBUNTU_NEW_IMGS = xenial-server-cloudimg-powerpc-disk1.img \ jammy-server-cloudimg-s390x.img UBUNTU_IMGS = $(UBUNTU_OLD_IMGS) $(UBUNTU_NEW_IMGS)

-DOWNLOAD_ASSETS = mbuto podman \ +DOWNLOAD_ASSETS = exeter mbuto podman \ $(DEBIAN_IMGS) $(FEDORA_IMGS) $(OPENSUSE_IMGS) $(UBUNTU_IMGS)

Pre-existing, but this broke a while after you switched the image preparation to Makefile targets, so you can't really expect people to issue 'make' at the moment.

Sorry, I'm not quite sure what the problem is you're getting at.

This: --

Hm, could you avoid using -- to mark quotes like this. mutt mistakes it for the sig and cuts it out; it's kind of a pain to paste it back in for repying.

$ mkdir psst $ cd psst $ git clone git://passt.top/passt [...] $ cd passt/test/ $ make [...] wget -c -O debian-11-generic-ppc64el.qcow2 https://cloud.debian.org/images/cloud/bullseye/latest/debian-11-generic-ppc6... --2025-08-20 11:27:05-- https://cloud.debian.org/images/cloud/bullseye/latest/debian-11-generic-ppc6... Resolving cloud.debian.org (cloud.debian.org)... 2001:6b0:19::165, 2001:6b0:19::173, 2001:6b0:19::163, ... Connecting to cloud.debian.org (cloud.debian.org)|2001:6b0:19::165|:443... connected. HTTP request sent, awaiting response... 404 Not Found 2025-08-20 11:27:05 ERROR 404: Not Found.

make: *** [Makefile:150: debian-11-generic-ppc64el.qcow2] Error 8

Ah, right. All the distro wgets are so slow from Australia that I've always avoided them by keeping the image files in place (or even copying the back from backups). So, I didn't hit this. Yeah.. we should do something about that.

The (very) old logic would make the "distro" tests fail, but it's been a while that one can't really run 'make' under test/ anymore (unless you have everything already prepared, that is).

On one hand, we don't have a replacement for those tests, and, while ugly and clumsy, they were actually useful as they would have told us in advance about stuff such as:

https://bugs.passt.top/show_bug.cgi?id=121

...and probably much more that we're ignoring now. I don't even test routinely on big-endian, we just have build tests from distributions. That's bad, and it will be ages before we can reasonably get a replacement for that.

Yeah. On the other hand, we could at least remove the links that are actually broken. The idea of testing across distros seems to me more important than the exact set of distros/versions to check - that's relatively easily expanded if we have the structure.

So it would be still valuable to get them working again, and it shouldn't be much effort.

On the other hand... you can't do 'make' there, because of tests we don't even run. Maybe those failures should be warnings instead. If somebody finds a "moment" it would be nice to fix those though...

...

...

I ran 'make exeter' to get further. It would be nice to update test/README.md (or fix the Makefile... or both).

Hrm.. "make check" already depends on "make assets" which should (now) include exeter. I'm not sure why that's not working for you.

...and not 'make assets', either.

...

...

...

TESTDATA_ASSETS = small.bin big.bin medium.bin \ rampstream @@ -70,6 +70,9 @@ assets: $(ASSETS) pull-%: % git -C $* pull

+exeter: + git clone https://gitlab.com/dgibson/exeter.git + mbuto: git clone git://mbuto.sh/mbuto

diff --git a/test/lib/exeter b/test/lib/exeter new file mode 100644 index 00000000..4f7bcff9 --- /dev/null +++ b/test/lib/exeter @@ -0,0 +1,46 @@ +#!/bin/sh +# +# SPDX-License-Identifier: GPL-2.0-or-later +# +# PASST - Plug A Simple Socket Transport +# for qemu/UNIX domain socket mode +# +# PASTA - Pack A Subtle Tap Abstraction +# for network namespace/tap device mode +# +# test/lib/exeter - Run exeter tests within the rest of passt's tests +# +# Copyright Red Hat +# Author: David Gibson + +exeter() {

It would be nice to have the usual arguments / function description here.

Good point, added.

...

...

+ STATESETUP="${STATEBASE}/$1" + mkdir -p "${STATESETUP}" + + context_setup_host host + layout_host

Indentation here was done with tabs, this adds spaces instead.

Oops. For some reason I'd added emacs configuration to explicitly *disable* tab indent mode for shell mode in the passt tree. Weird, and now reversed.

...

...

+ + cd test + + __ntests=$("$@" --list | wc -l) + if [ $? != 0 ]; then + info "Failed to get exeter manifest for $@" + pause_continue \ + "Press any key to pause test session" \ + "Resuming in " \ + "Paused, press any key to continue" \ + 5 + return + fi + + status_file_start "Exeter tests: $*" ${__ntests} + + for __testid in $("$@" --list); do + status_test_start "${__testid}" + context_run host "$@" "${__testid}" && status_test_ok || status_test_fail + done + + cd .. + + teardown_context_watch ${PANE_HOST} host +} diff --git a/test/run b/test/run index f73c3119..9e183682 100755 --- a/test/run +++ b/test/run @@ -53,6 +53,7 @@ COMMIT="$(git log --oneline --no-decorate -1)" . lib/layout_ugly . lib/test . lib/video +. lib/exeter

# cleanup() - Remove temporary files cleanup() { @@ -67,6 +68,8 @@ run() { perf_init [ ${CI} -eq 1 ] && video_start ci

+ exeter smoke/smoke.sh + setup build test build/all test build/cppcheck @@ -223,6 +226,10 @@ run_selected() {

__setup= for __test; do + if "test/${__test}" --list; then + exeter "${__test}" + continue + fi # HACK: the migrate tests need the setup repeated for # each test if [ "${__test%%/*}" != "${__setup}" -o \ @@ -234,7 +241,7 @@ run_selected() {

test "${__test}" done - teardown "${__setup}" + [ -n "${__setup}" ] && teardown "${__setup}"

log "PASS: ${STATUS_PASS}, FAIL: ${STATUS_FAIL}, SKIPPED: ${STATUS_SKIPPED}"

diff --git a/test/smoke/smoke.sh b/test/smoke/smoke.sh new file mode 100755 index 00000000..26a98d31 --- /dev/null +++ b/test/smoke/smoke.sh @@ -0,0 +1,27 @@ +#! /bin/sh

...this is desirable, I think (and, for consistency, I'd use "#!/bin/sh") as Bash is not universally available, but:

So.. I believe "#! " (with the space) is actually the traditional / correct version. I did once encounter a situation where it mattered, although to be fair, that might have been with 25 year old SunOS or something equally obscure. Nonetheless I've habitually included the space ever since (see find-arm64-firmware.sh, prepare-distro-img.sh and rapstream-check.sh).

Oh, I didn't know. I thought it was just something that happened to work somehow, but I didn't realise about this:

https://en.wikipedia.org/wiki/Shebang_(Unix)#Version_8_improved_shell_script...

and while this has some details too:

https://www.in-ulm.de/~mascheck/various/shebang/#blankrequired

...sure, makes sense, let's keep those.

-- Stefano

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

On Wed, Aug 20, 2025 at 01:44:27PM +0200, Stefano Brivio wrote:

On Wed, 20 Aug 2025 21:10:17 +1000 David Gibson wrote:

...

On Wed, Aug 20, 2025 at 11:52:18AM +0200, Stefano Brivio wrote:

...

On Wed, 20 Aug 2025 12:55:58 +1000 David Gibson wrote:

...

On Tue, Aug 19, 2025 at 04:27:38PM +0200, Stefano Brivio wrote:

...

On Thu, 7 Aug 2025 21:32:35 +1000 David Gibson wrote:

...

Introduce some trivial testcases based on the exeter library. These run passt and pasta with --help and --version options. Extend our test scripts to run these tests.

Signed-off-by: David Gibson --- test/.gitignore | 1 + test/Makefile | 5 ++++- test/lib/exeter | 46 +++++++++++++++++++++++++++++++++++++++++++++ test/run | 9 ++++++++- test/smoke/smoke.sh | 27 ++++++++++++++++++++++++++ 5 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 test/lib/exeter create mode 100755 test/smoke/smoke.sh

diff --git a/test/.gitignore b/test/.gitignore index 3573444f..cf48b885 100644 --- a/test/.gitignore +++ b/test/.gitignore @@ -11,3 +11,4 @@ nstool rampstream guest-key guest-key.pub +/exeter/ diff --git a/test/Makefile b/test/Makefile index bf63db87..332f3f3e 100644 --- a/test/Makefile +++ b/test/Makefile @@ -50,7 +50,7 @@ UBUNTU_NEW_IMGS = xenial-server-cloudimg-powerpc-disk1.img \ jammy-server-cloudimg-s390x.img UBUNTU_IMGS = $(UBUNTU_OLD_IMGS) $(UBUNTU_NEW_IMGS)

-DOWNLOAD_ASSETS = mbuto podman \ +DOWNLOAD_ASSETS = exeter mbuto podman \ $(DEBIAN_IMGS) $(FEDORA_IMGS) $(OPENSUSE_IMGS) $(UBUNTU_IMGS)

Pre-existing, but this broke a while after you switched the image preparation to Makefile targets, so you can't really expect people to issue 'make' at the moment.

Sorry, I'm not quite sure what the problem is you're getting at.

This: --

Hm, could you avoid using -- to mark quotes like this. mutt mistakes it for the sig and cuts it out; it's kind of a pain to paste it back in for repying.

Oops, weird, because that was originally "--\n", without a space:

Yes, that's what I see too.

https://en.wikipedia.org/wiki/Signature_block#Standard_delimiter

Yeah, I know. I guess mutt considers it the delimiter even without the space, since it's pretty easy to miss.

In your quote that has now three spaces. Is it a bug in mutt? I never hit it though (but I almost exclusively use claws-mail).

Maybe? At least, I think it's a mutt behaviour. It's possible it could be emacs post-mode.

Would "---\n" avoid that?

I think so.

...

...

$ mkdir psst $ cd psst $ git clone git://passt.top/passt [...] $ cd passt/test/ $ make [...] wget -c -O debian-11-generic-ppc64el.qcow2 https://cloud.debian.org/images/cloud/bullseye/latest/debian-11-generic-ppc6... --2025-08-20 11:27:05-- https://cloud.debian.org/images/cloud/bullseye/latest/debian-11-generic-ppc6... Resolving cloud.debian.org (cloud.debian.org)... 2001:6b0:19::165, 2001:6b0:19::173, 2001:6b0:19::163, ... Connecting to cloud.debian.org (cloud.debian.org)|2001:6b0:19::165|:443... connected. HTTP request sent, awaiting response... 404 Not Found 2025-08-20 11:27:05 ERROR 404: Not Found.

...

make: *** [Makefile:150: debian-11-generic-ppc64el.qcow2] Error 8

Ah, right. All the distro wgets are so slow from Australia that I've always avoided them by keeping the image files in place (or even copying the back from backups). So, I didn't hit this. Yeah.. we should do something about that.

...

The (very) old logic would make the "distro" tests fail, but it's been a while that one can't really run 'make' under test/ anymore (unless you have everything already prepared, that is).

...

On one hand, we don't have a replacement for those tests, and, while ugly and clumsy, they were actually useful as they would have told us in advance about stuff such as:

...

https://bugs.passt.top/show_bug.cgi?id=121

...

...and probably much more that we're ignoring now. I don't even test routinely on big-endian, we just have build tests from distributions. That's bad, and it will be ages before we can reasonably get a replacement for that.

Yeah. On the other hand, we could at least remove the links that are actually broken. The idea of testing across distros seems to me more important than the exact set of distros/versions to check - that's relatively easily expanded if we have the structure.

...or find alternate links / similar versions. Older versions are important, too. For that one, this looks like a stable link:

https://cdimage.debian.org/cdimage/cloud/bullseye/20250703-2162/debian-11-ge...

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

On Thu, Aug 21, 2025 at 11:26:59PM +0200, Stefano Brivio wrote:

On Thu, 21 Aug 2025 09:31:01 +1000 David Gibson wrote:

...

On Wed, Aug 20, 2025 at 01:44:27PM +0200, Stefano Brivio wrote:

...

On Wed, 20 Aug 2025 21:10:17 +1000 David Gibson wrote:

...

On Wed, Aug 20, 2025 at 11:52:18AM +0200, Stefano Brivio wrote:

...

On Wed, 20 Aug 2025 12:55:58 +1000 David Gibson wrote:

...

On Tue, Aug 19, 2025 at 04:27:38PM +0200, Stefano Brivio wrote: > On Thu, 7 Aug 2025 21:32:35 +1000 > David Gibson wrote: > > > Introduce some trivial testcases based on the exeter library. These run > > passt and pasta with --help and --version options. Extend our test > > scripts to run these tests. > > > > Signed-off-by: David Gibson > > --- > > test/.gitignore | 1 + > > test/Makefile | 5 ++++- > > test/lib/exeter | 46 +++++++++++++++++++++++++++++++++++++++++++++ > > test/run | 9 ++++++++- > > test/smoke/smoke.sh | 27 ++++++++++++++++++++++++++ > > 5 files changed, 86 insertions(+), 2 deletions(-) > > create mode 100644 test/lib/exeter > > create mode 100755 test/smoke/smoke.sh > > > > diff --git a/test/.gitignore b/test/.gitignore > > index 3573444f..cf48b885 100644 > > --- a/test/.gitignore > > +++ b/test/.gitignore > > @@ -11,3 +11,4 @@ nstool > > rampstream > > guest-key > > guest-key.pub > > +/exeter/ > > diff --git a/test/Makefile b/test/Makefile > > index bf63db87..332f3f3e 100644 > > --- a/test/Makefile > > +++ b/test/Makefile > > @@ -50,7 +50,7 @@ UBUNTU_NEW_IMGS = xenial-server-cloudimg-powerpc-disk1.img \ > > jammy-server-cloudimg-s390x.img > > UBUNTU_IMGS = $(UBUNTU_OLD_IMGS) $(UBUNTU_NEW_IMGS) > > > > -DOWNLOAD_ASSETS = mbuto podman \ > > +DOWNLOAD_ASSETS = exeter mbuto podman \ > > $(DEBIAN_IMGS) $(FEDORA_IMGS) $(OPENSUSE_IMGS) $(UBUNTU_IMGS) > > Pre-existing, but this broke a while after you switched the image > preparation to Makefile targets, so you can't really expect people to > issue 'make' at the moment.

Sorry, I'm not quite sure what the problem is you're getting at.

This: --

Hm, could you avoid using -- to mark quotes like this. mutt mistakes it for the sig and cuts it out; it's kind of a pain to paste it back in for repying.

Oops, weird, because that was originally "--\n", without a space:

Yes, that's what I see too.

...

https://en.wikipedia.org/wiki/Signature_block#Standard_delimiter

Yeah, I know. I guess mutt considers it the delimiter even without the space, since it's pretty easy to miss.

It doesn't for me, and also judging from the implementation:

https://gitlab.com/muttmua/mutt/-/blob/5fd040e3aa1f807db1cab4ca7d7ba5fc8d487...

it looks like it's done properly.

...

...

In your quote that has now three spaces. Is it a bug in mutt? I never hit it though (but I almost exclusively use claws-mail).

Maybe? At least, I think it's a mutt behaviour. It's possible it could be emacs post-mode.

The latter sounds more likely to me. I'm not sure what you can configure in Emacs though.

Aha! Looks like it is that. Finally managed to figure it out. post-signature-sep-regexp was wrong... but changing it didn't seem to fix things. Brute force turning off post-kill-quoted-sig did the trick. -- 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

On Tue, Aug 19, 2025 at 04:27:54PM +0200, Stefano Brivio wrote:

On Thu, 7 Aug 2025 21:32:37 +1000 David Gibson wrote:

...

Convert the tests in build/all to be based on exeter. The new version of the tests is more robust than the original, since it makes a temporary copy of the source tree so will not be affected by concurrent manual builds.

Signed-off-by: David Gibson --- test/build/all | 61 -------------------------------- test/build/build.py | 84 +++++++++++++++++++++++++++++++++++++++++++++ test/run | 8 ++--- 3 files changed, 88 insertions(+), 65 deletions(-) delete mode 100644 test/build/all create mode 100755 test/build/build.py

diff --git a/test/build/all b/test/build/all deleted file mode 100644 index 1f79e0d8..00000000 --- a/test/build/all +++ /dev/null @@ -1,61 +0,0 @@ -# SPDX-License-Identifier: GPL-2.0-or-later -# -# PASST - Plug A Simple Socket Transport -# for qemu/UNIX domain socket mode -# -# PASTA - Pack A Subtle Tap Abstraction -# for network namespace/tap device mode -# -# test/build/all - Build targets, one by one, then all together, check output -# -# Copyright (c) 2021 Red Hat GmbH -# Author: Stefano Brivio - -htools make cc rm uname getconf mkdir cp rm man - -test Build passt -host make clean -check ! [ -e passt ] -host CFLAGS="-Werror" make passt -check [ -f passt ] - -test Build pasta -host make clean -check ! [ -e pasta ] -host CFLAGS="-Werror" make pasta -check [ -h pasta ] - -test Build qrap -host make clean -check ! [ -e qrap ] -host CFLAGS="-Werror" make qrap -check [ -f qrap ] - -test Build all -host make clean -check ! [ -e passt ] -check ! [ -e pasta ] -check ! [ -e qrap ] -host CFLAGS="-Werror" make -check [ -f passt ] -check [ -h pasta ] -check [ -f qrap ] - -test Install -host mkdir __STATEDIR__/prefix -host prefix=__STATEDIR__/prefix make install -check [ -f __STATEDIR__/prefix/bin/passt ] -check [ -h __STATEDIR__/prefix/bin/pasta ] -check [ -f __STATEDIR__/prefix/bin/qrap ] -check man -M __STATEDIR__/prefix/share/man -W passt -check man -M __STATEDIR__/prefix/share/man -W pasta -check man -M __STATEDIR__/prefix/share/man -W qrap - -test Uninstall -host prefix=__STATEDIR__/prefix make uninstall -check ! [ -f __STATEDIR__/prefix/bin/passt ] -check ! [ -h __STATEDIR__/prefix/bin/pasta ] -check ! [ -f __STATEDIR__/prefix/bin/qrap ] -check ! man -M __STATEDIR__/prefix/share/man -W passt 2>/dev/null -check ! man -M __STATEDIR__/prefix/share/man -W pasta 2>/dev/null -check ! man -M __STATEDIR__/prefix/share/man -W qrap 2>/dev/null diff --git a/test/build/build.py b/test/build/build.py new file mode 100755 index 00000000..12bb82d8 --- /dev/null +++ b/test/build/build.py @@ -0,0 +1,84 @@ +#! /usr/bin/env python3 +# +# SPDX-License-Identifier: GPL-2.0-or-later +# +# PASST - Plug A Simple Socket Transport +# for qemu/UNIX domain socket mode +# +# PASTA - Pack A Subtle Tap Abstraction +# for network namespace/tap device mode +# +# test/build/build.py - Test build and install targets +# +# Copyright Red Hat +# Author: David Gibson + +import contextlib +import os +from pathlib import Path +import subprocess +import tempfile +from typing import Iterable, Iterator + +import exeter + +def sh(cmd): + subprocess.run(cmd, shell=True) + + +@contextlib.contextmanager +def clone_sources() -> Iterator[str]: + os.chdir('..') # Move from test/ to repo base + with tempfile.TemporaryDirectory(ignore_cleanup_errors=False) as tmpdir:

I guess I see the advantage of this syntax, it's still a bit less obvious to me than a mere sequence of steps and an explicit cleanup function.

I agree that it's less obvious, but I think the advantages are worth it. Namely that it will correctly run the cleanup in nearly all cases of an interrupted test (not a SIGKILL, obviously). Arguably, it's not a good trade off in this simple case. However once we get to real network tests, it's pretty common to have multiple nested layers of setup, each with their own teardown. Sometimes you need something torn down, but it's only set up partway through the test, or you want something for the first part of the test but not after, so it needs to be torn down only if interrupted at certain points. Manually keeping track of that quickly becomes really painful, I really want to use this technique there. Given that I intend to use it there, I think it makes sense to use it here too: after all it will be even less obvious if there are only hard examples of it and no easy ones.

In any case, it's clear enough to copy and paste if needed, and if one has the possibility to define tests as you showed in 2/3 (even just as an emergency / quick experiment thing), I think it's absolutely fine.

...

+ sh(f"cp --parents -d $(git ls-files) {tmpdir}") + os.chdir(tmpdir) + yield tmpdir + + +def test_make(target: str, outputs: Iterable[str]) -> None:

I probably mentioned this offline but it's hard to figure out that this produces a list of executable paths as output.

It doesn't, which kind of reinforces your point. It takes a list of files it expects the make to generate.

A comment or a different argument name (binary_output_files?) might help. A simple list might be a bit more obvious than Iterable, I suppose.

Yeah, I'll work on all of the above, and try to make it clearer.

...

+ outpaths = [Path(o) for o in outputs] + with clone_sources(): + for o in outpaths: + assert not o.exists(), f"{o} existed before make" + sh(f'make {target} CFLAGS="-Werror"') + for o in outpaths: + assert o.exists(), f"{o} wasn't made" + sh('make clean') + for o in outpaths: + assert not o.exists(), f"{o} existed after make clean" + + +exeter.register('make_passt', test_make, 'passt', ['passt']) +exeter.register('make_pasta', test_make, 'pasta', ['pasta']) +exeter.register('make_qrap', test_make, 'qrap', ['qrap']) +exeter.register('make_all', test_make, 'all', ['passt', 'pasta', 'qrap']) + + +@exeter.test +def test_install_uninstall() -> None: + with clone_sources(): + with tempfile.TemporaryDirectory(ignore_cleanup_errors=False) \ + as prefix: + bindir = Path(prefix) / 'bin' + mandir = Path(prefix) / 'share/man' + progs = ['passt', 'pasta', 'qrap'] + + # Install + sh(f'make install CFLAGS="-Werror" prefix={prefix}') + + for prog in progs: + exe = bindir / prog + assert exe.is_file(), f"{exe} does not exist as a regular file" + sh(f'man -M {mandir} -W {prog}') + + # Uninstall + sh(f'make uninstall prefix={prefix}') + + for prog in progs: + exe = bindir / prog + assert not exe.exists(), f"{exe} exists after uninstall" + sh(f'! man -M {mandir} -W {prog}')

This looks almost as simple and obvious as the shell script equivalent, which is quite remarkable.

Hooray! f"" strings and that sh() wrapper help substantially. -- 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

On Wed, Aug 20, 2025 at 12:40:44PM +0200, Stefano Brivio wrote:

On Wed, 20 Aug 2025 13:10:02 +1000 David Gibson wrote:

...

On Tue, Aug 19, 2025 at 04:27:54PM +0200, Stefano Brivio wrote:

...

On Thu, 7 Aug 2025 21:32:37 +1000 David Gibson wrote:

...

Convert the tests in build/all to be based on exeter. The new version of the tests is more robust than the original, since it makes a temporary copy of the source tree so will not be affected by concurrent manual builds.

Signed-off-by: David Gibson --- test/build/all | 61 -------------------------------- test/build/build.py | 84 +++++++++++++++++++++++++++++++++++++++++++++ test/run | 8 ++--- 3 files changed, 88 insertions(+), 65 deletions(-) delete mode 100644 test/build/all create mode 100755 test/build/build.py

diff --git a/test/build/all b/test/build/all deleted file mode 100644 index 1f79e0d8..00000000 --- a/test/build/all +++ /dev/null @@ -1,61 +0,0 @@ -# SPDX-License-Identifier: GPL-2.0-or-later -# -# PASST - Plug A Simple Socket Transport -# for qemu/UNIX domain socket mode -# -# PASTA - Pack A Subtle Tap Abstraction -# for network namespace/tap device mode -# -# test/build/all - Build targets, one by one, then all together, check output -# -# Copyright (c) 2021 Red Hat GmbH -# Author: Stefano Brivio - -htools make cc rm uname getconf mkdir cp rm man - -test Build passt -host make clean -check ! [ -e passt ] -host CFLAGS="-Werror" make passt -check [ -f passt ] - -test Build pasta -host make clean -check ! [ -e pasta ] -host CFLAGS="-Werror" make pasta -check [ -h pasta ] - -test Build qrap -host make clean -check ! [ -e qrap ] -host CFLAGS="-Werror" make qrap -check [ -f qrap ] - -test Build all -host make clean -check ! [ -e passt ] -check ! [ -e pasta ] -check ! [ -e qrap ] -host CFLAGS="-Werror" make -check [ -f passt ] -check [ -h pasta ] -check [ -f qrap ] - -test Install -host mkdir __STATEDIR__/prefix -host prefix=__STATEDIR__/prefix make install -check [ -f __STATEDIR__/prefix/bin/passt ] -check [ -h __STATEDIR__/prefix/bin/pasta ] -check [ -f __STATEDIR__/prefix/bin/qrap ] -check man -M __STATEDIR__/prefix/share/man -W passt -check man -M __STATEDIR__/prefix/share/man -W pasta -check man -M __STATEDIR__/prefix/share/man -W qrap - -test Uninstall -host prefix=__STATEDIR__/prefix make uninstall -check ! [ -f __STATEDIR__/prefix/bin/passt ] -check ! [ -h __STATEDIR__/prefix/bin/pasta ] -check ! [ -f __STATEDIR__/prefix/bin/qrap ] -check ! man -M __STATEDIR__/prefix/share/man -W passt 2>/dev/null -check ! man -M __STATEDIR__/prefix/share/man -W pasta 2>/dev/null -check ! man -M __STATEDIR__/prefix/share/man -W qrap 2>/dev/null diff --git a/test/build/build.py b/test/build/build.py new file mode 100755 index 00000000..12bb82d8 --- /dev/null +++ b/test/build/build.py @@ -0,0 +1,84 @@ +#! /usr/bin/env python3 +# +# SPDX-License-Identifier: GPL-2.0-or-later +# +# PASST - Plug A Simple Socket Transport +# for qemu/UNIX domain socket mode +# +# PASTA - Pack A Subtle Tap Abstraction +# for network namespace/tap device mode +# +# test/build/build.py - Test build and install targets +# +# Copyright Red Hat +# Author: David Gibson + +import contextlib +import os +from pathlib import Path +import subprocess +import tempfile +from typing import Iterable, Iterator + +import exeter + +def sh(cmd): + subprocess.run(cmd, shell=True) + + +@contextlib.contextmanager +def clone_sources() -> Iterator[str]: + os.chdir('..') # Move from test/ to repo base + with tempfile.TemporaryDirectory(ignore_cleanup_errors=False) as tmpdir:

I guess I see the advantage of this syntax, it's still a bit less obvious to me than a mere sequence of steps and an explicit cleanup function.

I agree that it's less obvious, but I think the advantages are worth it. Namely that it will correctly run the cleanup in nearly all cases of an interrupted test (not a SIGKILL, obviously).

Arguably, it's not a good trade off in this simple case. However once we get to real network tests, it's pretty common to have multiple nested layers of setup, each with their own teardown. Sometimes you need something torn down, but it's only set up partway through the test, or you want something for the first part of the test but not after, so it needs to be torn down only if interrupted at certain points. Manually keeping track of that quickly becomes really painful, I really want to use this technique there.

Another idea (I'm not sure if it makes this useless, but we probably want to implement it anyway) is what nft tests (and some kselftests) do nowadays: every test runs in its own network namespace, so you don't need an explicit teardown. The kernel already tracks things for you.

Right, I'm planning to implement that in tunbridge. Unlike the earlier drafts, my intention is that all the namespaces / whatever you explicitly create will be nested inside a top-level sandboxing namespace. I don't think it obviates the use for context managers, though. For one thing I think we may have occasional need for things that won't be handled by the normal sandbox, but the contextmanager can handle those too. More widely, there are cases where you want to tear something down partway through normal test operation. The context manager will do that neatly, while also doing the teardown if interrupted before that point.

If you combine that with what pasta does (same as container runtimes really), you could get something that's also robust to SIGKILL, by making every test "parent" process (assuming there can be one) PID 1 in its own PID namespace.

Right, it can sometimes make things more confusing during debugging, though.

Add mount namespaces on top, with tmpfs, and we probably don't even need to clean up after ourselves in build tests.

Yes, it could be called a container but it's probably useful to retain tight control of what namespace we detach and when.

Given that pasta(1) is now available on any distribution where you might reasonably find Python, by the way, you could consider using it directly (the installed version, that is) if it helps.

I don't really want to. I consider the test cases being entirely cut off from the outside internet an advantage, in most cases.

...

Given that I intend to use it there, I think it makes sense to use it here too: after all it will be even less obvious if there are only hard examples of it and no easy ones.

Ah, sure, fair point.

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

On Wed, Aug 20, 2025 at 02:13:01PM +0200, Stefano Brivio wrote:

On Wed, 20 Aug 2025 21:19:40 +1000 David Gibson wrote:

...

On Wed, Aug 20, 2025 at 12:40:44PM +0200, Stefano Brivio wrote:

...

On Wed, 20 Aug 2025 13:10:02 +1000 David Gibson wrote:

...

On Tue, Aug 19, 2025 at 04:27:54PM +0200, Stefano Brivio wrote:

...

On Thu, 7 Aug 2025 21:32:37 +1000 David Gibson wrote:

...

Convert the tests in build/all to be based on exeter. The new version of the tests is more robust than the original, since it makes a temporary copy of the source tree so will not be affected by concurrent manual builds.

Signed-off-by: David Gibson --- test/build/all | 61 -------------------------------- test/build/build.py | 84 +++++++++++++++++++++++++++++++++++++++++++++ test/run | 8 ++--- 3 files changed, 88 insertions(+), 65 deletions(-) delete mode 100644 test/build/all create mode 100755 test/build/build.py

diff --git a/test/build/all b/test/build/all deleted file mode 100644 index 1f79e0d8..00000000 --- a/test/build/all +++ /dev/null @@ -1,61 +0,0 @@ -# SPDX-License-Identifier: GPL-2.0-or-later -# -# PASST - Plug A Simple Socket Transport -# for qemu/UNIX domain socket mode -# -# PASTA - Pack A Subtle Tap Abstraction -# for network namespace/tap device mode -# -# test/build/all - Build targets, one by one, then all together, check output -# -# Copyright (c) 2021 Red Hat GmbH -# Author: Stefano Brivio - -htools make cc rm uname getconf mkdir cp rm man - -test Build passt -host make clean -check ! [ -e passt ] -host CFLAGS="-Werror" make passt -check [ -f passt ] - -test Build pasta -host make clean -check ! [ -e pasta ] -host CFLAGS="-Werror" make pasta -check [ -h pasta ] - -test Build qrap -host make clean -check ! [ -e qrap ] -host CFLAGS="-Werror" make qrap -check [ -f qrap ] - -test Build all -host make clean -check ! [ -e passt ] -check ! [ -e pasta ] -check ! [ -e qrap ] -host CFLAGS="-Werror" make -check [ -f passt ] -check [ -h pasta ] -check [ -f qrap ] - -test Install -host mkdir __STATEDIR__/prefix -host prefix=__STATEDIR__/prefix make install -check [ -f __STATEDIR__/prefix/bin/passt ] -check [ -h __STATEDIR__/prefix/bin/pasta ] -check [ -f __STATEDIR__/prefix/bin/qrap ] -check man -M __STATEDIR__/prefix/share/man -W passt -check man -M __STATEDIR__/prefix/share/man -W pasta -check man -M __STATEDIR__/prefix/share/man -W qrap - -test Uninstall -host prefix=__STATEDIR__/prefix make uninstall -check ! [ -f __STATEDIR__/prefix/bin/passt ] -check ! [ -h __STATEDIR__/prefix/bin/pasta ] -check ! [ -f __STATEDIR__/prefix/bin/qrap ] -check ! man -M __STATEDIR__/prefix/share/man -W passt 2>/dev/null -check ! man -M __STATEDIR__/prefix/share/man -W pasta 2>/dev/null -check ! man -M __STATEDIR__/prefix/share/man -W qrap 2>/dev/null diff --git a/test/build/build.py b/test/build/build.py new file mode 100755 index 00000000..12bb82d8 --- /dev/null +++ b/test/build/build.py @@ -0,0 +1,84 @@ +#! /usr/bin/env python3 +# +# SPDX-License-Identifier: GPL-2.0-or-later +# +# PASST - Plug A Simple Socket Transport +# for qemu/UNIX domain socket mode +# +# PASTA - Pack A Subtle Tap Abstraction +# for network namespace/tap device mode +# +# test/build/build.py - Test build and install targets +# +# Copyright Red Hat +# Author: David Gibson + +import contextlib +import os +from pathlib import Path +import subprocess +import tempfile +from typing import Iterable, Iterator + +import exeter + +def sh(cmd): + subprocess.run(cmd, shell=True) + + +@contextlib.contextmanager +def clone_sources() -> Iterator[str]: + os.chdir('..') # Move from test/ to repo base + with tempfile.TemporaryDirectory(ignore_cleanup_errors=False) as tmpdir:

I guess I see the advantage of this syntax, it's still a bit less obvious to me than a mere sequence of steps and an explicit cleanup function.

I agree that it's less obvious, but I think the advantages are worth it. Namely that it will correctly run the cleanup in nearly all cases of an interrupted test (not a SIGKILL, obviously).

Arguably, it's not a good trade off in this simple case. However once we get to real network tests, it's pretty common to have multiple nested layers of setup, each with their own teardown. Sometimes you need something torn down, but it's only set up partway through the test, or you want something for the first part of the test but not after, so it needs to be torn down only if interrupted at certain points. Manually keeping track of that quickly becomes really painful, I really want to use this technique there.

Another idea (I'm not sure if it makes this useless, but we probably want to implement it anyway) is what nft tests (and some kselftests) do nowadays: every test runs in its own network namespace, so you don't need an explicit teardown. The kernel already tracks things for you.

Right, I'm planning to implement that in tunbridge. Unlike the earlier drafts, my intention is that all the namespaces / whatever you explicitly create will be nested inside a top-level sandboxing namespace.

Ah, neat. One might probably argue that the top-level sandboxing should be part of a test runner rather than tunbridge itself, but I guess it's the only practical choice at the moment.

Right. To be clear, I'm planning for tunbridge to do _network_ sandboxing. I think that makes sense, since it's a tool for setting up simulated network configurations. It might do a bit more than that, if it's convenient to do so there, but as you say comprehensive sandboxing doesn't really belong there. It can't really go in exeter either. exeter is too lightweight to even know what kind of sandboxing would make sense for the program using it. An attempt would both make it much too complex, and also make it inflexible, because the sandboxing could interfere with certain tests you might want to write. Really, comprehensive sandboxing belongs in the runner - the thing that runs the exeter tests. Indeed, Avocado offers this - it can run tests in containers. Likewise things like the Git* CIs run things in various containerized environments. I wasn't about to try to implement that in the quick and dirty integration with our existing test scripts, of course.

And this is probably enough complexity at this stage, but eventually, I was thinking, if instead of a single top-level sandboxing, you allow several levels of nested sandboxing with some kind of descriptive way to define it, then...

I mean.. that's a nice idea, but not really in scope for what I currently have planned.

...

I don't think it obviates the use for context managers, though. For one thing I think we may have occasional need for things that won't be handled by the normal sandbox, but the contextmanager can handle those too. More widely, there are cases where you want to tear something down partway through normal test operation. The context manager will do that neatly, while also doing the teardown if interrupted before that point.

...you could take care of this kind of problem as well (at least in some cases?).

Maybe.

Consider the case where you have a group of tests with some expensive setup that's in common between them (say, setting up a guest image and starting a guest... even though eventually I think we should move to libkrun / virtiofs / muvm and get rid of the test image itself).

The single tests could be network-sandboxed between each other, and share the same mount namespace and tmpfs. Once a test finishes, its specific network setup is cleaned up, but you still have the guest image around and possibly the guest running (if we allow that as a special sandboxing level).

[counts occurrences of 'killall -9 nstool' in shell history :)] to me this approach looks more robust / enforcing than context managers, even though I'm not sure how pervasively it can be used.

Again, could be nice, but not really in scope for the immediate future. Also, TBH, I'd probably implement something like this with context managers (though in the wrapper/runner rather than in the test code itself). Or with RAII if using a non-Python, which is loosely equivalent.

...

...

If you combine that with what pasta does (same as container runtimes really), you could get something that's also robust to SIGKILL, by making every test "parent" process (assuming there can be one) PID 1 in its own PID namespace.

Right, it can sometimes make things more confusing during debugging, though.

On that subject, the test contexts stuff you implemented for the current test framework almost entirely eliminates this confusion, at least for my usage.

Well, I'll see how it turns out.

...

...

Add mount namespaces on top, with tmpfs, and we probably don't even need to clean up after ourselves in build tests.

Yes, it could be called a container but it's probably useful to retain tight control of what namespace we detach and when.

Given that pasta(1) is now available on any distribution where you might reasonably find Python, by the way, you could consider using it directly (the installed version, that is) if it helps.

I don't really want to. I consider the test cases being entirely cut off from the outside internet an advantage, in most cases.

That wasn't about internet access, more about network access and a quick way to do / draft sandboxing.

Right, but if you don't need external network access you can do the same thing with unshare(1). -- 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