Re: [kvm-unit-tests PATCH v2 03/18] scripts: Refuse to run the tests if not configured for qemu

[Alexandru Elisei <alexandru.elisei@arm.com>]

Hi Drew,

On Mon, Feb 10, 2025 at 02:56:25PM +0100, Andrew Jones wrote:
> On Mon, Feb 10, 2025 at 10:41:53AM +0000, Alexandru Elisei wrote:
> > Hi Drew,
> > 
> > On Tue, Jan 21, 2025 at 03:48:55PM +0100, Andrew Jones wrote:
> > > On Mon, Jan 20, 2025 at 04:43:01PM +0000, Alexandru Elisei wrote:
> > <snip>
> > > > ---
> > > >  arm/efi/run             | 8 ++++++++
> > > >  arm/run                 | 9 +++++++++
> > > >  run_tests.sh            | 8 ++++++++
> > > >  scripts/mkstandalone.sh | 8 ++++++++
> > > >  4 files changed, 33 insertions(+)
> > <snip>
> > > > +case "$TARGET" in
> > > > +qemu)
> > > > +    ;;
> > > > +*)
> > > > +    echo "'$TARGET' not supported for standlone tests"
> > > > +    exit 2
> > > > +esac
> > > 
> > > I think we could put the check in a function in scripts/arch-run.bash and
> > > just use the same error message for all cases.
> > 
> > Coming back to the series.
> > 
> > arm/efi/run and arm/run source scripts/arch-run.bash; run_tests.sh and
> > scripts/mkstandalone.sh don't source scripts/arch-run.bash. There doesn't
> > seem to be a common file that is sourced by all of them.
> 
> scripts/mkstandalone.sh uses arch-run.bash, see generate_test().

Are you referring to this bit:

generate_test ()
{
	<snip>
        (echo "#!/usr/bin/env bash"
         cat scripts/arch-run.bash "$TEST_DIR/run")

I think scripts/arch-run.bash would need to be sourced for any functions defined
there to be usable in mkstandalone.sh.

What I was thinking is something like this:

if ! vmm_supported $TARGET; then
	echo "$0 does not support '$TARGET'"
	exit 2
fi

Were you thinking of something else?

I think mkstandalone should error at the top level (when you do make
standalone), and not rely on the individual scripts to error if the VMM is
not supported. That's because I think creating the test files, booting a
machine and copying the files only to find out that kvm-unit-tests was
misconfigured is a pretty suboptimal experience.

> run_tests.sh doesn't, but I'm not sure it needs to validate TARGET
> since it can leave that to the lower-level scripts.

I put the check in arm/run, and removed it from run_tests.sh, and this is
what I got:

$ ./run_tests.sh selftest-setup
SKIP selftest-setup (./arm/run does not supported 'kvmtool')

which looks good to me.

> 
> > 
> > How about creating a new file in scripts (vmm.bash?) with only this
> > function?
> 
> If we need a new file, then we can add one, but I'd try using
> arch-run.bash or common.bash first.

common.bash seems to work (and the name fits), so I'll give that a go.

Thanks,
Alex