Re: [Qemu-devel] [PATCH 2/2] Acceptance tests: add make rule for running them

[Markus Armbruster]

Eduardo Habkost <address@hidden> writes:

> On Thu, Sep 20, 2018 at 11:19:56AM -0400, Cleber Rosa wrote:
>> The acceptance (aka functional, aka Avocado-based) tests are
>> Python files located in "tests/acceptance" that need to be run
>> with the Avocado libs and test runner.
>> 
>> Let's provide a convenient way for QEMU developers to run them,
>> by making use of the tests-venv with the required setup.
>> 
>> Also, while the Avocado test runner will take care of creating a
>> location to save test results to, it was understood that it's better
>> if the results are kept within the build tree.
>> 
>> Signed-off-by: Cleber Rosa <address@hidden>
>> ---
[...]
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index 9bb90a83d4..8cef694954 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -11,6 +11,7 @@ check-help:
>>      @echo " $(MAKE) check-qapi-schema    Run QAPI schema tests"
>>      @echo " $(MAKE) check-block          Run block tests"
>>      @echo " $(MAKE) check-tcg            Run TCG tests"
>> +    @echo " $(MAKE) check-acceptance     Run all acceptance (functional) 
>> tests"
>>      @echo " $(MAKE) check-report.html    Generates an HTML test report"
>>      @echo " $(MAKE) check-venv           Creates a Python venv for tests"
>>      @echo " $(MAKE) check-clean          Clean the tests"
>> @@ -1002,10 +1003,11 @@ check-decodetree:
>>  
>>  # Python venv for running tests
>>  
>> -.PHONY: check-venv
>> +.PHONY: check-venv check-acceptance
>>  
>>  TESTS_VENV_DIR=$(BUILD_DIR)/tests/venv
>>  TESTS_VENV_REQ=$(BUILD_DIR)/tests/venv-requirements.txt
>> +TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results
>>  
>>  $(TESTS_VENV_DIR):
>>      $(call quiet-command, \
>> @@ -1015,8 +1017,19 @@ $(TESTS_VENV_DIR):
>>              $(TESTS_VENV_DIR)/bin/pip -q install -r $(TESTS_VENV_REQ), \
>>              PIP, $(TESTS_VENV_REQ))
>>  
>> +$(TESTS_RESULTS_DIR):
>> +    $(call quiet-command, mkdir -p $@, \
>> +            MKDIR, $@)
>> +
>>  check-venv: $(TESTS_VENV_DIR)
>>  
>> +check-acceptance: check-venv $(TESTS_RESULTS_DIR)
>> +    $(call quiet-command, \
>> +            $(TESTS_VENV_DIR)/bin/avocado \
>> +            --show=none run --job-results-dir=$(TESTS_RESULTS_DIR) 
>> --failfast=on \
>> +            $(SRC_PATH)/tests/acceptance, \
>> +            "AVOCADO", "tests/acceptance")
>
> I think we should provide something easy to use for people who
> already have the right Avocado version installed in their system
> and want to avoid re-downloading Avocado every time.
>
> We already have plans to do this automatically/transparently in
> the future, but maybe while we don't have something automatic we
> could have two separate rules.  e.g.:
>
>   AVOCADO = avocado
>
>   check-acceptance: $(TESTS_RESULTS_DIR)
>       $(call quiet-command, \
>               $(AVOCADO) \
>               --show=none run --job-results-dir=$(TESTS_RESULTS_DIR) 
> --failfast=on \
>               $(SRC_PATH)/tests/acceptance, \
>               "AVOCADO", "tests/acceptance")
>
>   check-acceptance-venv: check-venv
>       $(MAKE) check-acceptance AVOCADO=$(TESTS_VENV_DIR)/bin/avocado

Recursion just to override a variable feels like overkill.
Have you considered target-specific variable values?

https://www.gnu.org/software/make/manual/html_node/Target_002dspecific.html#Target_002dspecific

[...]