[Qemu-devel] [PATCH 0/2] tests: Avocado-test for the SmartFusion2 board

[Qemu-devel] [PATCH 0/2] tests: Avocado-test for the SmartFusion2 board

[Philippe Mathieu-Daudé]

Hi,

I added test guide lines from Subbaraya Sundeep [*] to avoid this
board to bitrot.

Regards,

Phil.

[*] https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg03810.html

Philippe Mathieu-Daudé (2):
  BootLinuxConsoleTest: Do not log empty lines
  BootLinuxConsoleTest: Test the SmartFusion2 board

 tests/acceptance/boot_linux_console.py | 33 ++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

-- 
2.19.1

[Qemu-devel] [PATCH 1/2] BootLinuxConsoleTest: Do not log empty lines

[Philippe Mathieu-Daudé]

Avoid to log empty lines in console debug logs.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/acceptance/boot_linux_console.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index d5c500ea30..f593f3858e 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -37,8 +37,10 @@ class BootLinuxConsole(Test):
         console = self.vm.console_socket.makefile()
         console_logger = logging.getLogger('console')
         while True:
-            msg = console.readline()
-            console_logger.debug(msg.strip())
+            msg = console.readline().strip()
+            if not msg:
+                continue
+            console_logger.debug(msg)
             if success_message in msg:
                 break
             if failure_message in msg:
-- 
2.19.1

[Qemu-devel] [PATCH 2/2] BootLinuxConsoleTest: Test the SmartFusion2 board

[Philippe Mathieu-Daudé]

Similar to the x86_64/pc test, it boots a Linux kernel on an
Emcraft board and verify the serial is working.

If ARM is a target being built, "make check-acceptance" will
automatically include this test by the use of the "arch:arm" tags.

Alternatively, this test can be run using:

  $ avocado run -t arch:arm tests/acceptance
  $ avocado run -t machine:emcraft-sf2 tests/acceptance

Based on the recommended test setup from Subbaraya Sundeep:
https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg03810.html

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/acceptance/boot_linux_console.py | 27 ++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index f593f3858e..844cb80bb5 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -178,6 +178,33 @@ class BootLinuxConsole(Test):
         console_pattern = 'Kernel command line: %s' % kernel_command_line
         self.wait_for_console_pattern(console_pattern)
 
+    def test_arm_emcraft_sf2(self):
+        """
+        :avocado: tags=arch:arm
+        :avocado: tags=machine:emcraft-sf2
+        :avocado: tags=endian:little
+        """
+        uboot_url = ('https://raw.githubusercontent.com/'
+                     'Subbaraya-Sundeep/qemu-test-binaries/'
+                     'fa030bd77a014a0b8e360d3b7011df89283a2f0b/u-boot')
+        uboot_hash = 'abba5d9c24cdd2d49cdc2a8aa92976cf20737eff'
+        uboot_path = self.fetch_asset(uboot_url, asset_hash=uboot_hash)
+        spi_url = ('https://raw.githubusercontent.com/'
+                   'Subbaraya-Sundeep/qemu-test-binaries/'
+                   'fa030bd77a014a0b8e360d3b7011df89283a2f0b/spi.bin')
+        spi_hash = '85f698329d38de63aea6e884a86fbde70890a78a'
+        spi_path = self.fetch_asset(spi_url, asset_hash=spi_hash)
+
+        self.vm.set_machine('emcraft-sf2')
+        self.vm.set_console()
+        kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE
+        self.vm.add_args('-kernel', uboot_path,
+                         '-append', kernel_command_line,
+                         '-drive', 'file=' + spi_path + ',if=mtd,format=raw',
+                         '-no-reboot')
+        self.vm.launch()
+        self.wait_for_console_pattern('init started: BusyBox')
+
     def test_s390x_s390_ccw_virtio(self):
         """
         :avocado: tags=arch:s390x
-- 
2.19.1

Re: [Qemu-devel] [PATCH 1/2] BootLinuxConsoleTest: Do not log empty lines

[Alistair Francis]

On Mon, May 20, 2019 at 3:07 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Avoid to log empty lines in console debug logs.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  tests/acceptance/boot_linux_console.py | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
> index d5c500ea30..f593f3858e 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -37,8 +37,10 @@ class BootLinuxConsole(Test):
>          console = self.vm.console_socket.makefile()
>          console_logger = logging.getLogger('console')
>          while True:
> -            msg = console.readline()
> -            console_logger.debug(msg.strip())
> +            msg = console.readline().strip()
> +            if not msg:
> +                continue
> +            console_logger.debug(msg)
>              if success_message in msg:
>                  break
>              if failure_message in msg:
> --
> 2.19.1
>
>

Re: [Qemu-devel] [PATCH 2/2] BootLinuxConsoleTest: Test the SmartFusion2 board

[Alistair Francis]

On Mon, May 20, 2019 at 3:09 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Similar to the x86_64/pc test, it boots a Linux kernel on an
> Emcraft board and verify the serial is working.
>
> If ARM is a target being built, "make check-acceptance" will
> automatically include this test by the use of the "arch:arm" tags.
>
> Alternatively, this test can be run using:
>
>   $ avocado run -t arch:arm tests/acceptance
>   $ avocado run -t machine:emcraft-sf2 tests/acceptance
>
> Based on the recommended test setup from Subbaraya Sundeep:
> https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg03810.html
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  tests/acceptance/boot_linux_console.py | 27 ++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
> index f593f3858e..844cb80bb5 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -178,6 +178,33 @@ class BootLinuxConsole(Test):
>          console_pattern = 'Kernel command line: %s' % kernel_command_line
>          self.wait_for_console_pattern(console_pattern)
>
> +    def test_arm_emcraft_sf2(self):
> +        """
> +        :avocado: tags=arch:arm
> +        :avocado: tags=machine:emcraft-sf2
> +        :avocado: tags=endian:little
> +        """
> +        uboot_url = ('https://raw.githubusercontent.com/'
> +                     'Subbaraya-Sundeep/qemu-test-binaries/'
> +                     'fa030bd77a014a0b8e360d3b7011df89283a2f0b/u-boot')
> +        uboot_hash = 'abba5d9c24cdd2d49cdc2a8aa92976cf20737eff'
> +        uboot_path = self.fetch_asset(uboot_url, asset_hash=uboot_hash)
> +        spi_url = ('https://raw.githubusercontent.com/'
> +                   'Subbaraya-Sundeep/qemu-test-binaries/'
> +                   'fa030bd77a014a0b8e360d3b7011df89283a2f0b/spi.bin')
> +        spi_hash = '85f698329d38de63aea6e884a86fbde70890a78a'
> +        spi_path = self.fetch_asset(spi_url, asset_hash=spi_hash)
> +
> +        self.vm.set_machine('emcraft-sf2')
> +        self.vm.set_console()
> +        kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE
> +        self.vm.add_args('-kernel', uboot_path,
> +                         '-append', kernel_command_line,
> +                         '-drive', 'file=' + spi_path + ',if=mtd,format=raw',
> +                         '-no-reboot')
> +        self.vm.launch()
> +        self.wait_for_console_pattern('init started: BusyBox')
> +
>      def test_s390x_s390_ccw_virtio(self):
>          """
>          :avocado: tags=arch:s390x
> --
> 2.19.1
>
>

Re: [Qemu-devel] [PATCH 1/2] BootLinuxConsoleTest: Do not log empty lines

[Cleber Rosa]

----- Original Message -----
> From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
> To: qemu-devel@nongnu.org
> Cc: "Eduardo Habkost" <ehabkost@redhat.com>, "Caio Carrara" <ccarrara@redhat.com>, "Alistair Francis"
> <alistair@alistair23.me>, "Subbaraya Sundeep" <sundeep.lkml@gmail.com>, qemu-arm@nongnu.org, "Cleber Rosa"
> <crosa@redhat.com>, "Peter Maydell" <peter.maydell@linaro.org>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>
> Sent: Monday, May 20, 2019 6:06:34 PM
> Subject: [PATCH 1/2] BootLinuxConsoleTest: Do not log empty lines
> 
> Avoid to log empty lines in console debug logs.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/acceptance/boot_linux_console.py | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/acceptance/boot_linux_console.py
> b/tests/acceptance/boot_linux_console.py
> index d5c500ea30..f593f3858e 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -37,8 +37,10 @@ class BootLinuxConsole(Test):
>          console = self.vm.console_socket.makefile()
>          console_logger = logging.getLogger('console')
>          while True:
> -            msg = console.readline()
> -            console_logger.debug(msg.strip())
> +            msg = console.readline().strip()
> +            if not msg:
> +                continue
> +            console_logger.debug(msg)
>              if success_message in msg:
>                  break
>              if failure_message in msg:
> --
> 2.19.1
> 
> 

Reviewed-by: Cleber Rosa <crosa@redhat.com>

Re: [Qemu-devel] [PATCH 2/2] BootLinuxConsoleTest: Test the SmartFusion2 board

[Cleber Rosa]

----- Original Message -----
> From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
> To: qemu-devel@nongnu.org
> Cc: "Eduardo Habkost" <ehabkost@redhat.com>, "Caio Carrara" <ccarrara@redhat.com>, "Alistair Francis"
> <alistair@alistair23.me>, "Subbaraya Sundeep" <sundeep.lkml@gmail.com>, qemu-arm@nongnu.org, "Cleber Rosa"
> <crosa@redhat.com>, "Peter Maydell" <peter.maydell@linaro.org>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>
> Sent: Monday, May 20, 2019 6:06:35 PM
> Subject: [PATCH 2/2] BootLinuxConsoleTest: Test the SmartFusion2 board
> 
> Similar to the x86_64/pc test, it boots a Linux kernel on an
> Emcraft board and verify the serial is working.
> 
> If ARM is a target being built, "make check-acceptance" will
> automatically include this test by the use of the "arch:arm" tags.
> 
> Alternatively, this test can be run using:
> 
>   $ avocado run -t arch:arm tests/acceptance
>   $ avocado run -t machine:emcraft-sf2 tests/acceptance
> 
> Based on the recommended test setup from Subbaraya Sundeep:
> https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg03810.html
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/acceptance/boot_linux_console.py | 27 ++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/tests/acceptance/boot_linux_console.py
> b/tests/acceptance/boot_linux_console.py
> index f593f3858e..844cb80bb5 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -178,6 +178,33 @@ class BootLinuxConsole(Test):
>          console_pattern = 'Kernel command line: %s' % kernel_command_line
>          self.wait_for_console_pattern(console_pattern)
>  
> +    def test_arm_emcraft_sf2(self):
> +        """
> +        :avocado: tags=arch:arm
> +        :avocado: tags=machine:emcraft-sf2
> +        :avocado: tags=endian:little
> +        """
> +        uboot_url = ('https://raw.githubusercontent.com/'
> +                     'Subbaraya-Sundeep/qemu-test-binaries/'
> +                     'fa030bd77a014a0b8e360d3b7011df89283a2f0b/u-boot')
> +        uboot_hash = 'abba5d9c24cdd2d49cdc2a8aa92976cf20737eff'
> +        uboot_path = self.fetch_asset(uboot_url, asset_hash=uboot_hash)
> +        spi_url = ('https://raw.githubusercontent.com/'
> +                   'Subbaraya-Sundeep/qemu-test-binaries/'
> +                   'fa030bd77a014a0b8e360d3b7011df89283a2f0b/spi.bin')
> +        spi_hash = '85f698329d38de63aea6e884a86fbde70890a78a'
> +        spi_path = self.fetch_asset(spi_url, asset_hash=spi_hash)
> +
> +        self.vm.set_machine('emcraft-sf2')
> +        self.vm.set_console()
> +        kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE

The kernel_command_line variable is not needed...

> +        self.vm.add_args('-kernel', uboot_path,
> +                         '-append', kernel_command_line,

... and you can just use self.KERNEL_COMMON_COMMAND_LINE here.

> +                         '-drive', 'file=' + spi_path +
> ',if=mtd,format=raw',

Nitpick: it's more Pythonic to format strings than to concatenate
them.

> +                         '-no-reboot')
> +        self.vm.launch()
> +        self.wait_for_console_pattern('init started: BusyBox')

Another nitpick is, given that image is pinned down, maybe attempt to
match against the entire line?

  init started: BusyBox v1.24.1 (2017-05-15 09:57:00 IST)

> +
>      def test_s390x_s390_ccw_virtio(self):
>          """
>          :avocado: tags=arch:s390x
> --
> 2.19.1
> 
> 

Because of the other discussions about tests and timeouts, this is
what I get on a first run (having to download the images):

 (1/1) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_emcraft_sf2: PASS (27.49 s)

And then, a pretty consistent < 8s mark using the resources from
cache:

 (01/10) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_emcraft_sf2: PASS (7.55 s)
 (02/10) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_emcraft_sf2: PASS (7.35 s)
 (03/10) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_emcraft_sf2: PASS (7.40 s)
 (04/10) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_emcraft_sf2: PASS (7.60 s)
 (05/10) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_emcraft_sf2: PASS (7.62 s)
 (06/10) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_emcraft_sf2: PASS (7.33 s)
 (07/10) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_emcraft_sf2: PASS (7.35 s)
 (08/10) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_emcraft_sf2: PASS (6.38 s)
 (09/10) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_emcraft_sf2: PASS (7.39 s)
 (10/10) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_emcraft_sf2: PASS (7.44 s)

It'd be nice to know the average test execution time on real/typical
hardware, and with that, starting to adjust expected timeouts.  Anyway:

Tested-by: Cleber Rosa <crosa@redhat.com>

Re: [Qemu-devel] [PATCH 1/2] BootLinuxConsoleTest: Do not log empty lines

[Cleber Rosa]

On Tue, May 21, 2019 at 12:06:34AM +0200, Philippe Mathieu-Daudé wrote:
> Avoid to log empty lines in console debug logs.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/acceptance/boot_linux_console.py | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
> index d5c500ea30..f593f3858e 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -37,8 +37,10 @@ class BootLinuxConsole(Test):
>          console = self.vm.console_socket.makefile()
>          console_logger = logging.getLogger('console')
>          while True:
> -            msg = console.readline()
> -            console_logger.debug(msg.strip())
> +            msg = console.readline().strip()
> +            if not msg:
> +                continue
> +            console_logger.debug(msg)
>              if success_message in msg:
>                  break
>              if failure_message in msg:
> -- 
> 2.19.1
> 

This is particularly useful in the boot_linux_console.py:BootLinuxConsole.test_s390x_s390_ccw_virtio test, where in my experience empty lines were being logged.

Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>

Re: [Qemu-devel] [PATCH 2/2] BootLinuxConsoleTest: Test the SmartFusion2 board

[Cleber Rosa]

On Tue, May 21, 2019 at 12:06:35AM +0200, Philippe Mathieu-Daudé wrote:
> Similar to the x86_64/pc test, it boots a Linux kernel on an
> Emcraft board and verify the serial is working.
> 
> If ARM is a target being built, "make check-acceptance" will
> automatically include this test by the use of the "arch:arm" tags.
> 
> Alternatively, this test can be run using:
> 
>   $ avocado run -t arch:arm tests/acceptance
>   $ avocado run -t machine:emcraft-sf2 tests/acceptance
>

The pattern accepted as a tag currently doesn't include a dash, see:

   https://avocado-framework.readthedocs.io/en/68.0/ReferenceGuide.html#docstring-directives-rules

My suggestion is to replace the dash for an underline.  This was done
on the s390 test:

   :avocado: tags=machine:s390_ccw_virtio

> Based on the recommended test setup from Subbaraya Sundeep:
> https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg03810.html
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/acceptance/boot_linux_console.py | 27 ++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
> index f593f3858e..844cb80bb5 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -178,6 +178,33 @@ class BootLinuxConsole(Test):
>          console_pattern = 'Kernel command line: %s' % kernel_command_line
>          self.wait_for_console_pattern(console_pattern)
>  
> +    def test_arm_emcraft_sf2(self):
> +        """
> +        :avocado: tags=arch:arm
> +        :avocado: tags=machine:emcraft-sf2

As per the previous comment, this would become:

    :avocado: tags=machine:emcraft_sf2

> +        :avocado: tags=endian:little
> +        """
> +        uboot_url = ('https://raw.githubusercontent.com/'
> +                     'Subbaraya-Sundeep/qemu-test-binaries/'
> +                     'fa030bd77a014a0b8e360d3b7011df89283a2f0b/u-boot')
> +        uboot_hash = 'abba5d9c24cdd2d49cdc2a8aa92976cf20737eff'
> +        uboot_path = self.fetch_asset(uboot_url, asset_hash=uboot_hash)
> +        spi_url = ('https://raw.githubusercontent.com/'
> +                   'Subbaraya-Sundeep/qemu-test-binaries/'
> +                   'fa030bd77a014a0b8e360d3b7011df89283a2f0b/spi.bin')
> +        spi_hash = '85f698329d38de63aea6e884a86fbde70890a78a'
> +        spi_path = self.fetch_asset(spi_url, asset_hash=spi_hash)
> +
> +        self.vm.set_machine('emcraft-sf2')
> +        self.vm.set_console()
> +        kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE
> +        self.vm.add_args('-kernel', uboot_path,
> +                         '-append', kernel_command_line,
> +                         '-drive', 'file=' + spi_path + ',if=mtd,format=raw',

Nitpick:

   '-drive', 'file=%s,if=mtd,format=raw' % spi_path,

> +                         '-no-reboot')
> +        self.vm.launch()
> +        self.wait_for_console_pattern('init started: BusyBox')
> +
>      def test_s390x_s390_ccw_virtio(self):
>          """
>          :avocado: tags=arch:s390x
> -- 
> 2.19.1
> 

Other than that, it looks good to me.

Re: [Qemu-devel] [PATCH 2/2] BootLinuxConsoleTest: Test the SmartFusion2 board

[Eduardo Habkost]

On Wed, Jun 05, 2019 at 12:06:59PM -0400, Cleber Rosa wrote:
> On Tue, May 21, 2019 at 12:06:35AM +0200, Philippe Mathieu-Daudé wrote:
> > Similar to the x86_64/pc test, it boots a Linux kernel on an
> > Emcraft board and verify the serial is working.
> > 
> > If ARM is a target being built, "make check-acceptance" will
> > automatically include this test by the use of the "arch:arm" tags.
> > 
> > Alternatively, this test can be run using:
> > 
> >   $ avocado run -t arch:arm tests/acceptance
> >   $ avocado run -t machine:emcraft-sf2 tests/acceptance
> >
> 
> The pattern accepted as a tag currently doesn't include a dash, see:
> 
>    https://avocado-framework.readthedocs.io/en/68.0/ReferenceGuide.html#docstring-directives-rules
> 

Why isn't this causing test jobs to fail?

This patch is already queued on python-next, should I remove it?


> My suggestion is to replace the dash for an underline.  This was done
> on the s390 test:
> 
>    :avocado: tags=machine:s390_ccw_virtio

I would love if we stopped using the docstring to store
machine-readable data and used Python class attributes or
methods, like most Python APIs do.  e.g.:

  class MyTest(avocado.Test):
      tags = {'machine':'s390-ccw-virtio'}

      def test_my_feature(self):
          ...


I understand the Avocado runner currently tries to not run any
test code at the "list" step.  But can we have a mode where it
will actually load the Python module and look at the class
objects directly, instead of trying to parse the Python code
without evaluating it?

> [...]

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/2] BootLinuxConsoleTest: Do not log empty lines

[Aleksandar Markovic]

On May 21, 2019 12:07 AM, "Philippe Mathieu-Daudé" <f4bug@amsat.org> wrote:
>
> Avoid to log empty lines in console debug logs.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---

Acked-by: Aleksandar Markovic <amarkovic@wavecomp.com>

>  tests/acceptance/boot_linux_console.py | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tests/acceptance/boot_linux_console.py
b/tests/acceptance/boot_linux_console.py
> index d5c500ea30..f593f3858e 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -37,8 +37,10 @@ class BootLinuxConsole(Test):
>          console = self.vm.console_socket.makefile()
>          console_logger = logging.getLogger('console')
>          while True:
> -            msg = console.readline()
> -            console_logger.debug(msg.strip())
> +            msg = console.readline().strip()
> +            if not msg:
> +                continue
> +            console_logger.debug(msg)
>              if success_message in msg:
>                  break
>              if failure_message in msg:
> --
> 2.19.1
>
>

Re: [Qemu-devel] [PATCH 2/2] BootLinuxConsoleTest: Test the SmartFusion2 board

[Cleber Rosa]

On Wed, Jun 05, 2019 at 05:43:23PM -0300, Eduardo Habkost wrote:
> On Wed, Jun 05, 2019 at 12:06:59PM -0400, Cleber Rosa wrote:
> > On Tue, May 21, 2019 at 12:06:35AM +0200, Philippe Mathieu-Daudé wrote:
> > > Similar to the x86_64/pc test, it boots a Linux kernel on an
> > > Emcraft board and verify the serial is working.
> > > 
> > > If ARM is a target being built, "make check-acceptance" will
> > > automatically include this test by the use of the "arch:arm" tags.
> > > 
> > > Alternatively, this test can be run using:
> > > 
> > >   $ avocado run -t arch:arm tests/acceptance
> > >   $ avocado run -t machine:emcraft-sf2 tests/acceptance
> > >
> > 
> > The pattern accepted as a tag currently doesn't include a dash, see:
> > 
> >    https://avocado-framework.readthedocs.io/en/68.0/ReferenceGuide.html#docstring-directives-rules
> > 
> 
> Why isn't this causing test jobs to fail?

Because docstring directives are searched for "gracefully", that is,
if it doesn't match the pattern, it's not considered a docstring
directive.

Also, there's nothing in the jobs that require or check for that
specific machine tag.  The "check-acceptance" rule uses the built
targets, and adds "arch:<$TARGET>" tags to the command line.

Now, if one run this (as exemplified in the commit message):

  $ avocado run -t machine:emcraft-sf2 tests/acceptance

The *tag* failure/limitation would have been noticed.  BTW, it should
be easy to expand the docstring directive pattern and fix this
limitation on newer Avocado versions.

You can also see the parsed key/val tags with:

  $ avocado list -V tests/acceptance

Although only on new Avocado versions (>69.0), you can also see the
parsed key:val tags.

> 
> This patch is already queued on python-next, should I remove it?
>

Or ammended it?  It's a minor issue IMO (s/-/_/).

> 
> > My suggestion is to replace the dash for an underline.  This was done
> > on the s390 test:
> > 
> >    :avocado: tags=machine:s390_ccw_virtio
> 
> I would love if we stopped using the docstring to store
> machine-readable data and used Python class attributes or
> methods, like most Python APIs do.  e.g.:
> 
>   class MyTest(avocado.Test):
>       tags = {'machine':'s390-ccw-virtio'}
> 
>       def test_my_feature(self):
>           ...
>
> 
> I understand the Avocado runner currently tries to not run any
> test code at the "list" step.  But can we have a mode where it
> will actually load the Python module and look at the class
> objects directly, instead of trying to parse the Python code
> without evaluating it?
> 
> > [...]
> 
> -- 
> Eduardo

I get your point, and I'd love to have no "no learning wheels" mode
too.  But given that the "safe loader" was a very basic architecture
design, and we probably wouldn't be dropping it, this means twice the
code to maintain.

I'd be interested in collecting more use cases (besides this s/-/_/)
to better justify such an effort.

Regards,
- Cleber.