diff mbox series

[v2] UBUNTU: SAUCE: ubuntu_boot: fix second test execution

Message ID 20210603130352.38472-1-krzysztof.kozlowski@canonical.com
State New
Headers show
Series [v2] UBUNTU: SAUCE: ubuntu_boot: fix second test execution | expand

Commit Message

Krzysztof Kozlowski June 3, 2021, 1:03 p.m. UTC
The setup() might not be called on second test run and actually autotest
design assumes setup() for given version will be called exactly once.
It is expected from setup() to modify only persistent state of the
system, so on next run it's execution is not needed.  Runtime-only
settings should not be configured via setup().

This fixes failure on second test execution:

    00:54:03 INFO | 	START	ubuntu_boot.log_check	ubuntu_boot.log_check
    00:54:03 DEBUG| Persistent state client._record_indent now set to 2
    00:54:03 DEBUG| Persistent state client.unexpected_reboot now set to ('ubuntu_boot.log_check', 'ubuntu_boot.log_check')
    00:54:03 DEBUG| Waiting for pid 19403 for 300 seconds
    00:54:03 WARNI| System python is too old, crash handling disabled
    00:54:03 ERROR| Exception escaping from test:
    Traceback (most recent call last):
      File "/home/azure/autotest/client/shared/test.py", line 411, in _exec
        _call_test_function(self.execute, *p_args, **p_dargs)
      File "/home/azure/autotest/client/shared/test.py", line 830, in _call_test_function
        raise error.UnhandledTestFail(e)
    UnhandledTestFail: Unhandled AttributeError: 'ubuntu_boot' object has no attribute 'centos'
    Traceback (most recent call last):
      File "/home/azure/autotest/client/shared/test.py", line 823, in _call_test_function
        return func(*args, **dargs)
      File "/home/azure/autotest/client/shared/test.py", line 291, in execute
        postprocess_profiled_run, args, dargs)
      File "/home/azure/autotest/client/shared/test.py", line 212, in _call_run_once
        self.run_once(*args, **dargs)
      File "/home/azure/autotest/client/tests/ubuntu_boot/ubuntu_boot.py", line 63, in run_once
        if not self.log_check():
      File "/home/azure/autotest/client/tests/ubuntu_boot/ubuntu_boot.py", line 25, in log_check
        if self.centos:
    AttributeError: 'ubuntu_boot' object has no attribute 'centos'

Fixes: e14cb0e1bd0c ("UBUNTU SAUCE: ubuntu_boot Centos Support")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

---

Changes since v1:
1. Add prefix to subject
---
 ubuntu_boot/ubuntu_boot.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Po-Hsu Lin June 4, 2021, 3:04 a.m. UTC | #1
On Thu, Jun 3, 2021 at 9:04 PM Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
>
> The setup() might not be called on second test run and actually autotest
> design assumes setup() for given version will be called exactly once.
> It is expected from setup() to modify only persistent state of the
> system, so on next run it's execution is not needed.  Runtime-only
> settings should not be configured via setup().
>
> This fixes failure on second test execution:
>
>     00:54:03 INFO |     START   ubuntu_boot.log_check   ubuntu_boot.log_check
>     00:54:03 DEBUG| Persistent state client._record_indent now set to 2
>     00:54:03 DEBUG| Persistent state client.unexpected_reboot now set to ('ubuntu_boot.log_check', 'ubuntu_boot.log_check')
>     00:54:03 DEBUG| Waiting for pid 19403 for 300 seconds
>     00:54:03 WARNI| System python is too old, crash handling disabled
>     00:54:03 ERROR| Exception escaping from test:
>     Traceback (most recent call last):
>       File "/home/azure/autotest/client/shared/test.py", line 411, in _exec
>         _call_test_function(self.execute, *p_args, **p_dargs)
>       File "/home/azure/autotest/client/shared/test.py", line 830, in _call_test_function
>         raise error.UnhandledTestFail(e)
>     UnhandledTestFail: Unhandled AttributeError: 'ubuntu_boot' object has no attribute 'centos'
>     Traceback (most recent call last):
>       File "/home/azure/autotest/client/shared/test.py", line 823, in _call_test_function
>         return func(*args, **dargs)
>       File "/home/azure/autotest/client/shared/test.py", line 291, in execute
>         postprocess_profiled_run, args, dargs)
>       File "/home/azure/autotest/client/shared/test.py", line 212, in _call_run_once
>         self.run_once(*args, **dargs)
>       File "/home/azure/autotest/client/tests/ubuntu_boot/ubuntu_boot.py", line 63, in run_once
>         if not self.log_check():
>       File "/home/azure/autotest/client/tests/ubuntu_boot/ubuntu_boot.py", line 25, in log_check
>         if self.centos:
>     AttributeError: 'ubuntu_boot' object has no attribute 'centos'
>
> Fixes: e14cb0e1bd0c ("UBUNTU SAUCE: ubuntu_boot Centos Support")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>
> ---
>
> Changes since v1:
> 1. Add prefix to subject
> ---
>  ubuntu_boot/ubuntu_boot.py | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/ubuntu_boot/ubuntu_boot.py b/ubuntu_boot/ubuntu_boot.py
> index a67f21d49fc7..b67a588285f8 100644
> --- a/ubuntu_boot/ubuntu_boot.py
> +++ b/ubuntu_boot/ubuntu_boot.py
> @@ -12,17 +12,17 @@ class ubuntu_boot(test.test):
>          cmd = 'yes "" | DEBIAN_FRONTEND=noninteractive apt-get install --yes --force-yes ' + ' '.join(pkgs)
>          self.results = utils.system_output(cmd, retain_output=True)
>
> +    def log_check(self):
> +        '''Test for checking error patterns in log files'''
>          '''Centos Specific Boot Test Checks'''
> -        self.centos = False
> +        centos = False
>          os_dist = platform.linux_distribution()[0].split(' ')[0]
>          if os_dist == 'CentOS':
> -            self.centos = True
> +            centos = True
>
> -    def log_check(self):
> -        '''Test for checking error patterns in log files'''
>          # dmesg will be cleared out in autotest with dmesg -c before the test starts
>          # Let's check for /var/log/syslog instead
> -        if self.centos:
> +        if centos:
>              logfile = '/var/log/messages'
I think this if statement can be simplified along with the one above:
    if os_dist == 'CentOS':
        logfile = '/var/log/messages'
And drop the centos variable.

Thanks
Sam
>          else:
>              logfile = '/var/log/syslog'
> --
> 2.27.0
>
>
> --
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Krzysztof Kozlowski June 7, 2021, 10:01 a.m. UTC | #2
On 04/06/2021 05:04, Po-Hsu Lin wrote:
> On Thu, Jun 3, 2021 at 9:04 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@canonical.com> wrote:
>>
>> The setup() might not be called on second test run and actually autotest
>> design assumes setup() for given version will be called exactly once.
>> It is expected from setup() to modify only persistent state of the
>> system, so on next run it's execution is not needed.  Runtime-only
>> settings should not be configured via setup().
>>
>> This fixes failure on second test execution:
>>
>>     00:54:03 INFO |     START   ubuntu_boot.log_check   ubuntu_boot.log_check
>>     00:54:03 DEBUG| Persistent state client._record_indent now set to 2
>>     00:54:03 DEBUG| Persistent state client.unexpected_reboot now set to ('ubuntu_boot.log_check', 'ubuntu_boot.log_check')
>>     00:54:03 DEBUG| Waiting for pid 19403 for 300 seconds
>>     00:54:03 WARNI| System python is too old, crash handling disabled
>>     00:54:03 ERROR| Exception escaping from test:
>>     Traceback (most recent call last):
>>       File "/home/azure/autotest/client/shared/test.py", line 411, in _exec
>>         _call_test_function(self.execute, *p_args, **p_dargs)
>>       File "/home/azure/autotest/client/shared/test.py", line 830, in _call_test_function
>>         raise error.UnhandledTestFail(e)
>>     UnhandledTestFail: Unhandled AttributeError: 'ubuntu_boot' object has no attribute 'centos'
>>     Traceback (most recent call last):
>>       File "/home/azure/autotest/client/shared/test.py", line 823, in _call_test_function
>>         return func(*args, **dargs)
>>       File "/home/azure/autotest/client/shared/test.py", line 291, in execute
>>         postprocess_profiled_run, args, dargs)
>>       File "/home/azure/autotest/client/shared/test.py", line 212, in _call_run_once
>>         self.run_once(*args, **dargs)
>>       File "/home/azure/autotest/client/tests/ubuntu_boot/ubuntu_boot.py", line 63, in run_once
>>         if not self.log_check():
>>       File "/home/azure/autotest/client/tests/ubuntu_boot/ubuntu_boot.py", line 25, in log_check
>>         if self.centos:
>>     AttributeError: 'ubuntu_boot' object has no attribute 'centos'
>>
>> Fixes: e14cb0e1bd0c ("UBUNTU SAUCE: ubuntu_boot Centos Support")
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>>
>> ---
>>
>> Changes since v1:
>> 1. Add prefix to subject
>> ---
>>  ubuntu_boot/ubuntu_boot.py | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/ubuntu_boot/ubuntu_boot.py b/ubuntu_boot/ubuntu_boot.py
>> index a67f21d49fc7..b67a588285f8 100644
>> --- a/ubuntu_boot/ubuntu_boot.py
>> +++ b/ubuntu_boot/ubuntu_boot.py
>> @@ -12,17 +12,17 @@ class ubuntu_boot(test.test):
>>          cmd = 'yes "" | DEBIAN_FRONTEND=noninteractive apt-get install --yes --force-yes ' + ' '.join(pkgs)
>>          self.results = utils.system_output(cmd, retain_output=True)
>>
>> +    def log_check(self):
>> +        '''Test for checking error patterns in log files'''
>>          '''Centos Specific Boot Test Checks'''
>> -        self.centos = False
>> +        centos = False
>>          os_dist = platform.linux_distribution()[0].split(' ')[0]
>>          if os_dist == 'CentOS':
>> -            self.centos = True
>> +            centos = True
>>
>> -    def log_check(self):
>> -        '''Test for checking error patterns in log files'''
>>          # dmesg will be cleared out in autotest with dmesg -c before the test starts
>>          # Let's check for /var/log/syslog instead
>> -        if self.centos:
>> +        if centos:
>>              logfile = '/var/log/messages'
> I think this if statement can be simplified along with the one above:
>     if os_dist == 'CentOS':
>         logfile = '/var/log/messages'
> And drop the centos variable.
> 

True, I'll send a v2. Thanks!


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/ubuntu_boot/ubuntu_boot.py b/ubuntu_boot/ubuntu_boot.py
index a67f21d49fc7..b67a588285f8 100644
--- a/ubuntu_boot/ubuntu_boot.py
+++ b/ubuntu_boot/ubuntu_boot.py
@@ -12,17 +12,17 @@  class ubuntu_boot(test.test):
         cmd = 'yes "" | DEBIAN_FRONTEND=noninteractive apt-get install --yes --force-yes ' + ' '.join(pkgs)
         self.results = utils.system_output(cmd, retain_output=True)
 
+    def log_check(self):
+        '''Test for checking error patterns in log files'''
         '''Centos Specific Boot Test Checks'''
-        self.centos = False
+        centos = False
         os_dist = platform.linux_distribution()[0].split(' ')[0]
         if os_dist == 'CentOS':
-            self.centos = True
+            centos = True
 
-    def log_check(self):
-        '''Test for checking error patterns in log files'''
         # dmesg will be cleared out in autotest with dmesg -c before the test starts
         # Let's check for /var/log/syslog instead
-        if self.centos:
+        if centos:
             logfile = '/var/log/messages'
         else:
             logfile = '/var/log/syslog'