diff mbox series

[v2,1/2] tests/acceptance: introduce new check-avocado tartget

Message ID 20211105155354.154864-2-willianr@redhat.com
State Handled Elsewhere
Headers show
Series tests/acceptance: rename tests acceptance to tests avocado | expand

Commit Message

Willian Rampazzo Nov. 5, 2021, 3:53 p.m. UTC
This introduces a new `make` target, `check-avocado`, and adds a
deprecation message about the `check-acceptance` target. This is
a preparation for renaming the `tests/acceptance` folder to
 `tests/avocado`.

The plan is to remove the call to the `check-avocado` target one
or two months after the release and leave the warning to force
people to move to the new `check-avocado` target.

Later, the `check-acceptance` target can be removed. The intent
is to avoid a direct impact during the current soft freeze.

Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Willian Rampazzo <willianr@redhat.com>
---
 docs/about/deprecated.rst | 13 +++++++++++++
 tests/Makefile.include    | 17 ++++++++++++-----
 2 files changed, 25 insertions(+), 5 deletions(-)

Comments

Philippe Mathieu-Daudé Nov. 5, 2021, 4:19 p.m. UTC | #1
On 11/5/21 16:53, Willian Rampazzo wrote:
> This introduces a new `make` target, `check-avocado`, and adds a
> deprecation message about the `check-acceptance` target. This is
> a preparation for renaming the `tests/acceptance` folder to
>  `tests/avocado`.
> 
> The plan is to remove the call to the `check-avocado` target one
> or two months after the release and leave the warning to force
> people to move to the new `check-avocado` target.
> 
> Later, the `check-acceptance` target can be removed. The intent
> is to avoid a direct impact during the current soft freeze.
> 
> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Willian Rampazzo <willianr@redhat.com>
> ---
>  docs/about/deprecated.rst | 13 +++++++++++++
>  tests/Makefile.include    | 17 ++++++++++++-----
>  2 files changed, 25 insertions(+), 5 deletions(-)

Typo "target" in subject (no need to respin).

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Thomas Huth Nov. 8, 2021, 7:59 a.m. UTC | #2
On 05/11/2021 16.53, Willian Rampazzo wrote:
> This introduces a new `make` target, `check-avocado`, and adds a
> deprecation message about the `check-acceptance` target. This is
> a preparation for renaming the `tests/acceptance` folder to
>   `tests/avocado`.
> 
> The plan is to remove the call to the `check-avocado` target one
> or two months after the release and leave the warning to force
> people to move to the new `check-avocado` target.
> 
> Later, the `check-acceptance` target can be removed. The intent
> is to avoid a direct impact during the current soft freeze.
> 
> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Willian Rampazzo <willianr@redhat.com>
> ---
>   docs/about/deprecated.rst | 13 +++++++++++++
>   tests/Makefile.include    | 17 ++++++++++++-----
>   2 files changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 56f9ad15ab..7bf8da8325 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -410,3 +410,16 @@ nanoMIPS ISA
>   
>   The ``nanoMIPS`` ISA has never been upstreamed to any compiler toolchain.
>   As it is hard to generate binaries for it, declare it deprecated.
> +
> +Testing
> +-------
> +
> +Renaming of the acceptance folder to avocado
> +''''''''''''''''''''''''''''''''''''''''''''
> +
> +The ``tests/acceptance`` folder was never used to store acceptance tests
> +in terms of software engineering. This naming can confuse developers
> +adding tests using the Avocado Framework to this folder. The folder
> +name change to ``tests/avocado`` also changed the ``make`` target from
> +``check-acceptance`` to ``check-avocado``. In this case, the use of the
> +``check-acceptance`` target is deprecated.

Not sure whether we need  to document this in deprecated.rst, too, since 
we're normally only listing the things here that affect the users of the 
qemu binaries, not the people who want to recompile and run the tests... 
OTOH, I don't mind too much either if we list it here... Anybody else got an 
opinion on this?

> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 8434a33fe6..8e8ee58493 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -16,7 +16,7 @@ ifneq ($(filter $(all-check-targets), check-softfloat),)
>   	@echo " $(MAKE) check-tcg            Run TCG tests"
>   	@echo " $(MAKE) check-softfloat      Run FPU emulation tests"
>   endif
> -	@echo " $(MAKE) check-acceptance     Run acceptance (functional) tests for currently configured targets"
> +	@echo " $(MAKE) check-avocado        Run avocado (integration) tests for currently configured targets"
>   	@echo
>   	@echo " $(MAKE) check-report.tap     Generates an aggregated TAP test report"
>   	@echo " $(MAKE) check-venv           Creates a Python venv for tests"
> @@ -24,7 +24,7 @@ endif
>   	@echo
>   	@echo "The following are useful for CI builds"
>   	@echo " $(MAKE) check-build          Build most test binaris"
> -	@echo " $(MAKE) get-vm-images        Downloads all images used by acceptance tests, according to configured targets (~350 MB each, 1.5 GB max)"
> +	@echo " $(MAKE) get-vm-images        Downloads all images used by avocado tests, according to configured targets (~350 MB each, 1.5 GB max)"
>   	@echo
>   	@echo
>   	@echo "The variable SPEED can be set to control the gtester speed setting."
> @@ -83,7 +83,7 @@ clean-tcg: $(CLEAN_TCG_TARGET_RULES)
>   
>   # Python venv for running tests
>   
> -.PHONY: check-venv check-acceptance
> +.PHONY: check-venv check-avocado check-acceptance check-acceptance-deprecated-warning
>   
>   TESTS_VENV_DIR=$(BUILD_DIR)/tests/venv
>   TESTS_VENV_REQ=$(SRC_PATH)/tests/requirements.txt
> @@ -127,12 +127,12 @@ get-vm-image-fedora-31-%: check-venv
>   	$(call quiet-command, \
>                $(TESTS_VENV_DIR)/bin/python -m avocado vmimage get \
>                --distro=fedora --distro-version=31 --arch=$*, \
> -	"AVOCADO", "Downloading acceptance tests VM image for $*")
> +	"AVOCADO", "Downloading avocado tests VM image for $*")
>   
>   # download all vm images, according to defined targets
>   get-vm-images: check-venv $(patsubst %,get-vm-image-fedora-31-%, $(FEDORA_31_DOWNLOAD))
>   
> -check-acceptance: check-venv $(TESTS_RESULTS_DIR) get-vm-images
> +check-avocado: check-venv $(TESTS_RESULTS_DIR) get-vm-images
>   	$(call quiet-command, \
>               $(TESTS_VENV_DIR)/bin/python -m avocado \
>               --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
> @@ -142,6 +142,13 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR) get-vm-images
>               $(if $(GITLAB_CI),,--failfast) $(AVOCADO_TESTS), \
>               "AVOCADO", "tests/acceptance")
>   
> +check-acceptance-deprecated-warning:
> +	@echo
> +	@echo "Note '$(MAKE) check-acceptance' is deprecated, use '$(MAKE) check-avocado' instead."
> +	@echo
> +
> +check-acceptance: check-acceptance-deprecated-warning | check-avocado
> +
>   # Consolidated targets
>   
>   .PHONY: check-block check check-clean get-vm-images
> 

Acked-by: Thomas Huth <thuth@redhat.com>
Philippe Mathieu-Daudé Nov. 8, 2021, 8:13 a.m. UTC | #3
On 11/8/21 08:59, Thomas Huth wrote:
> On 05/11/2021 16.53, Willian Rampazzo wrote:
>> This introduces a new `make` target, `check-avocado`, and adds a
>> deprecation message about the `check-acceptance` target. This is
>> a preparation for renaming the `tests/acceptance` folder to
>>   `tests/avocado`.
>>
>> The plan is to remove the call to the `check-avocado` target one
>> or two months after the release and leave the warning to force
>> people to move to the new `check-avocado` target.
>>
>> Later, the `check-acceptance` target can be removed. The intent
>> is to avoid a direct impact during the current soft freeze.
>>
>> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Signed-off-by: Willian Rampazzo <willianr@redhat.com>
>> ---
>>   docs/about/deprecated.rst | 13 +++++++++++++
>>   tests/Makefile.include    | 17 ++++++++++++-----
>>   2 files changed, 25 insertions(+), 5 deletions(-)
>>
>> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>> index 56f9ad15ab..7bf8da8325 100644
>> --- a/docs/about/deprecated.rst
>> +++ b/docs/about/deprecated.rst
>> @@ -410,3 +410,16 @@ nanoMIPS ISA
>>     The ``nanoMIPS`` ISA has never been upstreamed to any compiler
>> toolchain.
>>   As it is hard to generate binaries for it, declare it deprecated.
>> +
>> +Testing
>> +-------
>> +
>> +Renaming of the acceptance folder to avocado
>> +''''''''''''''''''''''''''''''''''''''''''''
>> +
>> +The ``tests/acceptance`` folder was never used to store acceptance tests
>> +in terms of software engineering. This naming can confuse developers
>> +adding tests using the Avocado Framework to this folder. The folder
>> +name change to ``tests/avocado`` also changed the ``make`` target from
>> +``check-acceptance`` to ``check-avocado``. In this case, the use of the
>> +``check-acceptance`` target is deprecated.
> 
> Not sure whether we need  to document this in deprecated.rst, too, since
> we're normally only listing the things here that affect the users of the
> qemu binaries, not the people who want to recompile and run the tests...
> OTOH, I don't mind too much either if we list it here... Anybody else
> got an opinion on this?

Hmm OK my bad, I asked Willian to add that without noticing this file
is for "only things that affect the users".

Willian, if you agree with Thomas, I can remove this change from your
patch.
Daniel P. Berrangé Nov. 8, 2021, 9:24 a.m. UTC | #4
On Mon, Nov 08, 2021 at 08:59:51AM +0100, Thomas Huth wrote:
> On 05/11/2021 16.53, Willian Rampazzo wrote:
> > This introduces a new `make` target, `check-avocado`, and adds a
> > deprecation message about the `check-acceptance` target. This is
> > a preparation for renaming the `tests/acceptance` folder to
> >   `tests/avocado`.
> > 
> > The plan is to remove the call to the `check-avocado` target one
> > or two months after the release and leave the warning to force
> > people to move to the new `check-avocado` target.
> > 
> > Later, the `check-acceptance` target can be removed. The intent
> > is to avoid a direct impact during the current soft freeze.
> > 
> > Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > Signed-off-by: Willian Rampazzo <willianr@redhat.com>
> > ---
> >   docs/about/deprecated.rst | 13 +++++++++++++
> >   tests/Makefile.include    | 17 ++++++++++++-----
> >   2 files changed, 25 insertions(+), 5 deletions(-)
> > 
> > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> > index 56f9ad15ab..7bf8da8325 100644
> > --- a/docs/about/deprecated.rst
> > +++ b/docs/about/deprecated.rst
> > @@ -410,3 +410,16 @@ nanoMIPS ISA
> >   The ``nanoMIPS`` ISA has never been upstreamed to any compiler toolchain.
> >   As it is hard to generate binaries for it, declare it deprecated.
> > +
> > +Testing
> > +-------
> > +
> > +Renaming of the acceptance folder to avocado
> > +''''''''''''''''''''''''''''''''''''''''''''
> > +
> > +The ``tests/acceptance`` folder was never used to store acceptance tests
> > +in terms of software engineering. This naming can confuse developers
> > +adding tests using the Avocado Framework to this folder. The folder
> > +name change to ``tests/avocado`` also changed the ``make`` target from
> > +``check-acceptance`` to ``check-avocado``. In this case, the use of the
> > +``check-acceptance`` target is deprecated.
> 
> Not sure whether we need  to document this in deprecated.rst, too, since
> we're normally only listing the things here that affect the users of the
> qemu binaries, not the people who want to recompile and run the tests...
> OTOH, I don't mind too much either if we list it here... Anybody else got an
> opinion on this?

Deprecations are only things for user facing changes in the apps.

For build system changes we don't bother with any deprecation cycle.
Just make the change immediately and document it in the release notes.


Regards,
Daniel
Philippe Mathieu-Daudé Nov. 8, 2021, 9:49 a.m. UTC | #5
On 11/8/21 10:24, Daniel P. Berrangé wrote:
> On Mon, Nov 08, 2021 at 08:59:51AM +0100, Thomas Huth wrote:
>> On 05/11/2021 16.53, Willian Rampazzo wrote:
>>> This introduces a new `make` target, `check-avocado`, and adds a
>>> deprecation message about the `check-acceptance` target. This is
>>> a preparation for renaming the `tests/acceptance` folder to
>>>   `tests/avocado`.
>>>
>>> The plan is to remove the call to the `check-avocado` target one
>>> or two months after the release and leave the warning to force
>>> people to move to the new `check-avocado` target.
>>>
>>> Later, the `check-acceptance` target can be removed. The intent
>>> is to avoid a direct impact during the current soft freeze.
>>>
>>> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> Signed-off-by: Willian Rampazzo <willianr@redhat.com>
>>> ---
>>>   docs/about/deprecated.rst | 13 +++++++++++++
>>>   tests/Makefile.include    | 17 ++++++++++++-----
>>>   2 files changed, 25 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>>> index 56f9ad15ab..7bf8da8325 100644
>>> --- a/docs/about/deprecated.rst
>>> +++ b/docs/about/deprecated.rst
>>> @@ -410,3 +410,16 @@ nanoMIPS ISA
>>>   The ``nanoMIPS`` ISA has never been upstreamed to any compiler toolchain.
>>>   As it is hard to generate binaries for it, declare it deprecated.
>>> +
>>> +Testing
>>> +-------
>>> +
>>> +Renaming of the acceptance folder to avocado
>>> +''''''''''''''''''''''''''''''''''''''''''''
>>> +
>>> +The ``tests/acceptance`` folder was never used to store acceptance tests
>>> +in terms of software engineering. This naming can confuse developers
>>> +adding tests using the Avocado Framework to this folder. The folder
>>> +name change to ``tests/avocado`` also changed the ``make`` target from
>>> +``check-acceptance`` to ``check-avocado``. In this case, the use of the
>>> +``check-acceptance`` target is deprecated.
>>
>> Not sure whether we need  to document this in deprecated.rst, too, since
>> we're normally only listing the things here that affect the users of the
>> qemu binaries, not the people who want to recompile and run the tests...
>> OTOH, I don't mind too much either if we list it here... Anybody else got an
>> opinion on this?
> 
> Deprecations are only things for user facing changes in the apps.

OK.

> For build system changes we don't bother with any deprecation cycle.
> Just make the change immediately and document it in the release notes.

Understood.

Willian, do you mind updating the release notes?
https://wiki.qemu.org/ChangeLog/6.2#Testing_and_CI
Willian Rampazzo Nov. 8, 2021, 11:57 a.m. UTC | #6
On Mon, Nov 8, 2021 at 6:49 AM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> On 11/8/21 10:24, Daniel P. Berrangé wrote:
> > On Mon, Nov 08, 2021 at 08:59:51AM +0100, Thomas Huth wrote:
> >> On 05/11/2021 16.53, Willian Rampazzo wrote:
> >>> This introduces a new `make` target, `check-avocado`, and adds a
> >>> deprecation message about the `check-acceptance` target. This is
> >>> a preparation for renaming the `tests/acceptance` folder to
> >>>   `tests/avocado`.
> >>>
> >>> The plan is to remove the call to the `check-avocado` target one
> >>> or two months after the release and leave the warning to force
> >>> people to move to the new `check-avocado` target.
> >>>
> >>> Later, the `check-acceptance` target can be removed. The intent
> >>> is to avoid a direct impact during the current soft freeze.
> >>>
> >>> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >>> Signed-off-by: Willian Rampazzo <willianr@redhat.com>
> >>> ---
> >>>   docs/about/deprecated.rst | 13 +++++++++++++
> >>>   tests/Makefile.include    | 17 ++++++++++++-----
> >>>   2 files changed, 25 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> >>> index 56f9ad15ab..7bf8da8325 100644
> >>> --- a/docs/about/deprecated.rst
> >>> +++ b/docs/about/deprecated.rst
> >>> @@ -410,3 +410,16 @@ nanoMIPS ISA
> >>>   The ``nanoMIPS`` ISA has never been upstreamed to any compiler toolchain.
> >>>   As it is hard to generate binaries for it, declare it deprecated.
> >>> +
> >>> +Testing
> >>> +-------
> >>> +
> >>> +Renaming of the acceptance folder to avocado
> >>> +''''''''''''''''''''''''''''''''''''''''''''
> >>> +
> >>> +The ``tests/acceptance`` folder was never used to store acceptance tests
> >>> +in terms of software engineering. This naming can confuse developers
> >>> +adding tests using the Avocado Framework to this folder. The folder
> >>> +name change to ``tests/avocado`` also changed the ``make`` target from
> >>> +``check-acceptance`` to ``check-avocado``. In this case, the use of the
> >>> +``check-acceptance`` target is deprecated.
> >>
> >> Not sure whether we need  to document this in deprecated.rst, too, since
> >> we're normally only listing the things here that affect the users of the
> >> qemu binaries, not the people who want to recompile and run the tests...
> >> OTOH, I don't mind too much either if we list it here... Anybody else got an
> >> opinion on this?
> >
> > Deprecations are only things for user facing changes in the apps.
>
> OK.
>
> > For build system changes we don't bother with any deprecation cycle.
> > Just make the change immediately and document it in the release notes.
>
> Understood.
>
> Willian, do you mind updating the release notes?
> https://wiki.qemu.org/ChangeLog/6.2#Testing_and_CI
>

Sure, I can do that, but I think I need to wait for the patch to be
merged, right?
diff mbox series

Patch

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 56f9ad15ab..7bf8da8325 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -410,3 +410,16 @@  nanoMIPS ISA
 
 The ``nanoMIPS`` ISA has never been upstreamed to any compiler toolchain.
 As it is hard to generate binaries for it, declare it deprecated.
+
+Testing
+-------
+
+Renaming of the acceptance folder to avocado
+''''''''''''''''''''''''''''''''''''''''''''
+
+The ``tests/acceptance`` folder was never used to store acceptance tests
+in terms of software engineering. This naming can confuse developers
+adding tests using the Avocado Framework to this folder. The folder
+name change to ``tests/avocado`` also changed the ``make`` target from
+``check-acceptance`` to ``check-avocado``. In this case, the use of the
+``check-acceptance`` target is deprecated.
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 8434a33fe6..8e8ee58493 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -16,7 +16,7 @@  ifneq ($(filter $(all-check-targets), check-softfloat),)
 	@echo " $(MAKE) check-tcg            Run TCG tests"
 	@echo " $(MAKE) check-softfloat      Run FPU emulation tests"
 endif
-	@echo " $(MAKE) check-acceptance     Run acceptance (functional) tests for currently configured targets"
+	@echo " $(MAKE) check-avocado        Run avocado (integration) tests for currently configured targets"
 	@echo
 	@echo " $(MAKE) check-report.tap     Generates an aggregated TAP test report"
 	@echo " $(MAKE) check-venv           Creates a Python venv for tests"
@@ -24,7 +24,7 @@  endif
 	@echo
 	@echo "The following are useful for CI builds"
 	@echo " $(MAKE) check-build          Build most test binaris"
-	@echo " $(MAKE) get-vm-images        Downloads all images used by acceptance tests, according to configured targets (~350 MB each, 1.5 GB max)"
+	@echo " $(MAKE) get-vm-images        Downloads all images used by avocado tests, according to configured targets (~350 MB each, 1.5 GB max)"
 	@echo
 	@echo
 	@echo "The variable SPEED can be set to control the gtester speed setting."
@@ -83,7 +83,7 @@  clean-tcg: $(CLEAN_TCG_TARGET_RULES)
 
 # Python venv for running tests
 
-.PHONY: check-venv check-acceptance
+.PHONY: check-venv check-avocado check-acceptance check-acceptance-deprecated-warning
 
 TESTS_VENV_DIR=$(BUILD_DIR)/tests/venv
 TESTS_VENV_REQ=$(SRC_PATH)/tests/requirements.txt
@@ -127,12 +127,12 @@  get-vm-image-fedora-31-%: check-venv
 	$(call quiet-command, \
              $(TESTS_VENV_DIR)/bin/python -m avocado vmimage get \
              --distro=fedora --distro-version=31 --arch=$*, \
-	"AVOCADO", "Downloading acceptance tests VM image for $*")
+	"AVOCADO", "Downloading avocado tests VM image for $*")
 
 # download all vm images, according to defined targets
 get-vm-images: check-venv $(patsubst %,get-vm-image-fedora-31-%, $(FEDORA_31_DOWNLOAD))
 
-check-acceptance: check-venv $(TESTS_RESULTS_DIR) get-vm-images
+check-avocado: check-venv $(TESTS_RESULTS_DIR) get-vm-images
 	$(call quiet-command, \
             $(TESTS_VENV_DIR)/bin/python -m avocado \
             --show=$(AVOCADO_SHOW) run --job-results-dir=$(TESTS_RESULTS_DIR) \
@@ -142,6 +142,13 @@  check-acceptance: check-venv $(TESTS_RESULTS_DIR) get-vm-images
             $(if $(GITLAB_CI),,--failfast) $(AVOCADO_TESTS), \
             "AVOCADO", "tests/acceptance")
 
+check-acceptance-deprecated-warning:
+	@echo
+	@echo "Note '$(MAKE) check-acceptance' is deprecated, use '$(MAKE) check-avocado' instead."
+	@echo
+
+check-acceptance: check-acceptance-deprecated-warning | check-avocado
+
 # Consolidated targets
 
 .PHONY: check-block check check-clean get-vm-images