[PATCH 0/8] Tests: introduce custom jobs

[PATCH 0/8] Tests: introduce custom jobs

[Cleber Rosa]

Different users (or even companies) have different interests, and
may want to run a reduced set of tests during development, or a
larger set of tests during QE.

To cover these use cases, this introduces some example (but
functional) jobs.

It's expected that some common jobs will come up from common
requirements for different users (and maybe be added to a common
location such as tests/jobs), and that very specific jobs will be
added to directories specific to certain groups, say
"contrib/com.redhat/jobs" or the like.

This series does *not* add new jobs to GitLab CI pipeline, but this is
expected to be done later on custom runners.  That is, custom runners
could be used for custom jobs.  Anyway, a GitLab CI pipeline can be
seen here:

 https://gitlab.com/cleber.gnu/qemu/-/pipelines/287210066

This is based on the Avocado version bump patch:

 https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg02391.html

Based-On: <20210414161144.1598980-1-crosa@redhat.com>

Cleber Rosa (8):
  Acceptance Jobs: preserve the cache for pip on GitLab CI
  Acceptance tests: do not try to reuse packages from the system
  tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp
  tests/acceptance/migration.py: cancel test if migration is not
    supported
  tests/acceptance/cpu_queries.py: use the proper logging channels
  Acceptance tests: prevent shutdown on non-specific target tests
  tests/acceptance/migration.py: cancel test on s390x
  Tests: add custom test jobs

 .gitlab-ci.yml                           |  1 +
 configure                                |  2 +-
 tests/Makefile.include                   | 10 +++-
 tests/acceptance/cpu_queries.py          |  4 +-
 tests/acceptance/linux_ssh_mips_malta.py |  7 +--
 tests/acceptance/migration.py            | 16 ++++--
 tests/acceptance/version.py              |  2 +-
 tests/jobs/acceptance-all-targets.py     | 67 ++++++++++++++++++++++++
 tests/jobs/acceptance-kvm-only.py        | 35 +++++++++++++
 tests/jobs/qtest-unit-acceptance.py      | 31 +++++++++++
 tests/jobs/qtest-unit.py                 | 24 +++++++++
 tests/jobs/utils.py                      | 22 ++++++++
 12 files changed, 207 insertions(+), 14 deletions(-)
 create mode 100644 tests/jobs/acceptance-all-targets.py
 create mode 100644 tests/jobs/acceptance-kvm-only.py
 create mode 100644 tests/jobs/qtest-unit-acceptance.py
 create mode 100644 tests/jobs/qtest-unit.py
 create mode 100644 tests/jobs/utils.py

-- 
2.25.4

[PATCH 1/8] Acceptance Jobs: preserve the cache for pip on GitLab CI

[Cleber Rosa]

The acceptance jobs (via `make check-venv`) will setup a virtual
environment, and after that install packages defined in
tests/requirements.txt via pip.

Let's enable pip's default cache directory, so that we can save
a bit on time/bandwidth.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 .gitlab-ci.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 52d65d6c04..9cc4676912 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -53,6 +53,7 @@ include:
     key: "${CI_JOB_NAME}-cache"
     paths:
       - ${CI_PROJECT_DIR}/avocado-cache
+      - ~/.cache/pip
     policy: pull-push
   artifacts:
     name: "$CI_JOB_NAME-$CI_COMMIT_REF_SLUG"
-- 
2.25.4

Re: [PATCH 1/8] Acceptance Jobs: preserve the cache for pip on GitLab CI

[Thomas Huth]

On 15/04/2021 23.51, Cleber Rosa wrote:
> The acceptance jobs (via `make check-venv`) will setup a virtual
> environment, and after that install packages defined in
> tests/requirements.txt via pip.
> 
> Let's enable pip's default cache directory, so that we can save
> a bit on time/bandwidth.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   .gitlab-ci.yml | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 52d65d6c04..9cc4676912 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -53,6 +53,7 @@ include:
>       key: "${CI_JOB_NAME}-cache"
>       paths:
>         - ${CI_PROJECT_DIR}/avocado-cache
> +      - ~/.cache/pip

Did you check whether this works? AFAIK the cache directories have to be 
part of the project directory, see:

  https://docs.gitlab.com/ee/ci/yaml/README.html#cache

We already tried to cache ~/avocado/data/cache in the past, but it did not 
work and we had to move the cache manually to the current working directory 
(see commit 5896c539547).

  Thomas

Re: [PATCH 1/8] Acceptance Jobs: preserve the cache for pip on GitLab CI

[Cleber Rosa]

[-- Attachment #1: Type: text/plain, Size: 1811 bytes --]

On Fri, Apr 16, 2021 at 05:56:10AM +0200, Thomas Huth wrote:
> On 15/04/2021 23.51, Cleber Rosa wrote:
> > The acceptance jobs (via `make check-venv`) will setup a virtual
> > environment, and after that install packages defined in
> > tests/requirements.txt via pip.
> > 
> > Let's enable pip's default cache directory, so that we can save
> > a bit on time/bandwidth.
> > 
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >   .gitlab-ci.yml | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index 52d65d6c04..9cc4676912 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -53,6 +53,7 @@ include:
> >       key: "${CI_JOB_NAME}-cache"
> >       paths:
> >         - ${CI_PROJECT_DIR}/avocado-cache
> > +      - ~/.cache/pip
> 
> Did you check whether this works? AFAIK the cache directories have to be
> part of the project directory, see:
> 
>  https://docs.gitlab.com/ee/ci/yaml/README.html#cache
> 
> We already tried to cache ~/avocado/data/cache in the past, but it did not
> work and we had to move the cache manually to the current working directory
> (see commit 5896c539547).
> 
>  Thomas

You're absolutely right, it won't work like that.  My bad.

I was trying to avoid having to set variables or configurations, but
something like the following will be needed:

    before_script:
        - export PIP_CACHE_DIR=${CI_PROJECT_DIR}/pip-cache
        - mkdir -p ${CI_PROJECT_DIR}/pip-cache
    cache:
        paths:
            - ${CI_PROJECT_DIR}/pip-cache

Resulting in:

    Using cached avocado_framework-87.0-py3-none-any.whl (399 kB)

And the likes of:

    https://gitlab.com/cleber.gnu/democi/-/jobs/1186910932#L166

I'll change that on v2.

Thanks!
- Cleber.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 2/8] Acceptance tests: do not try to reuse packages from the system

[Cleber Rosa]

The premise behind the original behavior is that it would save people
from downloading Avocado (and other dependencies) if already installed
on the system.  To be honest, I think it's extremely rare that the
same versions described as dependencies will be available on most
systems.  But, the biggest motivations here are that:

 1) Hacking on QEMU in the same system used to develop Avocado leads
    to confusion with regards to the exact bits that are being used;

 2) Not reusing Python packages from system wide installations gives
    extra assurance that the same behavior will be seen from tests run
    on different machines;

With regards to downloads, pip already caches the downloaded wheels
and tarballs under ~/.cache/pip, so there should not be more than
one download even if the venv is destroyed and recreated.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 8f220e15d1..63477c8b4b 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -96,7 +96,7 @@ AVOCADO_TAGS=$(patsubst %-softmmu,-t arch:%, $(filter %-softmmu,$(TARGETS)))
 
 $(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
 	$(call quiet-command, \
-            $(PYTHON) -m venv --system-site-packages $@, \
+            $(PYTHON) -m venv $@, \
             VENV, $@)
 	$(call quiet-command, \
             $(TESTS_VENV_DIR)/bin/python -m pip -q install -r $(TESTS_VENV_REQ), \
-- 
2.25.4

Re: [PATCH 2/8] Acceptance tests: do not try to reuse packages from the system

[Philippe Mathieu-Daudé]

On 4/15/21 11:51 PM, Cleber Rosa wrote:
> The premise behind the original behavior is that it would save people
> from downloading Avocado (and other dependencies) if already installed
> on the system.  To be honest, I think it's extremely rare that the
> same versions described as dependencies will be available on most
> systems.  But, the biggest motivations here are that:
> 
>  1) Hacking on QEMU in the same system used to develop Avocado leads
>     to confusion with regards to the exact bits that are being used;
> 
>  2) Not reusing Python packages from system wide installations gives
>     extra assurance that the same behavior will be seen from tests run
>     on different machines;
> 
> With regards to downloads, pip already caches the downloaded wheels
> and tarballs under ~/.cache/pip, so there should not be more than
> one download even if the venv is destroyed and recreated.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/Makefile.include | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH 2/8] Acceptance tests: do not try to reuse packages from the system

[Willian Rampazzo]

On Thu, Apr 15, 2021 at 6:52 PM Cleber Rosa <crosa@redhat.com> wrote:
>
> The premise behind the original behavior is that it would save people
> from downloading Avocado (and other dependencies) if already installed
> on the system.  To be honest, I think it's extremely rare that the
> same versions described as dependencies will be available on most
> systems.  But, the biggest motivations here are that:
>
>  1) Hacking on QEMU in the same system used to develop Avocado leads
>     to confusion with regards to the exact bits that are being used;
>

Indeed!

>  2) Not reusing Python packages from system wide installations gives
>     extra assurance that the same behavior will be seen from tests run
>     on different machines;
>
> With regards to downloads, pip already caches the downloaded wheels
> and tarballs under ~/.cache/pip, so there should not be more than
> one download even if the venv is destroyed and recreated.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/Makefile.include | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH 2/8] Acceptance tests: do not try to reuse packages from the system

[Wainer dos Santos Moschetta]

On 4/15/21 6:51 PM, Cleber Rosa wrote:
> The premise behind the original behavior is that it would save people
> from downloading Avocado (and other dependencies) if already installed
> on the system.  To be honest, I think it's extremely rare that the
> same versions described as dependencies will be available on most
> systems.  But, the biggest motivations here are that:
>
>   1) Hacking on QEMU in the same system used to develop Avocado leads
>      to confusion with regards to the exact bits that are being used;
>
>   2) Not reusing Python packages from system wide installations gives
>      extra assurance that the same behavior will be seen from tests run
>      on different machines;
>
> With regards to downloads, pip already caches the downloaded wheels
> and tarballs under ~/.cache/pip, so there should not be more than
> one download even if the venv is destroyed and recreated.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/Makefile.include | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>


>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 8f220e15d1..63477c8b4b 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -96,7 +96,7 @@ AVOCADO_TAGS=$(patsubst %-softmmu,-t arch:%, $(filter %-softmmu,$(TARGETS)))
>   
>   $(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
>   	$(call quiet-command, \
> -            $(PYTHON) -m venv --system-site-packages $@, \
> +            $(PYTHON) -m venv $@, \
>               VENV, $@)
>   	$(call quiet-command, \
>               $(TESTS_VENV_DIR)/bin/python -m pip -q install -r $(TESTS_VENV_REQ), \

[PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp

[Cleber Rosa]

These tests' setUp do not do anything beyong what their base class do.
And while they do decorate the setUp() we can decorate the classes
instead, so no functionality is lost here.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/linux_ssh_mips_malta.py | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/tests/acceptance/linux_ssh_mips_malta.py b/tests/acceptance/linux_ssh_mips_malta.py
index 6dbd02d49d..e309a1105c 100644
--- a/tests/acceptance/linux_ssh_mips_malta.py
+++ b/tests/acceptance/linux_ssh_mips_malta.py
@@ -19,6 +19,8 @@
 from avocado.utils import ssh
 
 
+@skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might timeout')
+@skipUnless(ssh.SSH_CLIENT_BINARY, 'No SSH client available')
 class LinuxSSH(Test):
 
     timeout = 150 # Not for 'configure --enable-debug --enable-debug-tcg'
@@ -65,11 +67,6 @@ def get_kernel_info(self, endianess, wordsize):
         kernel_hash = self.IMAGE_INFO[endianess]['kernel_hash'][wordsize]
         return kernel_url, kernel_hash
 
-    @skipUnless(ssh.SSH_CLIENT_BINARY, 'No SSH client available')
-    @skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might timeout')
-    def setUp(self):
-        super(LinuxSSH, self).setUp()
-
     def get_portfwd(self):
         res = self.vm.command('human-monitor-command',
                               command_line='info usernet')
-- 
2.25.4

Re: [PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp

[Philippe Mathieu-Daudé]

On 4/15/21 11:51 PM, Cleber Rosa wrote:
> These tests' setUp do not do anything beyong what their base class do.
> And while they do decorate the setUp() we can decorate the classes
> instead, so no functionality is lost here.

This is what I did first when adding this test, but it was not working,
so I had to duplicate it to each method. Did something change so now
this is possible?

> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/linux_ssh_mips_malta.py | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/acceptance/linux_ssh_mips_malta.py b/tests/acceptance/linux_ssh_mips_malta.py
> index 6dbd02d49d..e309a1105c 100644
> --- a/tests/acceptance/linux_ssh_mips_malta.py
> +++ b/tests/acceptance/linux_ssh_mips_malta.py
> @@ -19,6 +19,8 @@
>  from avocado.utils import ssh
>  
>  
> +@skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might timeout')
> +@skipUnless(ssh.SSH_CLIENT_BINARY, 'No SSH client available')
>  class LinuxSSH(Test):
>  
>      timeout = 150 # Not for 'configure --enable-debug --enable-debug-tcg'
> @@ -65,11 +67,6 @@ def get_kernel_info(self, endianess, wordsize):
>          kernel_hash = self.IMAGE_INFO[endianess]['kernel_hash'][wordsize]
>          return kernel_url, kernel_hash
>  
> -    @skipUnless(ssh.SSH_CLIENT_BINARY, 'No SSH client available')
> -    @skipUnless(os.getenv('AVOCADO_TIMEOUT_EXPECTED'), 'Test might timeout')
> -    def setUp(self):
> -        super(LinuxSSH, self).setUp()
> -
>      def get_portfwd(self):
>          res = self.vm.command('human-monitor-command',
>                                command_line='info usernet')
> 

Re: [PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp

[Cleber Rosa]

[-- Attachment #1: Type: text/plain, Size: 708 bytes --]

On Fri, Apr 16, 2021 at 07:26:05AM +0200, Philippe Mathieu-Daudé wrote:
> On 4/15/21 11:51 PM, Cleber Rosa wrote:
> > These tests' setUp do not do anything beyong what their base class do.
> > And while they do decorate the setUp() we can decorate the classes
> > instead, so no functionality is lost here.
> 
> This is what I did first when adding this test, but it was not working,
> so I had to duplicate it to each method. Did something change so now
> this is possible?
>

It did, but quite a while ago:

  https://avocado-framework.readthedocs.io/en/87.0/releases/76_0.html#users-test-writers

It could have been updated much earlier, but, better late than never.

Cheers,
- Cleber.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp

[Philippe Mathieu-Daudé]

On 4/16/21 5:43 PM, Cleber Rosa wrote:
> On Fri, Apr 16, 2021 at 07:26:05AM +0200, Philippe Mathieu-Daudé wrote:
>> On 4/15/21 11:51 PM, Cleber Rosa wrote:
>>> These tests' setUp do not do anything beyong what their base class do.
>>> And while they do decorate the setUp() we can decorate the classes
>>> instead, so no functionality is lost here.
>>
>> This is what I did first when adding this test, but it was not working,
>> so I had to duplicate it to each method. Did something change so now
>> this is possible?
>>
> 
> It did, but quite a while ago:
> 
>   https://avocado-framework.readthedocs.io/en/87.0/releases/76_0.html#users-test-writers

OK, the test is older. Do you mind adding a comment?

"Since Avocado 76.0 we can decorate setUp() directly, ..."

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> It could have been updated much earlier, but, better late than never.

Sure :)

Thanks,

Phil.

Re: [PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp

[Wainer dos Santos Moschetta]

On 4/16/21 2:46 PM, Philippe Mathieu-Daudé wrote:
> On 4/16/21 5:43 PM, Cleber Rosa wrote:
>> On Fri, Apr 16, 2021 at 07:26:05AM +0200, Philippe Mathieu-Daudé wrote:
>>> On 4/15/21 11:51 PM, Cleber Rosa wrote:
>>>> These tests' setUp do not do anything beyong what their base class do.
>>>> And while they do decorate the setUp() we can decorate the classes
>>>> instead, so no functionality is lost here.
>>> This is what I did first when adding this test, but it was not working,
>>> so I had to duplicate it to each method. Did something change so now
>>> this is possible?
>>>
>> It did, but quite a while ago:
>>
>>    https://avocado-framework.readthedocs.io/en/87.0/releases/76_0.html#users-test-writers
> OK, the test is older. Do you mind adding a comment?
>
> "Since Avocado 76.0 we can decorate setUp() directly, ..."

Ditto.

Also you may want to adjust VirtiofsSubmountsTest.setUp() in 
tests/acceptance/virtiofs_submounts.py as well.

- Wainer

>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
>> It could have been updated much earlier, but, better late than never.
> Sure :)
>
> Thanks,
>
> Phil.
>

Re: [PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp

[Willian Rampazzo]

On Fri, Apr 16, 2021 at 2:26 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 4/15/21 11:51 PM, Cleber Rosa wrote:
> > These tests' setUp do not do anything beyong what their base class do.
> > And while they do decorate the setUp() we can decorate the classes
> > instead, so no functionality is lost here.
>
> This is what I did first when adding this test, but it was not working,
> so I had to duplicate it to each method. Did something change so now
> this is possible?
>

If your attempt was made prior
https://github.com/avocado-framework/avocado/pull/3570, or with a
version that did not have this fix, this, indeed, wasn't working.

Re: [PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp

[Willian Rampazzo]

On Thu, Apr 15, 2021 at 6:52 PM Cleber Rosa <crosa@redhat.com> wrote:
>
> These tests' setUp do not do anything beyong what their base class do.
> And while they do decorate the setUp() we can decorate the classes
> instead, so no functionality is lost here.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/linux_ssh_mips_malta.py | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

[PATCH 4/8] tests/acceptance/migration.py: cancel test if migration is not supported

[Cleber Rosa]

FIXME: check if there's a way to query migration support before
actually requesting migration.

Some targets/machines contain devices that do not support migration.
Let's acknowledge that and cancel the test as early as possible.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/migration.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
index 792639cb69..25ee55f36a 100644
--- a/tests/acceptance/migration.py
+++ b/tests/acceptance/migration.py
@@ -53,7 +53,11 @@ def do_migrate(self, dest_uri, src_uri=None):
         source_vm = self.get_vm()
         source_vm.add_args('-nodefaults')
         source_vm.launch()
-        source_vm.qmp('migrate', uri=src_uri)
+        response = source_vm.qmp('migrate', uri=src_uri)
+        if 'error' in response:
+            if 'desc' in response['error']:
+                msg = response['error']['desc']
+            self.cancel('Migration does not seem to be supported: %s' % msg)
         self.assert_migration(source_vm, dest_vm)
 
     def _get_free_port(self):
-- 
2.25.4

Re: [PATCH 4/8] tests/acceptance/migration.py: cancel test if migration is not supported

[Philippe Mathieu-Daudé]

On 4/15/21 11:51 PM, Cleber Rosa wrote:
> FIXME: check if there's a way to query migration support before
> actually requesting migration.
> 
> Some targets/machines contain devices that do not support migration.
> Let's acknowledge that and cancel the test as early as possible.
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/migration.py | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> index 792639cb69..25ee55f36a 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -53,7 +53,11 @@ def do_migrate(self, dest_uri, src_uri=None):
>          source_vm = self.get_vm()
>          source_vm.add_args('-nodefaults')
>          source_vm.launch()
> -        source_vm.qmp('migrate', uri=src_uri)
> +        response = source_vm.qmp('migrate', uri=src_uri)
> +        if 'error' in response:
> +            if 'desc' in response['error']:
> +                msg = response['error']['desc']
> +            self.cancel('Migration does not seem to be supported: %s' % msg)
>          self.assert_migration(source_vm, dest_vm)

It would be better to have this done as a generic check_requisites()
method. First because we could reuse it (also at the class level),
second because we could account the time spent for checking separately
from the time spent for the actual testing.

Re: [PATCH 4/8] tests/acceptance/migration.py: cancel test if migration is not supported

[Cleber Rosa]

[-- Attachment #1: Type: text/plain, Size: 2833 bytes --]

On Fri, Apr 16, 2021 at 07:11:04AM +0200, Philippe Mathieu-Daudé wrote:
> On 4/15/21 11:51 PM, Cleber Rosa wrote:
> > FIXME: check if there's a way to query migration support before
> > actually requesting migration.
> > 
> > Some targets/machines contain devices that do not support migration.
> > Let's acknowledge that and cancel the test as early as possible.
> > 
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >  tests/acceptance/migration.py | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> > index 792639cb69..25ee55f36a 100644
> > --- a/tests/acceptance/migration.py
> > +++ b/tests/acceptance/migration.py
> > @@ -53,7 +53,11 @@ def do_migrate(self, dest_uri, src_uri=None):
> >          source_vm = self.get_vm()
> >          source_vm.add_args('-nodefaults')
> >          source_vm.launch()
> > -        source_vm.qmp('migrate', uri=src_uri)
> > +        response = source_vm.qmp('migrate', uri=src_uri)
> > +        if 'error' in response:
> > +            if 'desc' in response['error']:
> > +                msg = response['error']['desc']
> > +            self.cancel('Migration does not seem to be supported: %s' % msg)
> >          self.assert_migration(source_vm, dest_vm)
> 
> It would be better to have this done as a generic check_requisites()
> method. First because we could reuse it (also at the class level),
> second because we could account the time spent for checking separately
> from the time spent for the actual testing.
> 

With regards to separating the time, you suggest that we should
perform the check at the setUp(), and I absolutely agree with the
principle.  But, I wonder if any characteristic of the "vm",
configured during the test (and not available earlier), could affect
its migration capabilities.

Right now we are proposing some "require_*()" methods, such as
require_accelerator("kvm"), because they are checks that, to the best
of my knowlege, do not depend on any further configuration for the vm
instance.

But, your second point, about this being in a method for common use,
is very sound.  IMO the place to put something like you suggest would
be QEMUMachine.  Something like:

   try:
      source_vm.require_migrate()
   except RequirementError as e:
      self.cancel(e)

Ideally, though, one instance of the QEMUMachine used for the checks,
would not be re-used during the test.  The ideal implementation of
QEMUMachine.require_*(), would create a fresh QEMUMachine instance
with user defined characteristics and verify the requirement, leaving
the original instance untouched.

IMO we can pursue that discussion further, while handling this error
condition locally for now.

Thanks,
- Cleber.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/8] tests/acceptance/migration.py: cancel test if migration is not supported

[Willian Rampazzo]

On Thu, Apr 15, 2021 at 6:52 PM Cleber Rosa <crosa@redhat.com> wrote:
>
> FIXME: check if there's a way to query migration support before
> actually requesting migration.
>
> Some targets/machines contain devices that do not support migration.
> Let's acknowledge that and cancel the test as early as possible.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/migration.py | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> index 792639cb69..25ee55f36a 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -53,7 +53,11 @@ def do_migrate(self, dest_uri, src_uri=None):
>          source_vm = self.get_vm()
>          source_vm.add_args('-nodefaults')
>          source_vm.launch()
> -        source_vm.qmp('migrate', uri=src_uri)
> +        response = source_vm.qmp('migrate', uri=src_uri)
> +        if 'error' in response:
> +            if 'desc' in response['error']:
> +                msg = response['error']['desc']
> +            self.cancel('Migration does not seem to be supported: %s' % msg)

I agree with Phil that a generic function would be reusable when
needed, but as it is not needed yet, this looks good to me:

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH 4/8] tests/acceptance/migration.py: cancel test if migration is not supported

[Wainer dos Santos Moschetta]

Hi,

On 4/15/21 6:51 PM, Cleber Rosa wrote:
> FIXME: check if there's a way to query migration support before
> actually requesting migration.
>
> Some targets/machines contain devices that do not support migration.
> Let's acknowledge that and cancel the test as early as possible.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/acceptance/migration.py | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> index 792639cb69..25ee55f36a 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -53,7 +53,11 @@ def do_migrate(self, dest_uri, src_uri=None):
>           source_vm = self.get_vm()
>           source_vm.add_args('-nodefaults')
>           source_vm.launch()
> -        source_vm.qmp('migrate', uri=src_uri)
> +        response = source_vm.qmp('migrate', uri=src_uri)
> +        if 'error' in response:
> +            if 'desc' in response['error']:
> +                msg = response['error']['desc']
> +            self.cancel('Migration does not seem to be supported: %s' % msg)

My concern is about that cancellation actually covering up a real bug.

Cleber, have you seen the test failing on CI?

- Wainer

>           self.assert_migration(source_vm, dest_vm)
>   
>       def _get_free_port(self):

[PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels

[Cleber Rosa]

The test contains methods for the proper log of test related
information.  Let's use that and remove the print and the unused
logging import.

Reference: https://avocado-framework.readthedocs.io/en/87.0/api/test/avocado.html#avocado.Test.log
Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/cpu_queries.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/acceptance/cpu_queries.py b/tests/acceptance/cpu_queries.py
index 293dccb89a..cc9e380cc7 100644
--- a/tests/acceptance/cpu_queries.py
+++ b/tests/acceptance/cpu_queries.py
@@ -8,8 +8,6 @@
 # This work is licensed under the terms of the GNU GPL, version 2 or
 # later.  See the COPYING file in the top-level directory.
 
-import logging
-
 from avocado_qemu import Test
 
 class QueryCPUModelExpansion(Test):
@@ -27,7 +25,7 @@ def test(self):
 
         cpus = self.vm.command('query-cpu-definitions')
         for c in cpus:
-            print(repr(c))
+            self.log.info("Checking CPU: %s", c)
             self.assertNotIn('', c['unavailable-features'], c['name'])
 
         for c in cpus:
-- 
2.25.4

Re: [PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels

[Philippe Mathieu-Daudé]

On 4/15/21 11:51 PM, Cleber Rosa wrote:
> The test contains methods for the proper log of test related

"The Test class ..."?

> information.  Let's use that and remove the print and the unused
> logging import.
> 
> Reference: https://avocado-framework.readthedocs.io/en/87.0/api/test/avocado.html#avocado.Test.log

This test predates Avocado 87.0 :) Maybe mention this is an update
to the recent API?

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/cpu_queries.py | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/tests/acceptance/cpu_queries.py b/tests/acceptance/cpu_queries.py
> index 293dccb89a..cc9e380cc7 100644
> --- a/tests/acceptance/cpu_queries.py
> +++ b/tests/acceptance/cpu_queries.py
> @@ -8,8 +8,6 @@
>  # This work is licensed under the terms of the GNU GPL, version 2 or
>  # later.  See the COPYING file in the top-level directory.
>  
> -import logging
> -
>  from avocado_qemu import Test
>  
>  class QueryCPUModelExpansion(Test):
> @@ -27,7 +25,7 @@ def test(self):
>  
>          cpus = self.vm.command('query-cpu-definitions')
>          for c in cpus:
> -            print(repr(c))
> +            self.log.info("Checking CPU: %s", c)
>              self.assertNotIn('', c['unavailable-features'], c['name'])
>  
>          for c in cpus:
> 

Re: [PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels

[Willian Rampazzo]

On Fri, Apr 16, 2021 at 2:15 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> On 4/15/21 11:51 PM, Cleber Rosa wrote:
> > The test contains methods for the proper log of test related
>
> "The Test class ..."?
>
> > information.  Let's use that and remove the print and the unused
> > logging import.
> >
> > Reference: https://avocado-framework.readthedocs.io/en/87.0/api/test/avocado.html#avocado.Test.log
>
> This test predates Avocado 87.0 :) Maybe mention this is an update
> to the recent API?

This is not a new feature of Avocado 87.0, it has been there since
before Avocado was used on QEMU.

Re: [PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels

[Cleber Rosa]

[-- Attachment #1: Type: text/plain, Size: 728 bytes --]

On Fri, Apr 16, 2021 at 07:15:07AM +0200, Philippe Mathieu-Daudé wrote:
> On 4/15/21 11:51 PM, Cleber Rosa wrote:
> > The test contains methods for the proper log of test related
> 
> "The Test class ..."?
>

Yes, good catch!

> > information.  Let's use that and remove the print and the unused
> > logging import.
> > 
> > Reference: https://avocado-framework.readthedocs.io/en/87.0/api/test/avocado.html#avocado.Test.log
> 
> This test predates Avocado 87.0 :) Maybe mention this is an update
> to the recent API?
>

In fact, that API is *ages* old.  I just happened to post a link to
the latest stable release, because the build of ancient versions are
disable on readthedocs.io.

Regards,
- Cleber.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels

[Willian Rampazzo]

On Thu, Apr 15, 2021 at 6:52 PM Cleber Rosa <crosa@redhat.com> wrote:
>
> The test contains methods for the proper log of test related
> information.  Let's use that and remove the print and the unused
> logging import.
>
> Reference: https://avocado-framework.readthedocs.io/en/87.0/api/test/avocado.html#avocado.Test.log
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/cpu_queries.py | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels

[Wainer dos Santos Moschetta]

Hi,

On 4/15/21 6:51 PM, Cleber Rosa wrote:
> The test contains methods for the proper log of test related
> information.  Let's use that and remove the print and the unused
> logging import.
>
> Reference: https://avocado-framework.readthedocs.io/en/87.0/api/test/avocado.html#avocado.Test.log
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/acceptance/cpu_queries.py | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/tests/acceptance/cpu_queries.py b/tests/acceptance/cpu_queries.py
> index 293dccb89a..cc9e380cc7 100644
> --- a/tests/acceptance/cpu_queries.py
> +++ b/tests/acceptance/cpu_queries.py
> @@ -8,8 +8,6 @@
>   # This work is licensed under the terms of the GNU GPL, version 2 or
>   # later.  See the COPYING file in the top-level directory.
>   
> -import logging
> -
>   from avocado_qemu import Test
>   
>   class QueryCPUModelExpansion(Test):
> @@ -27,7 +25,7 @@ def test(self):
>   
>           cpus = self.vm.command('query-cpu-definitions')
>           for c in cpus:
> -            print(repr(c))
> +            self.log.info("Checking CPU: %s", c)
>               self.assertNotIn('', c['unavailable-features'], c['name'])
>   
>           for c in cpus:

While you are here, maybe merge the two `for c in cpus` loops? :D

Anyway,

Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

[PATCH 6/8] Acceptance tests: prevent shutdown on non-specific target tests

[Cleber Rosa]

When running tests that are not target specific with various target
binaries, some specific behavior appears.  For s390x, when there's no
guest code running, it will produce GUEST_PANICKED events as the
firmware will shutdown the machine.

With this change, no GUEST_PANICKED *event* will be generated.

For some QMP commands, such as "query-migrate", a proper response
("guest-panicked" for the s390x target) will still be given.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/migration.py | 4 ++--
 tests/acceptance/version.py   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
index 25ee55f36a..b4d46becc6 100644
--- a/tests/acceptance/migration.py
+++ b/tests/acceptance/migration.py
@@ -46,12 +46,12 @@ def assert_migration(self, src_vm, dst_vm):
 
     def do_migrate(self, dest_uri, src_uri=None):
         dest_vm = self.get_vm('-incoming', dest_uri)
-        dest_vm.add_args('-nodefaults')
+        dest_vm.add_args('-nodefaults', '-no-shutdown')
         dest_vm.launch()
         if src_uri is None:
             src_uri = dest_uri
         source_vm = self.get_vm()
-        source_vm.add_args('-nodefaults')
+        source_vm.add_args('-nodefaults', '-no-shutdown')
         source_vm.launch()
         response = source_vm.qmp('migrate', uri=src_uri)
         if 'error' in response:
diff --git a/tests/acceptance/version.py b/tests/acceptance/version.py
index 79b923d4fc..3cf18c9878 100644
--- a/tests/acceptance/version.py
+++ b/tests/acceptance/version.py
@@ -17,7 +17,7 @@ class Version(Test):
     :avocado: tags=quick
     """
     def test_qmp_human_info_version(self):
-        self.vm.add_args('-nodefaults')
+        self.vm.add_args('-nodefaults', '-no-shutdown')
         self.vm.launch()
         res = self.vm.command('human-monitor-command',
                               command_line='info version')
-- 
2.25.4

Re: [PATCH 6/8] Acceptance tests: prevent shutdown on non-specific target tests

[Willian Rampazzo]

On Thu, Apr 15, 2021 at 6:52 PM Cleber Rosa <crosa@redhat.com> wrote:
>
> When running tests that are not target specific with various target
> binaries, some specific behavior appears.  For s390x, when there's no
> guest code running, it will produce GUEST_PANICKED events as the
> firmware will shutdown the machine.
>
> With this change, no GUEST_PANICKED *event* will be generated.
>
> For some QMP commands, such as "query-migrate", a proper response
> ("guest-panicked" for the s390x target) will still be given.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/migration.py | 4 ++--
>  tests/acceptance/version.py   | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

Re: [PATCH 6/8] Acceptance tests: prevent shutdown on non-specific target tests

[Wainer dos Santos Moschetta]

Hi,

On 4/15/21 6:51 PM, Cleber Rosa wrote:
> When running tests that are not target specific with various target
> binaries, some specific behavior appears.  For s390x, when there's no
> guest code running, it will produce GUEST_PANICKED events as the
> firmware will shutdown the machine.
>
> With this change, no GUEST_PANICKED *event* will be generated.
>
> For some QMP commands, such as "query-migrate", a proper response
> ("guest-panicked" for the s390x target) will still be given.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/acceptance/migration.py | 4 ++--
>   tests/acceptance/version.py   | 2 +-
>   2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> index 25ee55f36a..b4d46becc6 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -46,12 +46,12 @@ def assert_migration(self, src_vm, dst_vm):
>   
>       def do_migrate(self, dest_uri, src_uri=None):
>           dest_vm = self.get_vm('-incoming', dest_uri)
> -        dest_vm.add_args('-nodefaults')
> +        dest_vm.add_args('-nodefaults', '-no-shutdown')

On the other hand, can't that new argument introduce unwanted behavior 
on other targets? Maybe the argument should be set only for s390 because 
we know it prevents the test failure on that target only.

- Wainer

>           dest_vm.launch()
>           if src_uri is None:
>               src_uri = dest_uri
>           source_vm = self.get_vm()
> -        source_vm.add_args('-nodefaults')
> +        source_vm.add_args('-nodefaults', '-no-shutdown')
>           source_vm.launch()
>           response = source_vm.qmp('migrate', uri=src_uri)
>           if 'error' in response:
> diff --git a/tests/acceptance/version.py b/tests/acceptance/version.py
> index 79b923d4fc..3cf18c9878 100644
> --- a/tests/acceptance/version.py
> +++ b/tests/acceptance/version.py
> @@ -17,7 +17,7 @@ class Version(Test):
>       :avocado: tags=quick
>       """
>       def test_qmp_human_info_version(self):
> -        self.vm.add_args('-nodefaults')
> +        self.vm.add_args('-nodefaults', '-no-shutdown')
>           self.vm.launch()
>           res = self.vm.command('human-monitor-command',
>                                 command_line='info version')

[PATCH 7/8] tests/acceptance/migration.py: cancel test on s390x

[Cleber Rosa]

Because s390x targets it can not currently migrate without a guest
running.

Future work may provide a proper guest, but for now, it's safer to
cancel the test.

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 tests/acceptance/migration.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
index b4d46becc6..4174d55c81 100644
--- a/tests/acceptance/migration.py
+++ b/tests/acceptance/migration.py
@@ -48,6 +48,12 @@ def do_migrate(self, dest_uri, src_uri=None):
         dest_vm = self.get_vm('-incoming', dest_uri)
         dest_vm.add_args('-nodefaults', '-no-shutdown')
         dest_vm.launch()
+
+        cpus = dest_vm.command('query-cpus-fast')
+        if cpus:
+            if cpus[0].get('target') == 's390x':
+                self.cancel('Migration without a guest not possible on s390')
+
         if src_uri is None:
             src_uri = dest_uri
         source_vm = self.get_vm()
-- 
2.25.4

Re: [PATCH 7/8] tests/acceptance/migration.py: cancel test on s390x

[Wainer dos Santos Moschetta]

On 4/15/21 6:51 PM, Cleber Rosa wrote:
> Because s390x targets it can not currently migrate without a guest
> running.
>
> Future work may provide a proper guest, but for now, it's safer to
> cancel the test.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>   tests/acceptance/migration.py | 6 ++++++
>   1 file changed, 6 insertions(+)


Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>


>
> diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> index b4d46becc6..4174d55c81 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -48,6 +48,12 @@ def do_migrate(self, dest_uri, src_uri=None):
>           dest_vm = self.get_vm('-incoming', dest_uri)
>           dest_vm.add_args('-nodefaults', '-no-shutdown')
>           dest_vm.launch()
> +
> +        cpus = dest_vm.command('query-cpus-fast')
> +        if cpus:
> +            if cpus[0].get('target') == 's390x':
> +                self.cancel('Migration without a guest not possible on s390')
> +
>           if src_uri is None:
>               src_uri = dest_uri
>           source_vm = self.get_vm()

Re: [PATCH 7/8] tests/acceptance/migration.py: cancel test on s390x

[Willian Rampazzo]

On Thu, Apr 15, 2021 at 6:52 PM Cleber Rosa <crosa@redhat.com> wrote:
>
> Because s390x targets it can not currently migrate without a guest
> running.
>
> Future work may provide a proper guest, but for now, it's safer to
> cancel the test.
>
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  tests/acceptance/migration.py | 6 ++++++
>  1 file changed, 6 insertions(+)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>

[PATCH 8/8] Tests: add custom test jobs

[Cleber Rosa]

Different users (or even companies) have different interests, and
may want to run a reduced set of tests during development, or a
larger set of tests during QE.

To cover these use cases, some example (but functional) jobs are
introduced here:

1) acceptance-all-targets.py: runs all arch agnostic tests on all
   built targets, unless there are conditions that make them not work
   out of the box ATM, then run all tests that are specific to
   predefined targets.

2) acceptance-kvm-only.py: runs only tests that require KVM and are
   specific to the host architecture.

3) qtest-unit.py: runs a combination of qtest and unit tests (in
   parallel).

4) qtest-unit-acceptance.py: runs a combineation of qtest, unit tests
   and acceptance tests (all of them in parallel)

To run the first two manually, follow the example bellow:

 $ cd build
 $ make check-venv
 $ ./tests/venv/bin/python3 tests/jobs/acceptance-all-targets.py
 $ ./tests/venv/bin/python3 tests/jobs/acceptance-kvm-only.py

The third and fouth example depends on information coming from Meson,
so the easiest way to run it is:

 $ cd build
 $ make check-qtest-unit
 $ make check-qtest-unit-acceptance

These are based on Avocado's Job API, a way to customize an Avocado
job execution beyond the possibilities of command line arguments.
For more Job API resources, please refer to:

a) Job API Examples:
 - https://github.com/avocado-framework/avocado/tree/master/examples/jobs

b) Documentation about configurable features at the Job Level:
 - https://avocado-framework.readthedocs.io/en/87.0/config/index.html

c) Documentation about the TestSuite class
 - https://avocado-framework.readthedocs.io/en/87.0/api/core/avocado.core.html#avocado.core.suite.TestSuite

d) Documentation about the Job class
 - https://avocado-framework.readthedocs.io/en/87.0/api/core/avocado.core.html#avocado.core.job.Job

Signed-off-by: Cleber Rosa <crosa@redhat.com>
---
 configure                            |  2 +-
 tests/Makefile.include               |  8 ++++
 tests/jobs/acceptance-all-targets.py | 67 ++++++++++++++++++++++++++++
 tests/jobs/acceptance-kvm-only.py    | 35 +++++++++++++++
 tests/jobs/qtest-unit-acceptance.py  | 31 +++++++++++++
 tests/jobs/qtest-unit.py             | 24 ++++++++++
 tests/jobs/utils.py                  | 22 +++++++++
 7 files changed, 188 insertions(+), 1 deletion(-)
 create mode 100644 tests/jobs/acceptance-all-targets.py
 create mode 100644 tests/jobs/acceptance-kvm-only.py
 create mode 100644 tests/jobs/qtest-unit-acceptance.py
 create mode 100644 tests/jobs/qtest-unit.py
 create mode 100644 tests/jobs/utils.py

diff --git a/configure b/configure
index 4f374b4889..23273262e5 100755
--- a/configure
+++ b/configure
@@ -6265,7 +6265,7 @@ LINKS="$LINKS pc-bios/s390-ccw/Makefile"
 LINKS="$LINKS roms/seabios/Makefile"
 LINKS="$LINKS pc-bios/qemu-icon.bmp"
 LINKS="$LINKS .gdbinit scripts" # scripts needed by relative path in .gdbinit
-LINKS="$LINKS tests/acceptance tests/data"
+LINKS="$LINKS tests/acceptance tests/data tests/jobs"
 LINKS="$LINKS tests/qemu-iotests/check"
 LINKS="$LINKS python"
 LINKS="$LINKS contrib/plugins/Makefile "
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 63477c8b4b..03443dd0e8 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -133,6 +133,14 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR) get-vm-images
             $(if $(GITLAB_CI),,--failfast) tests/acceptance, \
             "AVOCADO", "tests/acceptance")
 
+# Runs qtest and unit tests on a custom Avocado job
+check-qtest-unit: check-venv $(TESTS_RESULTS_DIR)
+	$(MESON) introspect --tests | $(TESTS_VENV_DIR)/bin/python $(SRC_PATH)/tests/jobs/qtest-unit.py $(TESTS_RESULTS_DIR)
+
+# Runs qtest and unit and accpetance tests on a custom Avocado job
+check-qtest-unit-acceptance: check-venv $(TESTS_RESULTS_DIR)
+	$(MESON) introspect --tests | $(TESTS_VENV_DIR)/bin/python $(SRC_PATH)/tests/jobs/qtest-unit-acceptance.py $(TESTS_RESULTS_DIR)
+
 # Consolidated targets
 
 .PHONY: check-block check check-clean get-vm-images
diff --git a/tests/jobs/acceptance-all-targets.py b/tests/jobs/acceptance-all-targets.py
new file mode 100644
index 0000000000..96703824e6
--- /dev/null
+++ b/tests/jobs/acceptance-all-targets.py
@@ -0,0 +1,67 @@
+#!/usr/bin/env python3
+
+import glob
+import os
+import sys
+
+from avocado.core.job import Job
+from avocado.core.suite import TestSuite
+
+
+def filter_out_currently_broken(variants):
+    """Filter out targets that can not be currently used transparently."""
+    result = []
+    for variant in variants:
+        if (# qemu-system-m68k: Kernel image must be specified
+            variant['qemu_bin'].endswith('qemu-system-m68k') or
+            # qemu-system-sh4: Could not load SHIX bios 'shix_bios.bin'
+            variant['qemu_bin'].endswith('qemu-system-sh4')):
+            continue
+        result.append(variant)
+    return result
+
+
+def add_machine_type(variants):
+    """Add default machine type  parameters to targets that require one."""
+    for variant in variants:
+        if (variant['qemu_bin'].endswith('-arm') or
+            variant['qemu_bin'].endswith('-aarch64')):
+            variant['machine'] = 'virt'
+        if variant['qemu_bin'].endswith('-rx'):
+            variant['machine'] = 'none'
+        if variant['qemu_bin'].endswith('-avr'):
+            variant['machine'] = 'none'
+
+
+def all_system_binaries():
+    """Looks for all system binaries and creates dict variants."""
+    binaries = [target for target in glob.glob('./qemu-system-*')
+                if (os.path.isfile(target) and
+                    os.access(target, os.R_OK | os.X_OK))]
+    variants = []
+    for target in binaries:
+        variants.append({'qemu_bin': target})
+    variants = filter_out_currently_broken(variants)
+    add_machine_type(variants)
+    return variants
+
+
+def main():
+    suite1 = TestSuite.from_config(
+        {'run.references': ['tests/acceptance/'],
+         'filter.by_tags.tags': ['-arch'],
+         'run.dict_variants': all_system_binaries()},
+        name='non-arch-specific')
+
+    suite2 = TestSuite.from_config(
+        {'run.references': ['tests/acceptance/'],
+         'filter.by_tags.tags': ['arch']},
+        name='arch-specific')
+
+    with Job({'job.run.result.html.enabled': 'on'},
+             [suite1, suite2]) as job:
+        return job.run()
+
+
+if __name__ == '__main__':
+    sys.exit(main())
diff --git a/tests/jobs/acceptance-kvm-only.py b/tests/jobs/acceptance-kvm-only.py
new file mode 100644
index 0000000000..acdcbbe087
--- /dev/null
+++ b/tests/jobs/acceptance-kvm-only.py
@@ -0,0 +1,35 @@
+#!/usr/bin/env python3
+
+import os
+import sys
+
+# This comes from tests/acceptance/avocado_qemu/__init__.py and should
+# not be duplicated here.  The solution is to have the "avocado_qemu"
+# code and "python/qemu" available during build
+BUILD_DIR = os.path.dirname(os.path.dirname(os.path.dirname(__file__)))
+if os.path.islink(os.path.dirname(os.path.dirname(__file__))):
+    # The link to the acceptance tests dir in the source code directory
+    lnk = os.path.dirname(os.path.dirname(__file__))
+    #: The QEMU root source directory
+    SOURCE_DIR = os.path.dirname(os.path.dirname(os.readlink(lnk)))
+else:
+    SOURCE_DIR = BUILD_DIR
+sys.path.append(os.path.join(SOURCE_DIR, 'python'))
+
+from avocado.core.job import Job
+
+from qemu.accel import kvm_available
+
+
+def main():
+    if not kvm_available():
+        sys.exit(0)
+
+    config = {'run.references': ['tests/acceptance/'],
+              'filter.by_tags.tags': ['accel:kvm,arch:%s' % os.uname()[4]]}
+    with Job.from_config(config) as job:
+        return job.run()
+
+
+if __name__ == '__main__':
+    sys.exit(main())
diff --git a/tests/jobs/qtest-unit-acceptance.py b/tests/jobs/qtest-unit-acceptance.py
new file mode 100644
index 0000000000..67a25c93f5
--- /dev/null
+++ b/tests/jobs/qtest-unit-acceptance.py
@@ -0,0 +1,31 @@
+#!/usr/bin/env python3
+
+import json
+import random
+import sys
+
+from avocado.core.job import Job
+from avocado.core.resolver import resolve
+from avocado.core.suite import resolutions_to_runnables
+
+from utils import meson_introspect_to_avocado_suite
+
+
+def main():
+    config = {'run.test_runner': 'nrunner'}
+    if len(sys.argv) > 1:
+        config['run.results_dir'] = sys.argv[1]
+
+    suite = meson_introspect_to_avocado_suite(json.load(sys.stdin),
+                                              'qtest-unit-acceptance',
+                                              config)
+    acceptance = resolutions_to_runnables(resolve(["tests/acceptance"]),
+                                          config)
+    suite.tests += acceptance
+    random.shuffle(suite.tests)
+    with Job(config, [suite]) as j:
+        return j.run()
+
+
+if __name__ == '__main__':
+    sys.exit(main())
diff --git a/tests/jobs/qtest-unit.py b/tests/jobs/qtest-unit.py
new file mode 100644
index 0000000000..6f4d1b40c6
--- /dev/null
+++ b/tests/jobs/qtest-unit.py
@@ -0,0 +1,24 @@
+#!/usr/bin/env python3
+
+import sys
+import json
+
+from avocado.core.job import Job
+
+from utils import meson_introspect_to_avocado_suite
+
+
+def main():
+    config = {'run.test_runner': 'nrunner'}
+    if len(sys.argv) > 1:
+        config['run.results_dir'] = sys.argv[1]
+
+    suite = meson_introspect_to_avocado_suite(json.load(sys.stdin),
+                                              'qtest-unit',
+                                              config)
+    with Job(config, [suite]) as j:
+        return j.run()
+
+
+if __name__ == '__main__':
+    sys.exit(main())
diff --git a/tests/jobs/utils.py b/tests/jobs/utils.py
new file mode 100644
index 0000000000..79ac129231
--- /dev/null
+++ b/tests/jobs/utils.py
@@ -0,0 +1,22 @@
+from avocado.core.suite import TestSuite
+from avocado.core.nrunner import Runnable
+
+
+def protocol_tap_to_runnable(entry):
+    runnable = Runnable('tap',
+                        entry['cmd'][0],
+                        *entry['cmd'][1:],
+                        **entry['env'])
+    return runnable
+
+
+def meson_introspect_to_avocado_suite(introspect, suite_name, config):
+    tests = []
+    for entry in introspect:
+        if entry['protocol'] != 'tap':
+            continue
+        runnable = protocol_tap_to_runnable(entry)
+        tests.append(runnable)
+
+    suite = TestSuite(name=suite_name, config=config, tests=tests)
+    return suite
-- 
2.25.4

Re: [PATCH 8/8] Tests: add custom test jobs

[Philippe Mathieu-Daudé]

Hi Cleber,

On 4/15/21 11:51 PM, Cleber Rosa wrote:
> Different users (or even companies) have different interests, and
> may want to run a reduced set of tests during development, or a
> larger set of tests during QE.
> 
> To cover these use cases, some example (but functional) jobs are
> introduced here:
> 
> 1) acceptance-all-targets.py: runs all arch agnostic tests on all
>    built targets, unless there are conditions that make them not work
>    out of the box ATM, then run all tests that are specific to
>    predefined targets.
> 
> 2) acceptance-kvm-only.py: runs only tests that require KVM and are
>    specific to the host architecture.
> 
> 3) qtest-unit.py: runs a combination of qtest and unit tests (in
>    parallel).
> 
> 4) qtest-unit-acceptance.py: runs a combineation of qtest, unit tests

Typo "combination".

>    and acceptance tests (all of them in parallel)
> 
> To run the first two manually, follow the example bellow:
> 
>  $ cd build
>  $ make check-venv
>  $ ./tests/venv/bin/python3 tests/jobs/acceptance-all-targets.py
>  $ ./tests/venv/bin/python3 tests/jobs/acceptance-kvm-only.py
> 
> The third and fouth example depends on information coming from Meson,
> so the easiest way to run it is:
> 
>  $ cd build
>  $ make check-qtest-unit
>  $ make check-qtest-unit-acceptance
> 
> These are based on Avocado's Job API, a way to customize an Avocado
> job execution beyond the possibilities of command line arguments.
> For more Job API resources, please refer to:
> 
> a) Job API Examples:
>  - https://github.com/avocado-framework/avocado/tree/master/examples/jobs
> 
> b) Documentation about configurable features at the Job Level:
>  - https://avocado-framework.readthedocs.io/en/87.0/config/index.html
> 
> c) Documentation about the TestSuite class
>  - https://avocado-framework.readthedocs.io/en/87.0/api/core/avocado.core.html#avocado.core.suite.TestSuite
> 
> d) Documentation about the Job class
>  - https://avocado-framework.readthedocs.io/en/87.0/api/core/avocado.core.html#avocado.core.job.Job
> 
> Signed-off-by: Cleber Rosa <crosa@redhat.com>
> ---
>  configure                            |  2 +-
>  tests/Makefile.include               |  8 ++++
>  tests/jobs/acceptance-all-targets.py | 67 ++++++++++++++++++++++++++++
>  tests/jobs/acceptance-kvm-only.py    | 35 +++++++++++++++
>  tests/jobs/qtest-unit-acceptance.py  | 31 +++++++++++++
>  tests/jobs/qtest-unit.py             | 24 ++++++++++
>  tests/jobs/utils.py                  | 22 +++++++++
>  7 files changed, 188 insertions(+), 1 deletion(-)
>  create mode 100644 tests/jobs/acceptance-all-targets.py
>  create mode 100644 tests/jobs/acceptance-kvm-only.py
>  create mode 100644 tests/jobs/qtest-unit-acceptance.py
>  create mode 100644 tests/jobs/qtest-unit.py
>  create mode 100644 tests/jobs/utils.py

> +if __name__ == '__main__':
> +    sys.exit(main())
> diff --git a/tests/jobs/acceptance-kvm-only.py b/tests/jobs/acceptance-kvm-only.py
> new file mode 100644
> index 0000000000..acdcbbe087
> --- /dev/null
> +++ b/tests/jobs/acceptance-kvm-only.py
> @@ -0,0 +1,35 @@
> +#!/usr/bin/env python3
> +
> +import os
> +import sys
> +
> +# This comes from tests/acceptance/avocado_qemu/__init__.py and should
> +# not be duplicated here.  The solution is to have the "avocado_qemu"
> +# code and "python/qemu" available during build
> +BUILD_DIR = os.path.dirname(os.path.dirname(os.path.dirname(__file__)))
> +if os.path.islink(os.path.dirname(os.path.dirname(__file__))):
> +    # The link to the acceptance tests dir in the source code directory
> +    lnk = os.path.dirname(os.path.dirname(__file__))
> +    #: The QEMU root source directory
> +    SOURCE_DIR = os.path.dirname(os.path.dirname(os.readlink(lnk)))
> +else:
> +    SOURCE_DIR = BUILD_DIR
> +sys.path.append(os.path.join(SOURCE_DIR, 'python'))
> +
> +from avocado.core.job import Job
> +
> +from qemu.accel import kvm_available
> +
> +
> +def main():
> +    if not kvm_available():
> +        sys.exit(0)
> +
> +    config = {'run.references': ['tests/acceptance/'],
> +              'filter.by_tags.tags': ['accel:kvm,arch:%s' % os.uname()[4]]}

If we want forks to use their own set of tags, it would be better to
provide an uniform way, not adding new test entry point for each set
of fork tags. Could we consume a YAML config file instead? And provide
templates so forks could adapt to their needs?

Thanks,

Phil.

Re: [PATCH 8/8] Tests: add custom test jobs

[Cleber Rosa]

[-- Attachment #1: Type: text/plain, Size: 5363 bytes --]

On Fri, Apr 16, 2021 at 07:23:14AM +0200, Philippe Mathieu-Daudé wrote:
> Hi Cleber,
> 
> On 4/15/21 11:51 PM, Cleber Rosa wrote:
> > Different users (or even companies) have different interests, and
> > may want to run a reduced set of tests during development, or a
> > larger set of tests during QE.
> > 
> > To cover these use cases, some example (but functional) jobs are
> > introduced here:
> > 
> > 1) acceptance-all-targets.py: runs all arch agnostic tests on all
> >    built targets, unless there are conditions that make them not work
> >    out of the box ATM, then run all tests that are specific to
> >    predefined targets.
> > 
> > 2) acceptance-kvm-only.py: runs only tests that require KVM and are
> >    specific to the host architecture.
> > 
> > 3) qtest-unit.py: runs a combination of qtest and unit tests (in
> >    parallel).
> > 
> > 4) qtest-unit-acceptance.py: runs a combineation of qtest, unit tests
> 
> Typo "combination".
> 
> >    and acceptance tests (all of them in parallel)
> > 
> > To run the first two manually, follow the example bellow:
> > 
> >  $ cd build
> >  $ make check-venv
> >  $ ./tests/venv/bin/python3 tests/jobs/acceptance-all-targets.py
> >  $ ./tests/venv/bin/python3 tests/jobs/acceptance-kvm-only.py
> > 
> > The third and fouth example depends on information coming from Meson,
> > so the easiest way to run it is:
> > 
> >  $ cd build
> >  $ make check-qtest-unit
> >  $ make check-qtest-unit-acceptance
> > 
> > These are based on Avocado's Job API, a way to customize an Avocado
> > job execution beyond the possibilities of command line arguments.
> > For more Job API resources, please refer to:
> > 
> > a) Job API Examples:
> >  - https://github.com/avocado-framework/avocado/tree/master/examples/jobs
> > 
> > b) Documentation about configurable features at the Job Level:
> >  - https://avocado-framework.readthedocs.io/en/87.0/config/index.html
> > 
> > c) Documentation about the TestSuite class
> >  - https://avocado-framework.readthedocs.io/en/87.0/api/core/avocado.core.html#avocado.core.suite.TestSuite
> > 
> > d) Documentation about the Job class
> >  - https://avocado-framework.readthedocs.io/en/87.0/api/core/avocado.core.html#avocado.core.job.Job
> > 
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >  configure                            |  2 +-
> >  tests/Makefile.include               |  8 ++++
> >  tests/jobs/acceptance-all-targets.py | 67 ++++++++++++++++++++++++++++
> >  tests/jobs/acceptance-kvm-only.py    | 35 +++++++++++++++
> >  tests/jobs/qtest-unit-acceptance.py  | 31 +++++++++++++
> >  tests/jobs/qtest-unit.py             | 24 ++++++++++
> >  tests/jobs/utils.py                  | 22 +++++++++
> >  7 files changed, 188 insertions(+), 1 deletion(-)
> >  create mode 100644 tests/jobs/acceptance-all-targets.py
> >  create mode 100644 tests/jobs/acceptance-kvm-only.py
> >  create mode 100644 tests/jobs/qtest-unit-acceptance.py
> >  create mode 100644 tests/jobs/qtest-unit.py
> >  create mode 100644 tests/jobs/utils.py
> 
> > +if __name__ == '__main__':
> > +    sys.exit(main())
> > diff --git a/tests/jobs/acceptance-kvm-only.py b/tests/jobs/acceptance-kvm-only.py
> > new file mode 100644
> > index 0000000000..acdcbbe087
> > --- /dev/null
> > +++ b/tests/jobs/acceptance-kvm-only.py
> > @@ -0,0 +1,35 @@
> > +#!/usr/bin/env python3
> > +
> > +import os
> > +import sys
> > +
> > +# This comes from tests/acceptance/avocado_qemu/__init__.py and should
> > +# not be duplicated here.  The solution is to have the "avocado_qemu"
> > +# code and "python/qemu" available during build
> > +BUILD_DIR = os.path.dirname(os.path.dirname(os.path.dirname(__file__)))
> > +if os.path.islink(os.path.dirname(os.path.dirname(__file__))):
> > +    # The link to the acceptance tests dir in the source code directory
> > +    lnk = os.path.dirname(os.path.dirname(__file__))
> > +    #: The QEMU root source directory
> > +    SOURCE_DIR = os.path.dirname(os.path.dirname(os.readlink(lnk)))
> > +else:
> > +    SOURCE_DIR = BUILD_DIR
> > +sys.path.append(os.path.join(SOURCE_DIR, 'python'))
> > +
> > +from avocado.core.job import Job
> > +
> > +from qemu.accel import kvm_available
> > +
> > +
> > +def main():
> > +    if not kvm_available():
> > +        sys.exit(0)
> > +
> > +    config = {'run.references': ['tests/acceptance/'],
> > +              'filter.by_tags.tags': ['accel:kvm,arch:%s' % os.uname()[4]]}
> 
> If we want forks to use their own set of tags, it would be better to
> provide an uniform way, not adding new test entry point for each set
> of fork tags. Could we consume a YAML config file instead? And provide
> templates so forks could adapt to their needs?
>
> Thanks,
> 
> Phil.
> 

Yes, it should be possible indeed.  BTW, starting this kind of
discussion is one of the main goals of this series.

With regards to your suggestion, I believe there's an audience and
value to this KVM only job.  But, your idea of a YAML config file is
also very much valid.

Maybe even a job that retrieves the list of tags from a CI variable?
That could allow people to run jobs for different subset of tests at
"push" time, without the need to add committed changes to (YAML)
config files.

Cheers,
- Cleber.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/8] Tests: introduce custom jobs

[Paolo Bonzini]

On 15/04/21 23:51, Cleber Rosa wrote:
> Different users (or even companies) have different interests, and
> may want to run a reduced set of tests during development, or a
> larger set of tests during QE.
> 
> To cover these use cases, this introduces some example (but
> functional) jobs.
> 
> It's expected that some common jobs will come up from common
> requirements for different users (and maybe be added to a common
> location such as tests/jobs), and that very specific jobs will be
> added to directories specific to certain groups, say
> "contrib/com.redhat/jobs" or the like.
> 
> This series does*not*  add new jobs to GitLab CI pipeline, but this is
> expected to be done later on custom runners.  That is, custom runners
> could be used for custom jobs.  Anyway, a GitLab CI pipeline can be
> seen here:
> 
>   https://gitlab.com/cleber.gnu/qemu/-/pipelines/287210066
> 
> This is based on the Avocado version bump patch:
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg02391.html

I admit I haven't even having read the patches (only the diffstat), but 
still: documentation please.

Paolo

Re: [PATCH 0/8] Tests: introduce custom jobs

[Cleber Rosa]

[-- Attachment #1: Type: text/plain, Size: 1806 bytes --]

On Fri, Apr 16, 2021 at 06:22:12PM +0200, Paolo Bonzini wrote:
> On 15/04/21 23:51, Cleber Rosa wrote:
> > Different users (or even companies) have different interests, and
> > may want to run a reduced set of tests during development, or a
> > larger set of tests during QE.
> > 
> > To cover these use cases, this introduces some example (but
> > functional) jobs.
> > 
> > It's expected that some common jobs will come up from common
> > requirements for different users (and maybe be added to a common
> > location such as tests/jobs), and that very specific jobs will be
> > added to directories specific to certain groups, say
> > "contrib/com.redhat/jobs" or the like.
> > 
> > This series does*not*  add new jobs to GitLab CI pipeline, but this is
> > expected to be done later on custom runners.  That is, custom runners
> > could be used for custom jobs.  Anyway, a GitLab CI pipeline can be
> > seen here:
> > 
> >   https://gitlab.com/cleber.gnu/qemu/-/pipelines/287210066
> > 
> > This is based on the Avocado version bump patch:
> > 
> >   https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg02391.html
> 
> I admit I haven't even having read the patches (only the diffstat), but
> still: documentation please.
> 
> Paolo
> 

Hi Paolo,

Absolutely, formal docs are very much needed and will be provided.

For now, please refer to patch 8's commit message.  It contains basic
usage information for these jobs, and pointers to external
documentation.

PS: I'm very much interested in knowing what are user's first
questions or use cases, so that I can tune the documentation
accordingly.  Questions and ideas like the one from Phil (about a YAML
config file) would definitely help me to write a more relevant set of
docs.

Thanks,
- Cleber.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]