Hello Alexandru,
On Monday, February 10, 2025, 1:04:29 PM, you wrote:
> 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.
Grammar nit: This should be
SKIP selftest-setup (./arm/run does not support 'kvmtool')
Al
>>
>> >
>> > 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
