1. Patchew
  2. QEMU
  3. View series

[PATCH] tests/acceptance: Re-enable the microblaze test

Thomas Huth posted 1 patch 3 years, 3 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210128152815.585478-1-thuth@redhat.com
Maintainers: "Philippe Mathieu-Daudé" <philmd@redhat.com>, Wainer dos Santos Moschetta <wainersm@redhat.com>, Cleber Rosa <crosa@redhat.com>
MAINTAINERS                            |  1 +
tests/acceptance/boot_linux_console.py |  9 -------
tests/acceptance/machine_microblaze.py | 35 ++++++++++++++++++++++++++
3 files changed, 36 insertions(+), 9 deletions(-)
create mode 100644 tests/acceptance/machine_microblaze.py
[PATCH] tests/acceptance: Re-enable the microblaze test
Posted by Thomas Huth 3 years, 3 months ago 
The microblaze kernel sometimes gets stuck during boot (ca. 1 out of 200
times), so we disabled the corresponding acceptance tests some months
ago. However, it's likely better to check that the kernel is still
starting than to not testing it at all anymore. Move the test to
a separate file, enable it again and check for an earlier console
message that should always appear.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 MAINTAINERS                            |  1 +
 tests/acceptance/boot_linux_console.py |  9 -------
 tests/acceptance/machine_microblaze.py | 35 ++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 9 deletions(-)
 create mode 100644 tests/acceptance/machine_microblaze.py

diff --git a/MAINTAINERS b/MAINTAINERS
index 34359a99b8..157ad4f7ef 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1112,6 +1112,7 @@ M: Edgar E. Iglesias <edgar.iglesias@gmail.com>
 S: Maintained
 F: hw/microblaze/petalogix_s3adsp1800_mmu.c
 F: include/hw/char/xilinx_uartlite.h
+F: tests/acceptance/machine_microblaze.py
 
 petalogix_ml605
 M: Edgar E. Iglesias <edgar.iglesias@gmail.com>
diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
index fb41bb7144..969fbf3952 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -1047,15 +1047,6 @@ class BootLinuxConsole(LinuxKernelTest):
         tar_hash = 'ac688fd00561a2b6ce1359f9ff6aa2b98c9a570c'
         self.do_test_advcal_2018('07', tar_hash, 'sanity-clause.elf')
 
-    @skip("Test currently broken") # Console stuck as of 5.2-rc1
-    def test_microblaze_s3adsp1800(self):
-        """
-        :avocado: tags=arch:microblaze
-        :avocado: tags=machine:petalogix-s3adsp1800
-        """
-        tar_hash = '08bf3e3bfb6b6c7ce1e54ab65d54e189f2caf13f'
-        self.do_test_advcal_2018('17', tar_hash, 'ballerina.bin')
-
     def test_or1k_sim(self):
         """
         :avocado: tags=arch:or1k
diff --git a/tests/acceptance/machine_microblaze.py b/tests/acceptance/machine_microblaze.py
new file mode 100644
index 0000000000..7f6d18495d
--- /dev/null
+++ b/tests/acceptance/machine_microblaze.py
@@ -0,0 +1,35 @@
+# Functional test that boots a microblaze Linux kernel and checks the console
+#
+# Copyright (c) 2018, 2021 Red Hat, Inc.
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later. See the COPYING file in the top-level directory.
+
+from avocado_qemu import Test
+from avocado_qemu import wait_for_console_pattern
+from avocado.utils import archive
+
+class MicroblazeMachine(Test):
+
+    timeout = 90
+
+    def test_microblaze_s3adsp1800(self):
+        """
+        :avocado: tags=arch:microblaze
+        :avocado: tags=machine:petalogix-s3adsp1800
+        """
+
+        tar_url = ('https://www.qemu-advent-calendar.org'
+                   '/2018/download/day17.tar.xz')
+        tar_hash = '08bf3e3bfb6b6c7ce1e54ab65d54e189f2caf13f'
+        file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
+        archive.extract(file_path, self.workdir)
+        self.vm.set_console()
+        self.vm.add_args('-kernel', self.workdir + '/day17/ballerina.bin')
+        self.vm.launch()
+        wait_for_console_pattern(self, 'This architecture does not have '
+                                       'kernel memory protection')
+        # Note:
+        # The kernel sometimes gets stuck after the "This architecture ..."
+        # message, that's why we don't test for a later string here. This
+        # needs some investigation by a microblaze wizard one day...
-- 
2.27.0
Re: [PATCH] tests/acceptance: Re-enable the microblaze test
Posted by Wainer dos Santos Moschetta 3 years, 3 months ago 
Hi,

On 1/28/21 12:28 PM, Thomas Huth wrote:
> The microblaze kernel sometimes gets stuck during boot (ca. 1 out of 200
> times), so we disabled the corresponding acceptance tests some months
> ago. However, it's likely better to check that the kernel is still
> starting than to not testing it at all anymore. Move the test to
> a separate file, enable it again and check for an earlier console
> message that should always appear.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   MAINTAINERS                            |  1 +
>   tests/acceptance/boot_linux_console.py |  9 -------
>   tests/acceptance/machine_microblaze.py | 35 ++++++++++++++++++++++++++
>   3 files changed, 36 insertions(+), 9 deletions(-)
>   create mode 100644 tests/acceptance/machine_microblaze.py
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 34359a99b8..157ad4f7ef 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1112,6 +1112,7 @@ M: Edgar E. Iglesias <edgar.iglesias@gmail.com>
>   S: Maintained
>   F: hw/microblaze/petalogix_s3adsp1800_mmu.c
>   F: include/hw/char/xilinx_uartlite.h
> +F: tests/acceptance/machine_microblaze.py
>   
>   petalogix_ml605
>   M: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> diff --git a/tests/acceptance/boot_linux_console.py b/tests/acceptance/boot_linux_console.py
> index fb41bb7144..969fbf3952 100644
> --- a/tests/acceptance/boot_linux_console.py
> +++ b/tests/acceptance/boot_linux_console.py
> @@ -1047,15 +1047,6 @@ class BootLinuxConsole(LinuxKernelTest):
>           tar_hash = 'ac688fd00561a2b6ce1359f9ff6aa2b98c9a570c'
>           self.do_test_advcal_2018('07', tar_hash, 'sanity-clause.elf')
>   
> -    @skip("Test currently broken") # Console stuck as of 5.2-rc1
> -    def test_microblaze_s3adsp1800(self):
> -        """
> -        :avocado: tags=arch:microblaze
> -        :avocado: tags=machine:petalogix-s3adsp1800
> -        """
> -        tar_hash = '08bf3e3bfb6b6c7ce1e54ab65d54e189f2caf13f'
> -        self.do_test_advcal_2018('17', tar_hash, 'ballerina.bin')
> -
>       def test_or1k_sim(self):
>           """
>           :avocado: tags=arch:or1k
> diff --git a/tests/acceptance/machine_microblaze.py b/tests/acceptance/machine_microblaze.py
> new file mode 100644
> index 0000000000..7f6d18495d
> --- /dev/null
> +++ b/tests/acceptance/machine_microblaze.py
> @@ -0,0 +1,35 @@
> +# Functional test that boots a microblaze Linux kernel and checks the console
> +#
> +# Copyright (c) 2018, 2021 Red Hat, Inc.
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or
> +# later. See the COPYING file in the top-level directory.
> +
> +from avocado_qemu import Test
> +from avocado_qemu import wait_for_console_pattern
> +from avocado.utils import archive
> +
> +class MicroblazeMachine(Test):
> +
> +    timeout = 90
> +
> +    def test_microblaze_s3adsp1800(self):
> +        """
> +        :avocado: tags=arch:microblaze
> +        :avocado: tags=machine:petalogix-s3adsp1800
> +        """
> +
> +        tar_url = ('https://www.qemu-advent-calendar.org'
> +                   '/2018/download/day17.tar.xz')
> +        tar_hash = '08bf3e3bfb6b6c7ce1e54ab65d54e189f2caf13f'
> +        file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
> +        archive.extract(file_path, self.workdir)
> +        self.vm.set_console()
> +        self.vm.add_args('-kernel', self.workdir + '/day17/ballerina.bin')
> +        self.vm.launch()
> +        wait_for_console_pattern(self, 'This architecture does not have '
> +                                       'kernel memory protection')
> +        # Note:
> +        # The kernel sometimes gets stuck after the "This architecture ..."
> +        # message, that's why we don't test for a later string here. This
> +        # needs some investigation by a microblaze wizard one day...


The change looks good to me.

Also I appreciate the note you left on code. In addition I suggest to 
put some tag to indicate it needs some investigation or that it is flaky 
(although that's not the case anymore), because it will ease the 
discovery and filtering of "problematic" tests. What do you think?

- Wainer
Re: [PATCH] tests/acceptance: Re-enable the microblaze test
Posted by Thomas Huth 3 years, 3 months ago 
On 28/01/2021 20.34, Wainer dos Santos Moschetta wrote:
> Hi,
> 
> On 1/28/21 12:28 PM, Thomas Huth wrote:
>> The microblaze kernel sometimes gets stuck during boot (ca. 1 out of 200
>> times), so we disabled the corresponding acceptance tests some months
>> ago. However, it's likely better to check that the kernel is still
>> starting than to not testing it at all anymore. Move the test to
>> a separate file, enable it again and check for an earlier console
>> message that should always appear.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   MAINTAINERS                            |  1 +
>>   tests/acceptance/boot_linux_console.py |  9 -------
>>   tests/acceptance/machine_microblaze.py | 35 ++++++++++++++++++++++++++
>>   3 files changed, 36 insertions(+), 9 deletions(-)
>>   create mode 100644 tests/acceptance/machine_microblaze.py
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 34359a99b8..157ad4f7ef 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1112,6 +1112,7 @@ M: Edgar E. Iglesias <edgar.iglesias@gmail.com>
>>   S: Maintained
>>   F: hw/microblaze/petalogix_s3adsp1800_mmu.c
>>   F: include/hw/char/xilinx_uartlite.h
>> +F: tests/acceptance/machine_microblaze.py
>>   petalogix_ml605
>>   M: Edgar E. Iglesias <edgar.iglesias@gmail.com>
>> diff --git a/tests/acceptance/boot_linux_console.py 
>> b/tests/acceptance/boot_linux_console.py
>> index fb41bb7144..969fbf3952 100644
>> --- a/tests/acceptance/boot_linux_console.py
>> +++ b/tests/acceptance/boot_linux_console.py
>> @@ -1047,15 +1047,6 @@ class BootLinuxConsole(LinuxKernelTest):
>>           tar_hash = 'ac688fd00561a2b6ce1359f9ff6aa2b98c9a570c'
>>           self.do_test_advcal_2018('07', tar_hash, 'sanity-clause.elf')
>> -    @skip("Test currently broken") # Console stuck as of 5.2-rc1
>> -    def test_microblaze_s3adsp1800(self):
>> -        """
>> -        :avocado: tags=arch:microblaze
>> -        :avocado: tags=machine:petalogix-s3adsp1800
>> -        """
>> -        tar_hash = '08bf3e3bfb6b6c7ce1e54ab65d54e189f2caf13f'
>> -        self.do_test_advcal_2018('17', tar_hash, 'ballerina.bin')
>> -
>>       def test_or1k_sim(self):
>>           """
>>           :avocado: tags=arch:or1k
>> diff --git a/tests/acceptance/machine_microblaze.py 
>> b/tests/acceptance/machine_microblaze.py
>> new file mode 100644
>> index 0000000000..7f6d18495d
>> --- /dev/null
>> +++ b/tests/acceptance/machine_microblaze.py
>> @@ -0,0 +1,35 @@
>> +# Functional test that boots a microblaze Linux kernel and checks the 
>> console
>> +#
>> +# Copyright (c) 2018, 2021 Red Hat, Inc.
>> +#
>> +# This work is licensed under the terms of the GNU GPL, version 2 or
>> +# later. See the COPYING file in the top-level directory.
>> +
>> +from avocado_qemu import Test
>> +from avocado_qemu import wait_for_console_pattern
>> +from avocado.utils import archive
>> +
>> +class MicroblazeMachine(Test):
>> +
>> +    timeout = 90
>> +
>> +    def test_microblaze_s3adsp1800(self):
>> +        """
>> +        :avocado: tags=arch:microblaze
>> +        :avocado: tags=machine:petalogix-s3adsp1800
>> +        """
>> +
>> +        tar_url = ('https://www.qemu-advent-calendar.org'
>> +                   '/2018/download/day17.tar.xz')
>> +        tar_hash = '08bf3e3bfb6b6c7ce1e54ab65d54e189f2caf13f'
>> +        file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
>> +        archive.extract(file_path, self.workdir)
>> +        self.vm.set_console()
>> +        self.vm.add_args('-kernel', self.workdir + '/day17/ballerina.bin')
>> +        self.vm.launch()
>> +        wait_for_console_pattern(self, 'This architecture does not have '
>> +                                       'kernel memory protection')
>> +        # Note:
>> +        # The kernel sometimes gets stuck after the "This architecture ..."
>> +        # message, that's why we don't test for a later string here. This
>> +        # needs some investigation by a microblaze wizard one day...
> 
> 
> The change looks good to me.
> 
> Also I appreciate the note you left on code. In addition I suggest to put 
> some tag to indicate it needs some investigation or that it is flaky 
> (although that's not the case anymore), because it will ease the discovery 
> and filtering of "problematic" tests. What do you think?

Sounds reasonable, but I'm not aware of any such tags in the "acceptance" 
code yet... what exactly do you have in mind?

  Thomas
Re: [PATCH] tests/acceptance: Re-enable the microblaze test
Posted by Wainer dos Santos Moschetta 3 years, 3 months ago 
Hi Thomas,

On 1/29/21 2:40 AM, Thomas Huth wrote:
> On 28/01/2021 20.34, Wainer dos Santos Moschetta wrote:
>> Hi,
>>
>> On 1/28/21 12:28 PM, Thomas Huth wrote:
>>> The microblaze kernel sometimes gets stuck during boot (ca. 1 out of 
>>> 200
>>> times), so we disabled the corresponding acceptance tests some months
>>> ago. However, it's likely better to check that the kernel is still
>>> starting than to not testing it at all anymore. Move the test to
>>> a separate file, enable it again and check for an earlier console
>>> message that should always appear.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   MAINTAINERS                            |  1 +
>>>   tests/acceptance/boot_linux_console.py |  9 -------
>>>   tests/acceptance/machine_microblaze.py | 35 
>>> ++++++++++++++++++++++++++
>>>   3 files changed, 36 insertions(+), 9 deletions(-)
>>>   create mode 100644 tests/acceptance/machine_microblaze.py
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 34359a99b8..157ad4f7ef 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -1112,6 +1112,7 @@ M: Edgar E. Iglesias <edgar.iglesias@gmail.com>
>>>   S: Maintained
>>>   F: hw/microblaze/petalogix_s3adsp1800_mmu.c
>>>   F: include/hw/char/xilinx_uartlite.h
>>> +F: tests/acceptance/machine_microblaze.py
>>>   petalogix_ml605
>>>   M: Edgar E. Iglesias <edgar.iglesias@gmail.com>
>>> diff --git a/tests/acceptance/boot_linux_console.py 
>>> b/tests/acceptance/boot_linux_console.py
>>> index fb41bb7144..969fbf3952 100644
>>> --- a/tests/acceptance/boot_linux_console.py
>>> +++ b/tests/acceptance/boot_linux_console.py
>>> @@ -1047,15 +1047,6 @@ class BootLinuxConsole(LinuxKernelTest):
>>>           tar_hash = 'ac688fd00561a2b6ce1359f9ff6aa2b98c9a570c'
>>>           self.do_test_advcal_2018('07', tar_hash, 'sanity-clause.elf')
>>> -    @skip("Test currently broken") # Console stuck as of 5.2-rc1
>>> -    def test_microblaze_s3adsp1800(self):
>>> -        """
>>> -        :avocado: tags=arch:microblaze
>>> -        :avocado: tags=machine:petalogix-s3adsp1800
>>> -        """
>>> -        tar_hash = '08bf3e3bfb6b6c7ce1e54ab65d54e189f2caf13f'
>>> -        self.do_test_advcal_2018('17', tar_hash, 'ballerina.bin')
>>> -
>>>       def test_or1k_sim(self):
>>>           """
>>>           :avocado: tags=arch:or1k
>>> diff --git a/tests/acceptance/machine_microblaze.py 
>>> b/tests/acceptance/machine_microblaze.py
>>> new file mode 100644
>>> index 0000000000..7f6d18495d
>>> --- /dev/null
>>> +++ b/tests/acceptance/machine_microblaze.py
>>> @@ -0,0 +1,35 @@
>>> +# Functional test that boots a microblaze Linux kernel and checks 
>>> the console
>>> +#
>>> +# Copyright (c) 2018, 2021 Red Hat, Inc.
>>> +#
>>> +# This work is licensed under the terms of the GNU GPL, version 2 or
>>> +# later. See the COPYING file in the top-level directory.
>>> +
>>> +from avocado_qemu import Test
>>> +from avocado_qemu import wait_for_console_pattern
>>> +from avocado.utils import archive
>>> +
>>> +class MicroblazeMachine(Test):
>>> +
>>> +    timeout = 90
>>> +
>>> +    def test_microblaze_s3adsp1800(self):
>>> +        """
>>> +        :avocado: tags=arch:microblaze
>>> +        :avocado: tags=machine:petalogix-s3adsp1800
>>> +        """
>>> +
>>> +        tar_url = ('https://www.qemu-advent-calendar.org'
>>> +                   '/2018/download/day17.tar.xz')
>>> +        tar_hash = '08bf3e3bfb6b6c7ce1e54ab65d54e189f2caf13f'
>>> +        file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
>>> +        archive.extract(file_path, self.workdir)
>>> +        self.vm.set_console()
>>> +        self.vm.add_args('-kernel', self.workdir + 
>>> '/day17/ballerina.bin')
>>> +        self.vm.launch()
>>> +        wait_for_console_pattern(self, 'This architecture does not 
>>> have '
>>> +                                       'kernel memory protection')
>>> +        # Note:
>>> +        # The kernel sometimes gets stuck after the "This 
>>> architecture ..."
>>> +        # message, that's why we don't test for a later string 
>>> here. This
>>> +        # needs some investigation by a microblaze wizard one day...
>>
>>
>> The change looks good to me.
>>
>> Also I appreciate the note you left on code. In addition I suggest to 
>> put some tag to indicate it needs some investigation or that it is 
>> flaky (although that's not the case anymore), because it will ease 
>> the discovery and filtering of "problematic" tests. What do you think?
>
> Sounds reasonable, but I'm not aware of any such tags in the 
> "acceptance" code yet... what exactly do you have in mind?

First, I don't want to hold this patch any longer. So:

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

Then back to the tags discussion...

I agree with Philippe when he says [1] that Avocado tags might be 
underused. But unlike his idea (see [1], which I think is very good btw) 
here I mentioned the use of tags to solve a different problem: ease the 
search for tests that need maintenance.

The test in this patch, for example, could be tagged "needReview" 
(sorry, there must exist a better tag name). If it was failing 
intermittently (and we hope your changes prevent that) then we could 
have it tagged "flaky". Disabled tests get marked as "disabled", and so 
on. Thus, one can use the `avocado list -t needReview` command to get 
the list of tests.

I will come up with a RFC series so we may continue this conversation on 
concrete examples. Sounds good to you?

[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg772651.html

Thanks!

- Wainer

>
>
>  Thomas
Re: [PATCH] tests/acceptance: Re-enable the microblaze test
Posted by Philippe Mathieu-Daudé 3 years, 2 months ago 
On 1/28/21 4:28 PM, Thomas Huth wrote:
> The microblaze kernel sometimes gets stuck during boot (ca. 1 out of 200
> times), so we disabled the corresponding acceptance tests some months
> ago. However, it's likely better to check that the kernel is still
> starting than to not testing it at all anymore. Move the test to
> a separate file, enable it again and check for an earlier console
> message that should always appear.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  MAINTAINERS                            |  1 +
>  tests/acceptance/boot_linux_console.py |  9 -------
>  tests/acceptance/machine_microblaze.py | 35 ++++++++++++++++++++++++++
>  3 files changed, 36 insertions(+), 9 deletions(-)
>  create mode 100644 tests/acceptance/machine_microblaze.py

Thanks, applied to my acceptance-testing tree.

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.