qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] Tests: introduce custom jobs
@ 2021-04-15 21:51 Cleber Rosa
  2021-04-15 21:51 ` [PATCH 1/8] Acceptance Jobs: preserve the cache for pip on GitLab CI Cleber Rosa
                   ` (8 more replies)
  0 siblings, 9 replies; 37+ messages in thread
From: Cleber Rosa @ 2021-04-15 21:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Aleksandar Rikalo, Beraldo Leal, Philippe Mathieu-Daudé,
	Cornelia Huck, Thomas Huth, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Cleber Rosa, Alex Bennée, Aurelien Jarno, Eduardo Habkost

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




^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCH 1/8] Acceptance Jobs: preserve the cache for pip on GitLab CI
  2021-04-15 21:51 [PATCH 0/8] Tests: introduce custom jobs Cleber Rosa
@ 2021-04-15 21:51 ` Cleber Rosa
  2021-04-16  3:56   ` Thomas Huth
  2021-04-15 21:51 ` [PATCH 2/8] Acceptance tests: do not try to reuse packages from the system Cleber Rosa
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 37+ messages in thread
From: Cleber Rosa @ 2021-04-15 21:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Aleksandar Rikalo, Beraldo Leal, Philippe Mathieu-Daudé,
	Cornelia Huck, Thomas Huth, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Cleber Rosa, Alex Bennée, Aurelien Jarno, Eduardo Habkost

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



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 2/8] Acceptance tests: do not try to reuse packages from the system
  2021-04-15 21:51 [PATCH 0/8] Tests: introduce custom jobs Cleber Rosa
  2021-04-15 21:51 ` [PATCH 1/8] Acceptance Jobs: preserve the cache for pip on GitLab CI Cleber Rosa
@ 2021-04-15 21:51 ` Cleber Rosa
  2021-04-16  5:07   ` Philippe Mathieu-Daudé
                     ` (2 more replies)
  2021-04-15 21:51 ` [PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp Cleber Rosa
                   ` (6 subsequent siblings)
  8 siblings, 3 replies; 37+ messages in thread
From: Cleber Rosa @ 2021-04-15 21:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Aleksandar Rikalo, Beraldo Leal, Philippe Mathieu-Daudé,
	Cornelia Huck, Thomas Huth, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Cleber Rosa, Alex Bennée, Aurelien Jarno, Eduardo Habkost

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



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp
  2021-04-15 21:51 [PATCH 0/8] Tests: introduce custom jobs Cleber Rosa
  2021-04-15 21:51 ` [PATCH 1/8] Acceptance Jobs: preserve the cache for pip on GitLab CI Cleber Rosa
  2021-04-15 21:51 ` [PATCH 2/8] Acceptance tests: do not try to reuse packages from the system Cleber Rosa
@ 2021-04-15 21:51 ` Cleber Rosa
  2021-04-16  5:26   ` Philippe Mathieu-Daudé
  2021-04-16 15:41   ` Willian Rampazzo
  2021-04-15 21:51 ` [PATCH 4/8] tests/acceptance/migration.py: cancel test if migration is not supported Cleber Rosa
                   ` (5 subsequent siblings)
  8 siblings, 2 replies; 37+ messages in thread
From: Cleber Rosa @ 2021-04-15 21:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Aleksandar Rikalo, Beraldo Leal, Philippe Mathieu-Daudé,
	Cornelia Huck, Thomas Huth, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Cleber Rosa, Alex Bennée, Aurelien Jarno, Eduardo Habkost

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



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 4/8] tests/acceptance/migration.py: cancel test if migration is not supported
  2021-04-15 21:51 [PATCH 0/8] Tests: introduce custom jobs Cleber Rosa
                   ` (2 preceding siblings ...)
  2021-04-15 21:51 ` [PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp Cleber Rosa
@ 2021-04-15 21:51 ` Cleber Rosa
  2021-04-16  5:11   ` Philippe Mathieu-Daudé
                     ` (2 more replies)
  2021-04-15 21:51 ` [PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels Cleber Rosa
                   ` (4 subsequent siblings)
  8 siblings, 3 replies; 37+ messages in thread
From: Cleber Rosa @ 2021-04-15 21:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Aleksandar Rikalo, Beraldo Leal, Philippe Mathieu-Daudé,
	Cornelia Huck, Thomas Huth, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Cleber Rosa, Alex Bennée, Aurelien Jarno, Eduardo Habkost

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



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels
  2021-04-15 21:51 [PATCH 0/8] Tests: introduce custom jobs Cleber Rosa
                   ` (3 preceding siblings ...)
  2021-04-15 21:51 ` [PATCH 4/8] tests/acceptance/migration.py: cancel test if migration is not supported Cleber Rosa
@ 2021-04-15 21:51 ` Cleber Rosa
  2021-04-16  5:15   ` Philippe Mathieu-Daudé
                     ` (2 more replies)
  2021-04-15 21:51 ` [PATCH 6/8] Acceptance tests: prevent shutdown on non-specific target tests Cleber Rosa
                   ` (3 subsequent siblings)
  8 siblings, 3 replies; 37+ messages in thread
From: Cleber Rosa @ 2021-04-15 21:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Aleksandar Rikalo, Beraldo Leal, Philippe Mathieu-Daudé,
	Cornelia Huck, Thomas Huth, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Cleber Rosa, Alex Bennée, Aurelien Jarno, Eduardo Habkost

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



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 6/8] Acceptance tests: prevent shutdown on non-specific target tests
  2021-04-15 21:51 [PATCH 0/8] Tests: introduce custom jobs Cleber Rosa
                   ` (4 preceding siblings ...)
  2021-04-15 21:51 ` [PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels Cleber Rosa
@ 2021-04-15 21:51 ` Cleber Rosa
  2021-04-16 15:56   ` Willian Rampazzo
  2021-04-19 19:07   ` Wainer dos Santos Moschetta
  2021-04-15 21:51 ` [PATCH 7/8] tests/acceptance/migration.py: cancel test on s390x Cleber Rosa
                   ` (2 subsequent siblings)
  8 siblings, 2 replies; 37+ messages in thread
From: Cleber Rosa @ 2021-04-15 21:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Aleksandar Rikalo, Beraldo Leal, Philippe Mathieu-Daudé,
	Cornelia Huck, Thomas Huth, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Cleber Rosa, Alex Bennée, Aurelien Jarno, Eduardo Habkost

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



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 7/8] tests/acceptance/migration.py: cancel test on s390x
  2021-04-15 21:51 [PATCH 0/8] Tests: introduce custom jobs Cleber Rosa
                   ` (5 preceding siblings ...)
  2021-04-15 21:51 ` [PATCH 6/8] Acceptance tests: prevent shutdown on non-specific target tests Cleber Rosa
@ 2021-04-15 21:51 ` Cleber Rosa
  2021-04-19 19:11   ` Wainer dos Santos Moschetta
  2021-04-19 19:35   ` Willian Rampazzo
  2021-04-15 21:51 ` [PATCH 8/8] Tests: add custom test jobs Cleber Rosa
  2021-04-16 16:22 ` [PATCH 0/8] Tests: introduce custom jobs Paolo Bonzini
  8 siblings, 2 replies; 37+ messages in thread
From: Cleber Rosa @ 2021-04-15 21:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Aleksandar Rikalo, Beraldo Leal, Philippe Mathieu-Daudé,
	Cornelia Huck, Thomas Huth, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Cleber Rosa, Alex Bennée, Aurelien Jarno, Eduardo Habkost

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



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 8/8] Tests: add custom test jobs
  2021-04-15 21:51 [PATCH 0/8] Tests: introduce custom jobs Cleber Rosa
                   ` (6 preceding siblings ...)
  2021-04-15 21:51 ` [PATCH 7/8] tests/acceptance/migration.py: cancel test on s390x Cleber Rosa
@ 2021-04-15 21:51 ` Cleber Rosa
  2021-04-16  5:23   ` Philippe Mathieu-Daudé
  2021-04-16 16:22 ` [PATCH 0/8] Tests: introduce custom jobs Paolo Bonzini
  8 siblings, 1 reply; 37+ messages in thread
From: Cleber Rosa @ 2021-04-15 21:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Aleksandar Rikalo, Beraldo Leal, Philippe Mathieu-Daudé,
	Cornelia Huck, Thomas Huth, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Cleber Rosa, Alex Bennée, Aurelien Jarno, Eduardo Habkost

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



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCH 1/8] Acceptance Jobs: preserve the cache for pip on GitLab CI
  2021-04-15 21:51 ` [PATCH 1/8] Acceptance Jobs: preserve the cache for pip on GitLab CI Cleber Rosa
@ 2021-04-16  3:56   ` Thomas Huth
  2021-04-16 15:39     ` Cleber Rosa
  0 siblings, 1 reply; 37+ messages in thread
From: Thomas Huth @ 2021-04-16  3:56 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Aleksandar Rikalo, Beraldo Leal, Philippe Mathieu-Daudé,
	Cornelia Huck, Philippe Mathieu-Daudé,
	Wainer dos Santos Moschetta, Willian Rampazzo, Auger Eric,
	qemu-s390x, Willian Rampazzo, Alex Bennée, Aurelien Jarno,
	Eduardo Habkost

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



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 2/8] Acceptance tests: do not try to reuse packages from the system
  2021-04-15 21:51 ` [PATCH 2/8] Acceptance tests: do not try to reuse packages from the system Cleber Rosa
@ 2021-04-16  5:07   ` Philippe Mathieu-Daudé
  2021-04-16 15:39   ` Willian Rampazzo
  2021-04-19 18:14   ` Wainer dos Santos Moschetta
  2 siblings, 0 replies; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-16  5:07 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Aleksandar Rikalo, Beraldo Leal, Cornelia Huck, Thomas Huth,
	Wainer dos Santos Moschetta, Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Alex Bennée, Aurelien Jarno, Eduardo Habkost

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>



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 4/8] tests/acceptance/migration.py: cancel test if migration is not supported
  2021-04-15 21:51 ` [PATCH 4/8] tests/acceptance/migration.py: cancel test if migration is not supported Cleber Rosa
@ 2021-04-16  5:11   ` Philippe Mathieu-Daudé
  2021-04-16 16:14     ` Cleber Rosa
  2021-04-16 15:50   ` Willian Rampazzo
  2021-04-19 18:46   ` Wainer dos Santos Moschetta
  2 siblings, 1 reply; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-16  5:11 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Aleksandar Rikalo, Beraldo Leal, Cornelia Huck, Thomas Huth,
	Wainer dos Santos Moschetta, Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Alex Bennée, Aurelien Jarno, Eduardo Habkost

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.



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels
  2021-04-15 21:51 ` [PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels Cleber Rosa
@ 2021-04-16  5:15   ` Philippe Mathieu-Daudé
  2021-04-16 15:54     ` Willian Rampazzo
  2021-04-16 16:17     ` Cleber Rosa
  2021-04-16 15:54   ` Willian Rampazzo
  2021-04-19 18:56   ` Wainer dos Santos Moschetta
  2 siblings, 2 replies; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-16  5:15 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Aleksandar Rikalo, Beraldo Leal, Cornelia Huck, Thomas Huth,
	Wainer dos Santos Moschetta, Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Alex Bennée, Aurelien Jarno, Eduardo Habkost

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:
> 



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 8/8] Tests: add custom test jobs
  2021-04-15 21:51 ` [PATCH 8/8] Tests: add custom test jobs Cleber Rosa
@ 2021-04-16  5:23   ` Philippe Mathieu-Daudé
  2021-04-16 16:25     ` Cleber Rosa
  0 siblings, 1 reply; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-16  5:23 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Aleksandar Rikalo, Beraldo Leal, Cornelia Huck, Thomas Huth,
	Wainer dos Santos Moschetta, Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Alex Bennée, Aurelien Jarno, Eduardo Habkost

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.



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp
  2021-04-15 21:51 ` [PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp Cleber Rosa
@ 2021-04-16  5:26   ` Philippe Mathieu-Daudé
  2021-04-16 15:43     ` Cleber Rosa
  2021-04-16 15:46     ` Willian Rampazzo
  2021-04-16 15:41   ` Willian Rampazzo
  1 sibling, 2 replies; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-16  5:26 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Aleksandar Rikalo, Beraldo Leal, Cornelia Huck, Thomas Huth,
	Wainer dos Santos Moschetta, Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Alex Bennée, Aurelien Jarno, Eduardo Habkost

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')
> 



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 1/8] Acceptance Jobs: preserve the cache for pip on GitLab CI
  2021-04-16  3:56   ` Thomas Huth
@ 2021-04-16 15:39     ` Cleber Rosa
  0 siblings, 0 replies; 37+ messages in thread
From: Cleber Rosa @ 2021-04-16 15:39 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Aleksandar Rikalo, Beraldo Leal, Philippe Mathieu-Daudé,
	Cornelia Huck, qemu-devel, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Alex Bennée, Aurelien Jarno, Eduardo Habkost

[-- 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 --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 2/8] Acceptance tests: do not try to reuse packages from the system
  2021-04-15 21:51 ` [PATCH 2/8] Acceptance tests: do not try to reuse packages from the system Cleber Rosa
  2021-04-16  5:07   ` Philippe Mathieu-Daudé
@ 2021-04-16 15:39   ` Willian Rampazzo
  2021-04-19 18:14   ` Wainer dos Santos Moschetta
  2 siblings, 0 replies; 37+ messages in thread
From: Willian Rampazzo @ 2021-04-16 15:39 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Aleksandar Rikalo, Beraldo Leal, Philippe Mathieu-Daudé,
	Cornelia Huck, qemu-devel, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Auger Eric, open list:S390 Virtio-ccw, Thomas Huth,
	Alex Bennée, Aurelien Jarno, Eduardo Habkost

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>



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp
  2021-04-15 21:51 ` [PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp Cleber Rosa
  2021-04-16  5:26   ` Philippe Mathieu-Daudé
@ 2021-04-16 15:41   ` Willian Rampazzo
  1 sibling, 0 replies; 37+ messages in thread
From: Willian Rampazzo @ 2021-04-16 15:41 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Aleksandar Rikalo, Beraldo Leal, Philippe Mathieu-Daudé,
	Cornelia Huck, qemu-devel, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Auger Eric, open list:S390 Virtio-ccw, Thomas Huth,
	Alex Bennée, Aurelien Jarno, Eduardo Habkost

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>



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp
  2021-04-16  5:26   ` Philippe Mathieu-Daudé
@ 2021-04-16 15:43     ` Cleber Rosa
  2021-04-16 17:46       ` Philippe Mathieu-Daudé
  2021-04-16 15:46     ` Willian Rampazzo
  1 sibling, 1 reply; 37+ messages in thread
From: Cleber Rosa @ 2021-04-16 15:43 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Thomas Huth, Beraldo Leal, Cornelia Huck, Aleksandar Rikalo,
	Wainer dos Santos Moschetta, qemu-devel, Willian Rampazzo,
	Auger Eric, qemu-s390x, Willian Rampazzo, Alex Bennée,
	Aurelien Jarno, Eduardo Habkost

[-- 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 --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp
  2021-04-16  5:26   ` Philippe Mathieu-Daudé
  2021-04-16 15:43     ` Cleber Rosa
@ 2021-04-16 15:46     ` Willian Rampazzo
  1 sibling, 0 replies; 37+ messages in thread
From: Willian Rampazzo @ 2021-04-16 15:46 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Aleksandar Rikalo, Beraldo Leal, Cornelia Huck, qemu-devel,
	Wainer dos Santos Moschetta, Auger Eric,
	open list:S390 Virtio-ccw, Cleber Rosa, Thomas Huth,
	Alex Bennée, Aurelien Jarno, Eduardo Habkost

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.



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 4/8] tests/acceptance/migration.py: cancel test if migration is not supported
  2021-04-15 21:51 ` [PATCH 4/8] tests/acceptance/migration.py: cancel test if migration is not supported Cleber Rosa
  2021-04-16  5:11   ` Philippe Mathieu-Daudé
@ 2021-04-16 15:50   ` Willian Rampazzo
  2021-04-19 18:46   ` Wainer dos Santos Moschetta
  2 siblings, 0 replies; 37+ messages in thread
From: Willian Rampazzo @ 2021-04-16 15:50 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Aleksandar Rikalo, Beraldo Leal, Philippe Mathieu-Daudé,
	Cornelia Huck, qemu-devel, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Auger Eric, open list:S390 Virtio-ccw, Thomas Huth,
	Alex Bennée, Aurelien Jarno, Eduardo Habkost

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>



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels
  2021-04-16  5:15   ` Philippe Mathieu-Daudé
@ 2021-04-16 15:54     ` Willian Rampazzo
  2021-04-16 16:17     ` Cleber Rosa
  1 sibling, 0 replies; 37+ messages in thread
From: Willian Rampazzo @ 2021-04-16 15:54 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Aleksandar Rikalo, Beraldo Leal, Cornelia Huck, qemu-devel,
	Wainer dos Santos Moschetta, Philippe Mathieu-Daudé,
	Auger Eric, open list:S390 Virtio-ccw, Cleber Rosa, Thomas Huth,
	Alex Bennée, Aurelien Jarno, Eduardo Habkost

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.



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels
  2021-04-15 21:51 ` [PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels Cleber Rosa
  2021-04-16  5:15   ` Philippe Mathieu-Daudé
@ 2021-04-16 15:54   ` Willian Rampazzo
  2021-04-19 18:56   ` Wainer dos Santos Moschetta
  2 siblings, 0 replies; 37+ messages in thread
From: Willian Rampazzo @ 2021-04-16 15:54 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Aleksandar Rikalo, Beraldo Leal, Philippe Mathieu-Daudé,
	Cornelia Huck, qemu-devel, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Auger Eric, open list:S390 Virtio-ccw, Thomas Huth,
	Alex Bennée, Aurelien Jarno, Eduardo Habkost

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>



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 6/8] Acceptance tests: prevent shutdown on non-specific target tests
  2021-04-15 21:51 ` [PATCH 6/8] Acceptance tests: prevent shutdown on non-specific target tests Cleber Rosa
@ 2021-04-16 15:56   ` Willian Rampazzo
  2021-04-19 19:07   ` Wainer dos Santos Moschetta
  1 sibling, 0 replies; 37+ messages in thread
From: Willian Rampazzo @ 2021-04-16 15:56 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Aleksandar Rikalo, Beraldo Leal, Philippe Mathieu-Daudé,
	Cornelia Huck, qemu-devel, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Auger Eric, open list:S390 Virtio-ccw, Thomas Huth,
	Alex Bennée, Aurelien Jarno, Eduardo Habkost

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>



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 4/8] tests/acceptance/migration.py: cancel test if migration is not supported
  2021-04-16  5:11   ` Philippe Mathieu-Daudé
@ 2021-04-16 16:14     ` Cleber Rosa
  0 siblings, 0 replies; 37+ messages in thread
From: Cleber Rosa @ 2021-04-16 16:14 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Thomas Huth, Beraldo Leal, Cornelia Huck, Aleksandar Rikalo,
	Wainer dos Santos Moschetta, qemu-devel, Willian Rampazzo,
	Auger Eric, qemu-s390x, Willian Rampazzo, Eduardo Habkost,
	Alex Bennée, Aurelien Jarno, Philippe Mathieu-Daudé

[-- 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 --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels
  2021-04-16  5:15   ` Philippe Mathieu-Daudé
  2021-04-16 15:54     ` Willian Rampazzo
@ 2021-04-16 16:17     ` Cleber Rosa
  1 sibling, 0 replies; 37+ messages in thread
From: Cleber Rosa @ 2021-04-16 16:17 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Aleksandar Rikalo, Beraldo Leal, Cornelia Huck, qemu-devel,
	Wainer dos Santos Moschetta, Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Thomas Huth, Alex Bennée, Aurelien Jarno, Eduardo Habkost

[-- 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 --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 0/8] Tests: introduce custom jobs
  2021-04-15 21:51 [PATCH 0/8] Tests: introduce custom jobs Cleber Rosa
                   ` (7 preceding siblings ...)
  2021-04-15 21:51 ` [PATCH 8/8] Tests: add custom test jobs Cleber Rosa
@ 2021-04-16 16:22 ` Paolo Bonzini
  2021-04-16 16:42   ` Cleber Rosa
  8 siblings, 1 reply; 37+ messages in thread
From: Paolo Bonzini @ 2021-04-16 16:22 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Thomas Huth, Beraldo Leal, Alex Bennée, Cornelia Huck,
	Aleksandar Rikalo, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

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



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 8/8] Tests: add custom test jobs
  2021-04-16  5:23   ` Philippe Mathieu-Daudé
@ 2021-04-16 16:25     ` Cleber Rosa
  0 siblings, 0 replies; 37+ messages in thread
From: Cleber Rosa @ 2021-04-16 16:25 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Aleksandar Rikalo, Beraldo Leal, Cornelia Huck, qemu-devel,
	Wainer dos Santos Moschetta, Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Thomas Huth, Alex Bennée, Aurelien Jarno, Eduardo Habkost

[-- 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 --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 0/8] Tests: introduce custom jobs
  2021-04-16 16:22 ` [PATCH 0/8] Tests: introduce custom jobs Paolo Bonzini
@ 2021-04-16 16:42   ` Cleber Rosa
  0 siblings, 0 replies; 37+ messages in thread
From: Cleber Rosa @ 2021-04-16 16:42 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Thomas Huth, Beraldo Leal, Alex Bennée, Cornelia Huck,
	Aleksandar Rikalo, Wainer dos Santos Moschetta, qemu-devel,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Eduardo Habkost, Philippe Mathieu-Daudé,
	Aurelien Jarno, Philippe Mathieu-Daudé

[-- 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 --]

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp
  2021-04-16 15:43     ` Cleber Rosa
@ 2021-04-16 17:46       ` Philippe Mathieu-Daudé
  2021-04-19 18:25         ` Wainer dos Santos Moschetta
  0 siblings, 1 reply; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-16 17:46 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Thomas Huth, Beraldo Leal, Cornelia Huck, Aleksandar Rikalo,
	Wainer dos Santos Moschetta, qemu-devel, Willian Rampazzo,
	Auger Eric, qemu-s390x, Willian Rampazzo, Alex Bennée,
	Aurelien Jarno, Eduardo Habkost

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.


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 2/8] Acceptance tests: do not try to reuse packages from the system
  2021-04-15 21:51 ` [PATCH 2/8] Acceptance tests: do not try to reuse packages from the system Cleber Rosa
  2021-04-16  5:07   ` Philippe Mathieu-Daudé
  2021-04-16 15:39   ` Willian Rampazzo
@ 2021-04-19 18:14   ` Wainer dos Santos Moschetta
  2 siblings, 0 replies; 37+ messages in thread
From: Wainer dos Santos Moschetta @ 2021-04-19 18:14 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Thomas Huth, Beraldo Leal, Alex Bennée, Cornelia Huck,
	Aleksandar Rikalo, Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost


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), \



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp
  2021-04-16 17:46       ` Philippe Mathieu-Daudé
@ 2021-04-19 18:25         ` Wainer dos Santos Moschetta
  0 siblings, 0 replies; 37+ messages in thread
From: Wainer dos Santos Moschetta @ 2021-04-19 18:25 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Cleber Rosa
  Cc: Thomas Huth, Beraldo Leal, Cornelia Huck, Aleksandar Rikalo,
	qemu-devel, Willian Rampazzo, Auger Eric, qemu-s390x,
	Willian Rampazzo, Alex Bennée, Aurelien Jarno,
	Eduardo Habkost


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.
>



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 4/8] tests/acceptance/migration.py: cancel test if migration is not supported
  2021-04-15 21:51 ` [PATCH 4/8] tests/acceptance/migration.py: cancel test if migration is not supported Cleber Rosa
  2021-04-16  5:11   ` Philippe Mathieu-Daudé
  2021-04-16 15:50   ` Willian Rampazzo
@ 2021-04-19 18:46   ` Wainer dos Santos Moschetta
  2 siblings, 0 replies; 37+ messages in thread
From: Wainer dos Santos Moschetta @ 2021-04-19 18:46 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Thomas Huth, Beraldo Leal, Alex Bennée, Cornelia Huck,
	Aleksandar Rikalo, Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

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):



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels
  2021-04-15 21:51 ` [PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels Cleber Rosa
  2021-04-16  5:15   ` Philippe Mathieu-Daudé
  2021-04-16 15:54   ` Willian Rampazzo
@ 2021-04-19 18:56   ` Wainer dos Santos Moschetta
  2 siblings, 0 replies; 37+ messages in thread
From: Wainer dos Santos Moschetta @ 2021-04-19 18:56 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Thomas Huth, Beraldo Leal, Alex Bennée, Cornelia Huck,
	Aleksandar Rikalo, Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

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>




^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 6/8] Acceptance tests: prevent shutdown on non-specific target tests
  2021-04-15 21:51 ` [PATCH 6/8] Acceptance tests: prevent shutdown on non-specific target tests Cleber Rosa
  2021-04-16 15:56   ` Willian Rampazzo
@ 2021-04-19 19:07   ` Wainer dos Santos Moschetta
  1 sibling, 0 replies; 37+ messages in thread
From: Wainer dos Santos Moschetta @ 2021-04-19 19:07 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Thomas Huth, Beraldo Leal, Alex Bennée, Cornelia Huck,
	Aleksandar Rikalo, Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Philippe Mathieu-Daudé,
	Aurelien Jarno, Eduardo Habkost

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')



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 7/8] tests/acceptance/migration.py: cancel test on s390x
  2021-04-15 21:51 ` [PATCH 7/8] tests/acceptance/migration.py: cancel test on s390x Cleber Rosa
@ 2021-04-19 19:11   ` Wainer dos Santos Moschetta
  2021-04-19 19:35   ` Willian Rampazzo
  1 sibling, 0 replies; 37+ messages in thread
From: Wainer dos Santos Moschetta @ 2021-04-19 19:11 UTC (permalink / raw)
  To: Cleber Rosa, qemu-devel
  Cc: Aleksandar Rikalo, Beraldo Leal, Philippe Mathieu-Daudé,
	Cornelia Huck, Thomas Huth, Philippe Mathieu-Daudé,
	Willian Rampazzo, Auger Eric, qemu-s390x, Willian Rampazzo,
	Alex Bennée, Aurelien Jarno, Eduardo Habkost


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()



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 7/8] tests/acceptance/migration.py: cancel test on s390x
  2021-04-15 21:51 ` [PATCH 7/8] tests/acceptance/migration.py: cancel test on s390x Cleber Rosa
  2021-04-19 19:11   ` Wainer dos Santos Moschetta
@ 2021-04-19 19:35   ` Willian Rampazzo
  1 sibling, 0 replies; 37+ messages in thread
From: Willian Rampazzo @ 2021-04-19 19:35 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: Aleksandar Rikalo, Beraldo Leal, Philippe Mathieu-Daudé,
	Cornelia Huck, qemu-devel, Wainer dos Santos Moschetta,
	Philippe Mathieu-Daudé,
	Auger Eric, open list:S390 Virtio-ccw, Thomas Huth,
	Alex Bennée, Aurelien Jarno, Eduardo Habkost

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>



^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2021-04-19 20:22 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-15 21:51 [PATCH 0/8] Tests: introduce custom jobs Cleber Rosa
2021-04-15 21:51 ` [PATCH 1/8] Acceptance Jobs: preserve the cache for pip on GitLab CI Cleber Rosa
2021-04-16  3:56   ` Thomas Huth
2021-04-16 15:39     ` Cleber Rosa
2021-04-15 21:51 ` [PATCH 2/8] Acceptance tests: do not try to reuse packages from the system Cleber Rosa
2021-04-16  5:07   ` Philippe Mathieu-Daudé
2021-04-16 15:39   ` Willian Rampazzo
2021-04-19 18:14   ` Wainer dos Santos Moschetta
2021-04-15 21:51 ` [PATCH 3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp Cleber Rosa
2021-04-16  5:26   ` Philippe Mathieu-Daudé
2021-04-16 15:43     ` Cleber Rosa
2021-04-16 17:46       ` Philippe Mathieu-Daudé
2021-04-19 18:25         ` Wainer dos Santos Moschetta
2021-04-16 15:46     ` Willian Rampazzo
2021-04-16 15:41   ` Willian Rampazzo
2021-04-15 21:51 ` [PATCH 4/8] tests/acceptance/migration.py: cancel test if migration is not supported Cleber Rosa
2021-04-16  5:11   ` Philippe Mathieu-Daudé
2021-04-16 16:14     ` Cleber Rosa
2021-04-16 15:50   ` Willian Rampazzo
2021-04-19 18:46   ` Wainer dos Santos Moschetta
2021-04-15 21:51 ` [PATCH 5/8] tests/acceptance/cpu_queries.py: use the proper logging channels Cleber Rosa
2021-04-16  5:15   ` Philippe Mathieu-Daudé
2021-04-16 15:54     ` Willian Rampazzo
2021-04-16 16:17     ` Cleber Rosa
2021-04-16 15:54   ` Willian Rampazzo
2021-04-19 18:56   ` Wainer dos Santos Moschetta
2021-04-15 21:51 ` [PATCH 6/8] Acceptance tests: prevent shutdown on non-specific target tests Cleber Rosa
2021-04-16 15:56   ` Willian Rampazzo
2021-04-19 19:07   ` Wainer dos Santos Moschetta
2021-04-15 21:51 ` [PATCH 7/8] tests/acceptance/migration.py: cancel test on s390x Cleber Rosa
2021-04-19 19:11   ` Wainer dos Santos Moschetta
2021-04-19 19:35   ` Willian Rampazzo
2021-04-15 21:51 ` [PATCH 8/8] Tests: add custom test jobs Cleber Rosa
2021-04-16  5:23   ` Philippe Mathieu-Daudé
2021-04-16 16:25     ` Cleber Rosa
2021-04-16 16:22 ` [PATCH 0/8] Tests: introduce custom jobs Paolo Bonzini
2021-04-16 16:42   ` Cleber Rosa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).