Re: [PATCH v4 1/9] tests/functional: Re-activate the check-venv target

[Thomas Huth <thuth@redhat.com>]

On 29/09/2025 08.26, Thomas Huth wrote:
> On 26/09/2025 17.43, Gustavo Romero wrote:
>> Hi Thomas!
>>
>> On 9/26/25 05:34, Thomas Huth wrote:
>>> On 26/09/2025 07.15, Gustavo Romero wrote:
>>>> Add check-venv target as a dependency for the functional tests. This
>>>> causes Python modules listed in pythondeps.toml, under the testdeps
>>>> group, to be installed when 'make check-functional' is executed to
>>>> prepare and run the functional tests.
>>>>
>>>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>>>> Suggested-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>>   tests/Makefile.include | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>>>> index 3538c0c740..d012a9b25d 100644
>>>> --- a/tests/Makefile.include
>>>> +++ b/tests/Makefile.include
>>>> @@ -109,7 +109,7 @@ $(FUNCTIONAL_TARGETS):
>>>>       @$(MAKE) SPEED=thorough $(subst -functional,-func,$@)
>>>>   .PHONY: check-functional
>>>> -check-functional:
>>>> +check-functional: check-venv
>>>
>>> I just noticed that there's still a problem: If you run "make check- 
>>> functional-aarch64" immediately after configuring + compiling QEMU in a 
>>> fresh folder for the first time, the functional tests fail with:
>>>
>>> ModuleNotFoundError: No module named 'pygdbmi'
>>>
>>> We either need to add dependencies to the check-functional-<arch> 
>>> targets, too, or we have to make sure that tests still get properly 
>>> skipped in the case that pygdbmi has not been installed into the venv yet.
>>
>> Isn't it inconsistent that check-functional runs the test and
>> check-functional-<arch> doesn't? I think it's a good idea to
>> skip if the module is not available, yeah, I'll add it in v6,
>> but would it be ok to add check-venv to the check-functional-<arch>
>> targets too?
> 
> I think so... so please try to change this patch to add the "check-venv" 
> dependency to the "$(FUNCTIONAL_TARGETS):" line instead.

Looking at the code twice, I think you need it in addition, not "instead" 
(since there is no direct dependency from check-functional to the 
check-functional-<ARCH> targets).

  Thomas