Re: [PATCH v4 3/3] test: Convert build tests to exeter

[Stefano Brivio <sbrivio@redhat.com>]

On Thu,  7 Aug 2025 21:32:37 +1000
David Gibson <david@gibson.dropbear.id.au> 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 <david@gibson.dropbear.id.au>
> ---
>  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 <sbrivio@redhat.com>
> -
> -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 <david@gibson.dropbear.id.au>
> +
> +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.

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

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

> +
> +
> +if __name__ == '__main__':
> +    exeter.main()
> diff --git a/test/run b/test/run
> index 5fd02d91..6a53e24b 100755
> --- a/test/run
> +++ b/test/run
> @@ -43,6 +43,9 @@ KERNEL=${KERNEL:-"/boot/vmlinuz-$(uname -r)"}
>  
>  COMMIT="$(git log --oneline --no-decorate -1)"
>  
> +# Let exeter tests written in Python find their modules
> +export PYTHONPATH=${BASEPATH}/exeter/py3
> +
>  . lib/util
>  . lib/context
>  . lib/setup
> @@ -69,12 +72,9 @@ run() {
>  	[ ${CI} -eq 1 ]   && video_start ci
>  
>  	exeter smoke/smoke.sh
> +	exeter build/build.py
>  	exeter build/static_checkers.sh
>  
> -	setup build
> -	test build/all
> -	teardown build
> -
>  	setup pasta
>  	test pasta/ndp
>  	test pasta/dhcp

-- 
Stefano