diff mbox series

[3/8] tests/acceptance/linux_ssh_mips_malta.py: drop identical setUp

Message ID 20210415215141.1865467-4-crosa@redhat.com (mailing list archive)
State New
Headers show
Series Tests: introduce custom jobs | expand

Commit Message

Cleber Rosa April 15, 2021, 9:51 p.m. UTC
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(-)

Comments

Philippe Mathieu-Daudé April 16, 2021, 5:26 a.m. UTC | #1
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')
>
Willian Rampazzo April 16, 2021, 3:41 p.m. UTC | #2
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>
Cleber Rosa April 16, 2021, 3:43 p.m. UTC | #3
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.
Willian Rampazzo April 16, 2021, 3:46 p.m. UTC | #4
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.
Philippe Mathieu-Daudé April 16, 2021, 5:46 p.m. UTC | #5
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.
Wainer dos Santos Moschetta April 19, 2021, 6:25 p.m. UTC | #6
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.
>
diff mbox series

Patch

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