[PATCH v2 0/7] tests/acceptance: Handle tests with "cpu" tag

Wainer dos Santos Moschetta posted 7 patches 3 years ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210408195237.3489296-1-wainersm@redhat.com
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, Aurelien Jarno <aurelien@aurel32.net>, Paolo Bonzini <pbonzini@redhat.com>, Wainer dos Santos Moschetta <wainersm@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, John Snow <jsnow@redhat.com>, Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, Cleber Rosa <crosa@redhat.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>
There is a newer version of this series
docs/devel/testing.rst                     | 17 +++++++++
python/qemu/machine.py                     |  5 +++
tests/acceptance/avocado_qemu/__init__.py  | 21 ++++++++++++
tests/acceptance/boot_linux.py             |  3 --
tests/acceptance/boot_linux_console.py     | 16 +++++----
tests/acceptance/machine_mips_malta.py     |  7 ++--
tests/acceptance/pc_cpu_hotplug_props.py   |  2 +-
tests/acceptance/replay_kernel.py          | 17 ++++-----
tests/acceptance/reverse_debugging.py      |  2 +-
tests/acceptance/tcg_plugins.py            | 15 ++++----
tests/acceptance/virtio-gpu.py             |  4 +--
tests/acceptance/x86_cpu_model_versions.py | 40 +++++++++++++++++-----
12 files changed, 107 insertions(+), 42 deletions(-)
[PATCH v2 0/7] tests/acceptance: Handle tests with "cpu" tag
Posted by Wainer dos Santos Moschetta 3 years ago
Currently the acceptance tests tagged with "machine" have the "-M TYPE"
automatically added to the list of arguments of the QEMUMachine object.
In other words, that option is passed to the launched QEMU. On this
series it is implemented the same feature but instead for tests marked
with "cpu".

There is a caveat, however, in case the test needs additional arguments to
the CPU type they cannot be passed via tag, because the tags parser split
values by comma. For example, in tests/acceptance/x86_cpu_model_versions.py,
there are cases where:

  * -cpu is set to "Cascadelake-Server,x-force-features=on,check=off,enforce=off"
  * if it was tagged like "cpu:Cascadelake-Server,x-force-features=on,check=off,enforce=off"
    then the parser would break it into 4 tags ("cpu:Cascadelake-Server",
    "x-force-features=on", "check=off", "enforce=off")
  * resulting on "-cpu Cascadelake-Server" and the remaining arguments are ignored.

It was introduced the avocado_qemu.Test.set_vm_arg() method to deal with
cases like the example above, so that one can tag it as "cpu:Cascadelake-Server"
AND call self.set_vm_args('-cpu', "Cascadelake-Server,x-force-features=on,check=off,enforce=off"),
and that results on the reset of the initial value of -cpu.

This series was tested on CI (https://gitlab.com/wainersm/qemu/-/pipelines/277376246)
and with the following code:

from avocado_qemu import Test

class CPUTest(Test):
    def test_cpu(self):
        """
        :avocado: tags=cpu:host
        """
        # The cpu property is set to the tag value, or None on its absence
        self.assertEqual(self.cpu, "host")
        # The created VM has the '-cpu host' option
        self.assertIn("-cpu host", " ".join(self.vm._args))
        self.vm.launch()

    def test_cpu_none(self):
        self.assertEqual(self.cpu, None)
        self.assertNotIn('-cpu', self.vm._args)

    def test_cpu_reset(self):
        """
        :avocado: tags=cpu:host
        """
        self.assertIn("-cpu host", " ".join(self.vm._args))
        self.set_vm_arg("-cpu", "Cascadelake-Server,x-force-features=on")
        self.assertNotIn("-cpu host", " ".join(self.vm._args))
        self.assertIn("-cpu Cascadelake-Server,x-force-features=on", " ".join(self.vm._args))

Changes:
 - v1 -> v2:
   - Recognize the cpu value passed via test parameter [crosa]
   - Fixed tags (patch 02) on preparation to patch 03 [crosa]
   - Added QEMUMachine.args property (patch 04) so that _args could be handled
     without pylint complaining (protected property) 
   - Added Test.set_vm_arg() (patch 05) to handle the corner case [crosa]

Wainer dos Santos Moschetta (7):
  tests/acceptance: Automatic set -cpu to the test vm
  tests/acceptance: Fix mismatch on cpu tagged tests
  tests/acceptance: Let the framework handle "cpu:VALUE" tagged tests
  tests/acceptance: Tagging tests with "cpu:VALUE"
  python/qemu: Add args property to the QEMUMachine class
  tests/acceptance: Add set_vm_arg() to the Test class
  tests/acceptance: Handle cpu tag on x86_cpu_model_versions tests

 docs/devel/testing.rst                     | 17 +++++++++
 python/qemu/machine.py                     |  5 +++
 tests/acceptance/avocado_qemu/__init__.py  | 21 ++++++++++++
 tests/acceptance/boot_linux.py             |  3 --
 tests/acceptance/boot_linux_console.py     | 16 +++++----
 tests/acceptance/machine_mips_malta.py     |  7 ++--
 tests/acceptance/pc_cpu_hotplug_props.py   |  2 +-
 tests/acceptance/replay_kernel.py          | 17 ++++-----
 tests/acceptance/reverse_debugging.py      |  2 +-
 tests/acceptance/tcg_plugins.py            | 15 ++++----
 tests/acceptance/virtio-gpu.py             |  4 +--
 tests/acceptance/x86_cpu_model_versions.py | 40 +++++++++++++++++-----
 12 files changed, 107 insertions(+), 42 deletions(-)

-- 
2.29.2


Re: [PATCH v2 0/7] tests/acceptance: Handle tests with "cpu" tag
Posted by Cleber Rosa 3 years ago
On Thu, Apr 08, 2021 at 04:52:30PM -0300, Wainer dos Santos Moschetta wrote:
> Currently the acceptance tests tagged with "machine" have the "-M TYPE"
> automatically added to the list of arguments of the QEMUMachine object.
> In other words, that option is passed to the launched QEMU. On this
> series it is implemented the same feature but instead for tests marked
> with "cpu".
> 
> There is a caveat, however, in case the test needs additional arguments to
> the CPU type they cannot be passed via tag, because the tags parser split
> values by comma. For example, in tests/acceptance/x86_cpu_model_versions.py,
> there are cases where:

Hi Wainer,

I've created an Avocado issue to hopefully get rid of this limitation:

   https://github.com/avocado-framework/avocado/issues/4541

> 
>   * -cpu is set to "Cascadelake-Server,x-force-features=on,check=off,enforce=off"
>   * if it was tagged like "cpu:Cascadelake-Server,x-force-features=on,check=off,enforce=off"
>     then the parser would break it into 4 tags ("cpu:Cascadelake-Server",
>     "x-force-features=on", "check=off", "enforce=off")
>   * resulting on "-cpu Cascadelake-Server" and the remaining arguments are ignored.
> 
> It was introduced the avocado_qemu.Test.set_vm_arg() method to deal with
> cases like the example above, so that one can tag it as "cpu:Cascadelake-Server"
> AND call self.set_vm_args('-cpu', "Cascadelake-Server,x-force-features=on,check=off,enforce=off"),
> and that results on the reset of the initial value of -cpu.
>

So for now this seems reasonable enough.

> This series was tested on CI (https://gitlab.com/wainersm/qemu/-/pipelines/277376246)
> and with the following code:
> 
> from avocado_qemu import Test
> 
> class CPUTest(Test):
>     def test_cpu(self):
>         """
>         :avocado: tags=cpu:host
>         """
>         # The cpu property is set to the tag value, or None on its absence
>         self.assertEqual(self.cpu, "host")
>         # The created VM has the '-cpu host' option
>         self.assertIn("-cpu host", " ".join(self.vm._args))
>         self.vm.launch()
> 
>     def test_cpu_none(self):
>         self.assertEqual(self.cpu, None)
>         self.assertNotIn('-cpu', self.vm._args)
> 
>     def test_cpu_reset(self):
>         """
>         :avocado: tags=cpu:host
>         """
>         self.assertIn("-cpu host", " ".join(self.vm._args))
>         self.set_vm_arg("-cpu", "Cascadelake-Server,x-force-features=on")
>         self.assertNotIn("-cpu host", " ".join(self.vm._args))
>         self.assertIn("-cpu Cascadelake-Server,x-force-features=on", " ".join(self.vm._args))
>

We should not let this type of testing go to waste, so it's about time
to set aside a directory for tests that are about the framework,
rather than end user functionality.  I'll take a look at that.

Cheers,
- Cleber.