From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202508 header.b=a2YbDhOD; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id E86C05A027E for ; Wed, 20 Aug 2025 05:10:16 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202508; t=1755659413; bh=DJKHlnzi8kom5iSah4Fsy2E4giFWXwtFEjiXZgUZyBo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=a2YbDhODeLvVnUuZ4S8++06+CJr3k3jtFI6svdskgGFmrRnZJSomDyXp633qRZFgO r58akJn0cEDcC2nFnvxB0JWjP5Wq1jMliAz/lhIuPmMfLRkwGIM7Z07VgmXQ0H0uB4 4xRQu1TLW9BTavLe2Ul/8vP7G2+U+Ck4L8oSTN4wNUKFPRJGOmxQNfMofc+ibahi8R ZlPaAVyvy0R3ZN2As0s5+JhXBfuqDZ81sfA9eS31XfS7sAjEaClbPb2iChRQ3IrVKD SYhy9LpzDnwUQUCcsk7sL4VXLPweYRKebQSLrDsKgN1fOsi1oyjn6tludyT/XzReEf xcOAfyMDOmscA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4c6BK14Dy2z4x7F; Wed, 20 Aug 2025 13:10:13 +1000 (AEST) Date: Wed, 20 Aug 2025 13:10:02 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v4 3/3] test: Convert build tests to exeter Message-ID: References: <20250807113237.548294-1-david@gibson.dropbear.id.au> <20250807113237.548294-4-david@gibson.dropbear.id.au> <20250819162754.0fdb4c86@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="gQ3t4iskOYsH8bCo" Content-Disposition: inline In-Reply-To: <20250819162754.0fdb4c86@elisabeth> Message-ID-Hash: TFFM5VZDSYJGFKI3WWUOCH3A5MB7QZQV X-Message-ID-Hash: TFFM5VZDSYJGFKI3WWUOCH3A5MB7QZQV X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --gQ3t4iskOYsH8bCo Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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: >=20 > > 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. > >=20 > > 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 > >=20 > > 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=3D"-Werror" make passt > > -check [ -f passt ] > > - > > -test Build pasta > > -host make clean > > -check ! [ -e pasta ] > > -host CFLAGS=3D"-Werror" make pasta > > -check [ -h pasta ] > > - > > -test Build qrap > > -host make clean > > -check ! [ -e qrap ] > > -host CFLAGS=3D"-Werror" make qrap > > -check [ -f qrap ] > > - > > -test Build all > > -host make clean > > -check ! [ -e passt ] > > -check ! [ -e pasta ] > > -check ! [ -e qrap ] > > -host CFLAGS=3D"-Werror" make > > -check [ -f passt ] > > -check [ -h pasta ] > > -check [ -f qrap ] > > - > > -test Install > > -host mkdir __STATEDIR__/prefix > > -host prefix=3D__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=3D__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=3DTrue) > > + > > + > > +@contextlib.contextmanager > > +def clone_sources() -> Iterator[str]: > > + os.chdir('..') # Move from test/ to repo base > > + with tempfile.TemporaryDirectory(ignore_cleanup_errors=3DFalse) as= tmpdir: >=20 > 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. >=20 > > + sh(f"cp --parents -d $(git ls-files) {tmpdir}") > > + os.chdir(tmpdir) > > + yield tmpdir > > + > > + > > +def test_make(target: str, outputs: Iterable[str]) -> None: >=20 > 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. >=20 > > + outpaths =3D [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=3D"-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=3DFalse= ) \ > > + as prefix: > > + bindir =3D Path(prefix) / 'bin' > > + mandir =3D Path(prefix) / 'share/man' > > + progs =3D ['passt', 'pasta', 'qrap'] > > + > > + # Install > > + sh(f'make install CFLAGS=3D"-Werror" prefix=3D{prefix}') > > + > > + for prog in progs: > > + exe =3D bindir / prog > > + assert exe.is_file(), f"{exe} does not exist as a regu= lar file" > > + sh(f'man -M {mandir} -W {prog}') > > + > > + # Uninstall > > + sh(f'make uninstall prefix=3D{prefix}') > > + > > + for prog in progs: > > + exe =3D bindir / prog > > + assert not exe.exists(), f"{exe} exists after uninstal= l" > > + sh(f'! man -M {mandir} -W {prog}') >=20 > 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. --=20 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 --gQ3t4iskOYsH8bCo Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmilPIkACgkQzQJF27ox 2GfPeA//VWh+Yv10Z4pnwct0vf1fyo26Ntp3Fu9IGpLPnf2unXsijJCWABIL8ZgM w11/eGg4RGcksOHnbVzxinC3yh3n/M8ez9Ku0JOHpTTE4KcfBvdMiub9lyKrQsKO AdHcEEhC8VbnZBODKWB+vfJ6pr66Pz4MAdQAu8qF4kQ57BrbO61DYfZQnlCaj5n3 WpS61vAaGbFhfYKCMGRhtuxZ3vm2iqJl6K1lUMOHxzO76yOB5mK9RDwPDs764uQq qfKvwxWEMcPL2rw6yr8lrCIqO9OBs30bcaxKZyJcRPPuGN6yIf2v2FpcU2Mn7Bps 6LnppMXzr9Jr4SoDef5HeokeJsJcEPAxfRId0r+/O9JTOxebzK3ylqF/JDHV++x3 Zoej3XXphaFIyg55a6k7d/8W9Xh5xL2ObepLltev263zZu3auLX8zAdPipCeEXNA g7+wDr5ymXTBceHyyymPs7Rf1KrMes3l2zWhcXpd366XcJQGcKTJHL+aBE2snwlR isK/LQOch+QF+afRfvFFqnROCOKFeW57/VXXyS71UxSw9xVlDfGckHdBrvV2aSFM a0IqbTtBAUDJmhlmVLPYDVyOQkx66mvTyUBhJkWLR29JVDYStxfWa0x5Qr+QNWHY 2DYf2garZdD4eijzM3SnJYz7FQp3wAnK7RjIIDNEuegMM7l6kvI= =rJeT -----END PGP SIGNATURE----- --gQ3t4iskOYsH8bCo--