[U-Boot] [PATCH] test/py: Support setting up specific timeout

[U-Boot] [PATCH] test/py: Support setting up specific timeout

[Michal Simek]

Large file transfers, flash erasing and more complicated tests
requires more time to finish. Provide a way to setup specific
timeout directly in test.

For example description for 50s test:
timeout = 50000
with u_boot_console.temporary_timeout(timeout):
  u_boot_console.run_command(...)

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 test/py/u_boot_console_base.py | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py
index f7434363fbcb..82bbba43463e 100644
--- a/test/py/u_boot_console_base.py
+++ b/test/py/u_boot_console_base.py
@@ -56,6 +56,24 @@ class ConsoleDisableCheck(object):
         self.console.disable_check_count[self.check_type] -= 1
         self.console.eval_bad_patterns()
 
+
+class ConsoleSetupTimeout(object):
+    """Context manager (for Python's with statement) that temporarily setup
+    timeout for specific comnand. This is useful when execution time is greater
+    then default 30s."""
+
+    def __init__(self, console, timeout):
+        self.console = console
+        self.console.orig_timeout = self.console.p.timeout
+        self.console.p.timeout = timeout
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, extype, value, traceback):
+        self.console.p = self.console.get_spawn()
+        self.console.p.timeout = self.console.orig_timeout
+
 class ConsoleBase(object):
     """The interface through which test functions interact with the U-Boot
     console. This primarily involves executing shell commands, capturing their
@@ -391,3 +409,18 @@ class ConsoleBase(object):
         """
 
         return ConsoleDisableCheck(self, check_type)
+
+    def temporary_timeout(self, timeout):
+        """Temporarily setup different timeout for commands.
+
+        Create a new context manager (for use with the "with" statement) which
+        temporarily change timeout.
+
+        Args:
+            timeout: Time in miliseconds.
+
+        Returns:
+            A context manager object.
+        """
+
+        return ConsoleSetupTimeout(self, timeout)
-- 
1.9.1

[U-Boot] [PATCH] test/py: Support setting up specific timeout

[Michal Simek]

On 18.5.2016 15:11, Michal Simek wrote:
> Large file transfers, flash erasing and more complicated tests
> requires more time to finish. Provide a way to setup specific
> timeout directly in test.
> 
> For example description for 50s test:
> timeout = 50000
> with u_boot_console.temporary_timeout(timeout):
>   u_boot_console.run_command(...)
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
>  test/py/u_boot_console_base.py | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py
> index f7434363fbcb..82bbba43463e 100644
> --- a/test/py/u_boot_console_base.py
> +++ b/test/py/u_boot_console_base.py
> @@ -56,6 +56,24 @@ class ConsoleDisableCheck(object):
>          self.console.disable_check_count[self.check_type] -= 1
>          self.console.eval_bad_patterns()
>  
> +
> +class ConsoleSetupTimeout(object):
> +    """Context manager (for Python's with statement) that temporarily setup
> +    timeout for specific comnand. This is useful when execution time is greater
> +    then default 30s."""
> +
> +    def __init__(self, console, timeout):
> +        self.console = console
> +        self.console.orig_timeout = self.console.p.timeout
> +        self.console.p.timeout = timeout
> +
> +    def __enter__(self):
> +        return self
> +
> +    def __exit__(self, extype, value, traceback):
> +        self.console.p = self.console.get_spawn()

This line should be there. It is resetting console. I will do more
testing on real hw not on qemu.

Thanks,
Michal

> +        self.console.p.timeout = self.console.orig_timeout
> +
>  class ConsoleBase(object):
>      """The interface through which test functions interact with the U-Boot
>      console. This primarily involves executing shell commands, capturing their
> @@ -391,3 +409,18 @@ class ConsoleBase(object):
>          """
>  
>          return ConsoleDisableCheck(self, check_type)
> +
> +    def temporary_timeout(self, timeout):
> +        """Temporarily setup different timeout for commands.
> +
> +        Create a new context manager (for use with the "with" statement) which
> +        temporarily change timeout.
> +
> +        Args:
> +            timeout: Time in miliseconds.
> +
> +        Returns:
> +            A context manager object.
> +        """
> +
> +        return ConsoleSetupTimeout(self, timeout)
> 


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160518/f69b853c/attachment.sig>

[U-Boot] [PATCH] test/py: Support setting up specific timeout

[Michal Simek]

On 18.5.2016 16:02, Michal Simek wrote:
> On 18.5.2016 15:11, Michal Simek wrote:
>> Large file transfers, flash erasing and more complicated tests
>> requires more time to finish. Provide a way to setup specific
>> timeout directly in test.
>>
>> For example description for 50s test:
>> timeout = 50000
>> with u_boot_console.temporary_timeout(timeout):
>>   u_boot_console.run_command(...)
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>>  test/py/u_boot_console_base.py | 33 +++++++++++++++++++++++++++++++++
>>  1 file changed, 33 insertions(+)
>>
>> diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py
>> index f7434363fbcb..82bbba43463e 100644
>> --- a/test/py/u_boot_console_base.py
>> +++ b/test/py/u_boot_console_base.py
>> @@ -56,6 +56,24 @@ class ConsoleDisableCheck(object):
>>          self.console.disable_check_count[self.check_type] -= 1
>>          self.console.eval_bad_patterns()
>>  
>> +
>> +class ConsoleSetupTimeout(object):
>> +    """Context manager (for Python's with statement) that temporarily setup
>> +    timeout for specific comnand. This is useful when execution time is greater
>> +    then default 30s."""
>> +
>> +    def __init__(self, console, timeout):
>> +        self.console = console
>> +        self.console.orig_timeout = self.console.p.timeout
>> +        self.console.p.timeout = timeout
>> +
>> +    def __enter__(self):
>> +        return self
>> +
>> +    def __exit__(self, extype, value, traceback):
>> +        self.console.p = self.console.get_spawn()
> 
> This line should be there. It is resetting console. I will do more
> testing on real hw not on qemu.
> 
> Thanks,
> Michal
> 
>> +        self.console.p.timeout = self.console.orig_timeout

This is also wrong because self.console.p is closed when timeout occurs
which caused exception which calls cleanup_spawn which close p.

This exit should be like this.
    def __exit__(self, extype, value, traceback):
        if not self.console.p:
            return
        self.console.p.timeout = self.orig_timeout

Thanks,
Michal

[U-Boot] [PATCH] test/py: Support setting up specific timeout

[Stephen Warren]

On 05/18/2016 07:11 AM, Michal Simek wrote:
> Large file transfers, flash erasing and more complicated tests
> requires more time to finish. Provide a way to setup specific
> timeout directly in test.
>
> For example description for 50s test:
> timeout = 50000
> with u_boot_console.temporary_timeout(timeout):
>    u_boot_console.run_command(...)

> diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py

>           self.console.eval_bad_patterns()
>
> +

Nit: No need for two blank lines.

> +class ConsoleSetupTimeout(object):
> +    """Context manager (for Python's with statement) that temporarily setup

Nit: s/set/sets up/

> +    timeout for specific comnand. This is useful when execution time is greater

Nit: s/comnand/command/

> +    then default 30s."""
> +
> +    def __init__(self, console, timeout):
> +        self.console = console
> +        self.console.orig_timeout = self.console.p.timeout
> +        self.console.p.timeout = timeout

I'd suggest storing console.p not console. That way, even if console.p 
is replaced because of an issue, you still have a handle to it and don't 
have to make the code in __exit__ conditional:

self.p = console.p
self.orig_timeout = self.p.timeout
self.p.timeout = timeout

> +
> +    def __enter__(self):
> +        return self
> +
> +    def __exit__(self, extype, value, traceback):
> +        self.console.p = self.console.get_spawn()
> +        self.console.p.timeout = self.console.orig_timeout

This then doesn't need the conditionals you mentioned in your response 
to this patch, and can then be:

self.p.timeout = self.orig_timeout

> +    def temporary_timeout(self, timeout):
> +        """Temporarily setup different timeout for commands.

Nit: s/setup/set up/

> +        Args:
> +            timeout: Time in miliseconds.

Nit: s/miliseconds/milliseconds/

[U-Boot] [PATCH] test/py: Support setting up specific timeout

[Michal Simek]

On 18.5.2016 18:43, Stephen Warren wrote:
> On 05/18/2016 07:11 AM, Michal Simek wrote:
>> Large file transfers, flash erasing and more complicated tests
>> requires more time to finish. Provide a way to setup specific
>> timeout directly in test.
>>
>> For example description for 50s test:
>> timeout = 50000
>> with u_boot_console.temporary_timeout(timeout):
>>    u_boot_console.run_command(...)
> 
>> diff --git a/test/py/u_boot_console_base.py
>> b/test/py/u_boot_console_base.py
> 
>>           self.console.eval_bad_patterns()
>>
>> +
> 
> Nit: No need for two blank lines.
> 
>> +class ConsoleSetupTimeout(object):
>> +    """Context manager (for Python's with statement) that temporarily
>> setup
> 
> Nit: s/set/sets up/
> 
>> +    timeout for specific comnand. This is useful when execution time
>> is greater
> 
> Nit: s/comnand/command/
> 
>> +    then default 30s."""
>> +
>> +    def __init__(self, console, timeout):
>> +        self.console = console
>> +        self.console.orig_timeout = self.console.p.timeout
>> +        self.console.p.timeout = timeout
> 
> I'd suggest storing console.p not console. That way, even if console.p
> is replaced because of an issue, you still have a handle to it and don't
> have to make the code in __exit__ conditional:
> 
> self.p = console.p
> self.orig_timeout = self.p.timeout
> self.p.timeout = timeout
> 
>> +
>> +    def __enter__(self):
>> +        return self
>> +
>> +    def __exit__(self, extype, value, traceback):
>> +        self.console.p = self.console.get_spawn()
>> +        self.console.p.timeout = self.console.orig_timeout
> 
> This then doesn't need the conditionals you mentioned in your response
> to this patch, and can then be:
> 
> self.p.timeout = self.orig_timeout
> 
>> +    def temporary_timeout(self, timeout):
>> +        """Temporarily setup different timeout for commands.
> 
> Nit: s/setup/set up/
> 
>> +        Args:
>> +            timeout: Time in miliseconds.
> 
> Nit: s/miliseconds/milliseconds/
> 

I have fixed all of these comments and send v2.

Thanks,
Michal