On Tue, Mar 19, 2024 at 11:07:40AM +0100, Paul Holzinger wrote:
> 
> On 19/03/2024 06:07, David Gibson wrote:
> > The pasta_podman/bats test scrpt downloads and builds podman, then runs its
> > pasta specific tests.  Downloading from within a test case has some
> > drawbacks:
> >   * It can be very tedious if you have poor connectivity to the server
> >   * It makes a test that's ostensibly for pasta itself dependent on the
> >     state of the github server
> >   * It precludes runnning the tests in an isolated network environment
> > 
> > The same concerns largely apply to building podman too, because it's pretty
> > common for Go builds to download dependencies themselves.  Therefore move
> > the download and build of podman from the test itself, to the Makefile
> > where we prepare other test assets.
> FYI, we use vendoring in podman and most of our other repos. So running
> something like `make podman` should never download anything. The only target
> that downloads the deps is `make vendor` which is not needed unless you
> update the dependencies. Anyhow it doesn't change the fact that your other
> reasons for this change are valid.

Ok, good to know.

> > To avoid cryptic failures if something went wrong with the build, make
> > running the test dependent on having the built podman binary.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >   test/.gitignore        |  1 +
> >   test/Makefile          | 12 ++++++++++--
> >   test/pasta_podman/bats |  6 ++----
> >   3 files changed, 13 insertions(+), 6 deletions(-)
> > 
> > diff --git a/test/.gitignore b/test/.gitignore
> > index 48374028..6dd4790b 100644
> > --- a/test/.gitignore
> > +++ b/test/.gitignore
> > @@ -1,5 +1,6 @@
> >   test_logs/
> >   mbuto/
> > +podman/
> >   *.img
> >   QEMU_EFI.fd
> >   *.qcow2
> > diff --git a/test/Makefile b/test/Makefile
> > index 711c61c1..35a3b559 100644
> > --- a/test/Makefile
> > +++ b/test/Makefile
> > @@ -52,10 +52,10 @@ 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 \
> > +DOWNLOAD_ASSETS = mbuto podman \
> >   	$(DEBIAN_IMGS) $(FEDORA_IMGS) $(OPENSUSE_IMGS) $(UBUNTU_IMGS)
> >   TESTDATA_ASSETS = small.bin big.bin medium.bin
> > -LOCAL_ASSETS = mbuto.img mbuto.mem.img QEMU_EFI.fd \
> > +LOCAL_ASSETS = mbuto.img mbuto.mem.img podman/bin/podman QEMU_EFI.fd \
> >   	$(DEBIAN_IMGS:%=prepared-%) $(FEDORA_IMGS:%=prepared-%) \
> >   	$(UBUNTU_NEW_IMGS:%=prepared-%) \
> >   	nstool guest-key guest-key.pub \
> > @@ -76,6 +76,14 @@ mbuto:
> >   mbuto/mbuto: pull-mbuto
> > +podman:
> > +	git clone https://github.com/containers/podman.git
> > +
> > +# To succesfully build podman, you will need gpgme and systemd
> > +# development packages
> > +podman/bin/podman: pull-podman
> > +	$(MAKE) -C podman
> > +
> >   guest-key guest-key.pub:
> >   	ssh-keygen -f guest-key -N ''
> > diff --git a/test/pasta_podman/bats b/test/pasta_podman/bats
> > index 21446f08..cb88aa41 100644
> > --- a/test/pasta_podman/bats
> > +++ b/test/pasta_podman/bats
> > @@ -11,11 +11,9 @@
> >   # Copyright (c) 2022 Red Hat GmbH
> >   # Author: Stefano Brivio <sbrivio@redhat.com>
> > -htools	git make go bats catatonit ip jq socat
> > +htools	git make go bats catatonit ip jq socat ./test/podman/bin/podman
> >   test	Podman system test with bats
> > -host	git -C __STATEDIR__ clone https://github.com/containers/podman.git
> > -host	make -C __STATEDIR__/podman
> >   hout	WD pwd
> > -host	PODMAN="__STATEDIR__/podman/bin/podman" CONTAINERS_HELPER_BINARY_DIR="__WD__" bats __STATEDIR__/podman/test/system/505-networking-pasta.bats
> > +host	PODMAN="test/podman/bin/podman" CONTAINERS_HELPER_BINARY_DIR="__WD__" bats test/podman/test/system/505-networking-pasta.bats
> Note while these env vars are stable and should not change I would strongly
> recommend to add some kind of smoke test here in the form of running `podman
> info --format "{{.Host.Pasta.Executable}}"`, this will return the full path
> to the binary so you should make sure this is the path to your test binary
> and not the system pasta version to ensure you are actually testing the
> right thing.

Good idea, I've added that for v2.

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