Re: [PATCH v7 10/11] iotests: rewrite check into python - Vladimir Sementsov-Ogievskiy

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, mreitz@redhat.com,
	den@openvz.org, jsnow@redhat.com
Subject: Re: [PATCH v7 10/11] iotests: rewrite check into python
Date: Mon, 25 Jan 2021 15:31:25 +0300	[thread overview]
Message-ID: <22fe65f5-669b-0a23-a473-12bfee586de6@virtuozzo.com> (raw)
In-Reply-To: <20210125120240.GA7107@merkur.fritz.box>

25.01.2021 15:02, Kevin Wolf wrote:
> Am 23.01.2021 um 16:08 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> 22.01.2021 19:08, Kevin Wolf wrote:
>>> Am 16.01.2021 um 14:44 hat Vladimir Sementsov-Ogievskiy geschrieben:
>>>> Just use classes introduced in previous three commits. Behavior
>>>> difference is described in these three commits.
>>>>
>>>> Drop group file, as it becomes unused.
>>>>
>>>> Drop common.env: now check is in python, and for tests we use same
>>>> python interpreter that runs the check itself. Use build environment
>>>> PYTHON in check-block instead, to keep "make check" use the same
>>>> python.
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>
>>>> diff --git a/tests/check-block.sh b/tests/check-block.sh
>>>> index fb4c1baae9..26eb1c0a9b 100755
>>>> --- a/tests/check-block.sh
>>>> +++ b/tests/check-block.sh
>>>> @@ -69,7 +69,7 @@ export QEMU_CHECK_BLOCK_AUTO=1
>>>>    ret=0
>>>>    for fmt in $format_list ; do
>>>> -    ./check -makecheck -$fmt $group || ret=1
>>>> +    ${PYTHON} ./check -makecheck -$fmt $group || ret=1
>>>>    done
>>>
>>> When I add an echo to print that command line, it seems that ${PYTHON}
>>> is empty for me. Is this expected?
>>
>> It seems to be defined defined when called from make check. Did you
>> just call check-block directly?
> D>
>> It's not intentional, but I think it's OK: if PYTHON is not defined
>> let's just execute check as self-executable. And for make-check PYTHON
>> is defined and correct python is used.
> 
> Hm, where does that happen in 'make check'? It seems the old makefiles
> were quite readable in comparison to what we have now...
> 
> Anyway, I think 'make check-block' should run just the block-specific
> subset of 'make check', without changing the behaviour of the remaining
> tests. Anything that can be started through make should respect the
> configured Python interpreter.
> 
>>>>    exit $ret
>>>> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
>>>> index 952762d5ed..914321806a 100755
>>>> --- a/tests/qemu-iotests/check
>>>> +++ b/tests/qemu-iotests/check
>>
>> [..]
>>
>>>> -            if [ -x "$binary" ]
>>>> -            then
>>>> -                export QEMU_PROG="$build_root/$binary"
>>>> -                break
>>>> -            fi
>>>> -        done
>>>> -        popd > /dev/null
>>>> -        [ "$QEMU_PROG" = "" ] && _init_error "qemu not found"
>>>> -    fi
>>>
>>> I think this else branch is kind of important (if there is no system
>>> emulator binary for the host architecture, find _any_ system emulator
>>> binary that was built). I can't find its equivalent in the new code.
>>
>> Hmm, I decided testing "first found" emulator is strange.. It seems
>> like we have several emulators and user don't care which would be
>> tested?
> 
> Remember that we're mainly testing the block layer, which is the same in
> all qemu-system-* binaries anyway. So yes, any system emulator binary is
> good enough for many test cases, and certainly better than having no
> system emulator. Differences are only in the supported guest devices,
> which may cause some tests to be skipped.
> 
> If there are multiple binaries that we could use, we could change the
> way to select one instead of just the first one, e.g. by trying x86_64
> first because this is what enables the largest set of tests.
> 
> But anything is better than failing with "qemu not found".
> 
>> Probably we should instead used qemu-system-* binary only if there is
>> only one matching binary. And fail if there are many.
> 
> No, 'make check' shouldn't fail because I built arm and ppc emulators on
> my x86_64 machine without also building a x86_64 emulator. (And I think
> this is a case that fails both with the actual patch under review and
> with your suggested change.)
> 

OK, I'll send a squash-in


-- 
Best regards,
Vladimir