diff mbox series

[4/7] tests/acceptance: Sun4uMachine: Remove dependency to LinuxKernelTest

Message ID 20210503224326.206208-5-wainersm@redhat.com (mailing list archive)
State New, archived
Headers show
Series tests/acceptance: Introducing the ConsoleMixIn | expand

Commit Message

Wainer dos Santos Moschetta May 3, 2021, 10:43 p.m. UTC
The Sun4uMachine class inherit from LinuxKernelTest to effectively only use
the KERNEL_COMMON_COMMAND_LINE attribute. This change remove that unneeded
dependency, making Sun4uMachine self-content.

I took the occasion to delint the code: the unused os import was
removed, imports were reordered, and the module has a docstring now.

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
---
 tests/acceptance/machine_sparc64_sun4u.py | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Comments

Philippe Mathieu-Daudé May 4, 2021, 4:01 p.m. UTC | #1
Hi Wainer,

On 5/4/21 12:43 AM, Wainer dos Santos Moschetta wrote:
> The Sun4uMachine class inherit from LinuxKernelTest to effectively only use
> the KERNEL_COMMON_COMMAND_LINE attribute. This change remove that unneeded
> dependency, making Sun4uMachine self-content.

It is odd because the test boots a Linux kernel...

Once you added ConsoleMixIn, LinuxKernelTest is left with
2 methods related to archive extraction. Not particularly
specific to Linux kernel. Beside, shouldn't these methods
be provided by Avocado directly, by avocado.utils.archive
and avocado.utils.software_manager.backends?

> I took the occasion to delint the code: the unused os import was
> removed, imports were reordered, and the module has a docstring now.
> 
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
>  tests/acceptance/machine_sparc64_sun4u.py | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
Willian Rampazzo May 24, 2021, 6:24 p.m. UTC | #2
On Mon, May 3, 2021 at 7:44 PM Wainer dos Santos Moschetta
<wainersm@redhat.com> wrote:
>
> The Sun4uMachine class inherit from LinuxKernelTest to effectively only use
> the KERNEL_COMMON_COMMAND_LINE attribute. This change remove that unneeded
> dependency, making Sun4uMachine self-content.
>
> I took the occasion to delint the code: the unused os import was
> removed, imports were reordered, and the module has a docstring now.
>
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> ---
>  tests/acceptance/machine_sparc64_sun4u.py | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>

Reviewed-by: Willian Rampazzo <willianr@redhat.com>
Willian Rampazzo May 24, 2021, 6:30 p.m. UTC | #3
On Tue, May 4, 2021 at 1:01 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Hi Wainer,
>
> On 5/4/21 12:43 AM, Wainer dos Santos Moschetta wrote:
> > The Sun4uMachine class inherit from LinuxKernelTest to effectively only use
> > the KERNEL_COMMON_COMMAND_LINE attribute. This change remove that unneeded
> > dependency, making Sun4uMachine self-content.
>
> It is odd because the test boots a Linux kernel...
>
> Once you added ConsoleMixIn, LinuxKernelTest is left with
> 2 methods related to archive extraction. Not particularly
> specific to Linux kernel. Beside, shouldn't these methods
> be provided by Avocado directly, by avocado.utils.archive
> and avocado.utils.software_manager.backends?

Indeed, it makes a lot of sense to have those two methods inside the
Avocado utilities. I opened an issue on the Avocado side to track
that: https://github.com/avocado-framework/avocado/issues/4610.

>
> > I took the occasion to delint the code: the unused os import was
> > removed, imports were reordered, and the module has a docstring now.
> >
> > Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> > ---
> >  tests/acceptance/machine_sparc64_sun4u.py | 11 +++++------
> >  1 file changed, 5 insertions(+), 6 deletions(-)
>
Willian Rampazzo May 24, 2021, 6:54 p.m. UTC | #4
On Mon, May 24, 2021 at 3:30 PM Willian Rampazzo <wrampazz@redhat.com> wrote:
>
> On Tue, May 4, 2021 at 1:01 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >
> > Hi Wainer,
> >
> > On 5/4/21 12:43 AM, Wainer dos Santos Moschetta wrote:
> > > The Sun4uMachine class inherit from LinuxKernelTest to effectively only use
> > > the KERNEL_COMMON_COMMAND_LINE attribute. This change remove that unneeded
> > > dependency, making Sun4uMachine self-content.
> >
> > It is odd because the test boots a Linux kernel...
> >
> > Once you added ConsoleMixIn, LinuxKernelTest is left with
> > 2 methods related to archive extraction. Not particularly
> > specific to Linux kernel. Beside, shouldn't these methods
> > be provided by Avocado directly, by avocado.utils.archive
> > and avocado.utils.software_manager.backends?
>
> Indeed, it makes a lot of sense to have those two methods inside the
> Avocado utilities. I opened an issue on the Avocado side to track
> that: https://github.com/avocado-framework/avocado/issues/4610.

Beraldo reminded me there is already an issue to handle this:
https://github.com/avocado-framework/avocado/issues/3549.
diff mbox series

Patch

diff --git a/tests/acceptance/machine_sparc64_sun4u.py b/tests/acceptance/machine_sparc64_sun4u.py
index 458165500e..c7ad474bdc 100644
--- a/tests/acceptance/machine_sparc64_sun4u.py
+++ b/tests/acceptance/machine_sparc64_sun4u.py
@@ -1,4 +1,4 @@ 
-# Functional test that boots a Linux kernel and checks the console
+"""Functional test that boots a Linux kernel and checks the console"""
 #
 # Copyright (c) 2020 Red Hat, Inc.
 #
@@ -8,16 +8,15 @@ 
 # 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 os
-
-from avocado_qemu import wait_for_console_pattern
 from avocado.utils import archive
-from boot_linux_console import LinuxKernelTest
+from avocado_qemu import Test
+from avocado_qemu import wait_for_console_pattern
 
-class Sun4uMachine(LinuxKernelTest):
+class Sun4uMachine(Test):
     """Boots the Linux kernel and checks that the console is operational"""
 
     timeout = 90
+    KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
 
     def test_sparc64_sun4u(self):
         """