[U-Boot] [PATCH V2] test/py: ignore console read exceptions after test failure

[U-Boot] [PATCH V2] test/py: ignore console read exceptions after test failure

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

After a test has failed, test/py drains the U-Boot console log to ensure
that any relevant output is captured. At this point, we don't care about
detecting any additional errors, since the test is already known to have
failed, and U-Boot will be restarted. To ensure that the test cleanup code
is not interrupted, and can correctly terminate the log sections for the
failed test, ignore any exception that occurs while reading the U-Boot
console output during this limited period of time.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v2:
* Combine the two except cases into one, since one was a superset of the
  other.
* Fix TAB-vs-spaces issue in identation.
---
 test/py/u_boot_console_base.py | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py
index a14bad6e8c50..326b2ac51fbf 100644
--- a/test/py/u_boot_console_base.py
+++ b/test/py/u_boot_console_base.py
@@ -304,7 +304,17 @@ class ConsoleBase(object):
             # Wait for something U-Boot will likely never send. This will
             # cause the console output to be read and logged.
             self.p.expect(['This should never match U-Boot output'])
-        except u_boot_spawn.Timeout:
+        except:
+            # We expect a timeout, since U-Boot won't print what we waited
+            # for. Squash it when it happens.
+            #
+            # Squash any other exception too. This function is only used to
+            # drain (and log) the U-Boot console output after a failed test.
+            # The U-Boot process will be restarted, or target board reset, once
+            # this function returns. So, we don't care about detecting any
+            # additional errors, so they're squashed so that the rest of the
+            # post-test-failure cleanup code can continue operation, and
+            # correctly terminate any log sections, etc.
             pass
         finally:
             self.p.timeout = orig_timeout
-- 
2.18.0

[U-Boot] [PATCH V2] test/py: ignore console read exceptions after test failure

[Heinrich Schuchardt]

On 09/21/2018 12:55 AM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> After a test has failed, test/py drains the U-Boot console log to ensure
> that any relevant output is captured. At this point, we don't care about
> detecting any additional errors, since the test is already known to have
> failed, and U-Boot will be restarted. To ensure that the test cleanup code
> is not interrupted, and can correctly terminate the log sections for the
> failed test, ignore any exception that occurs while reading the U-Boot
> console output during this limited period of time.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> v2:
> * Combine the two except cases into one, since one was a superset of the
>   other.
> * Fix TAB-vs-spaces issue in identation.
> ---
>  test/py/u_boot_console_base.py | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py
> index a14bad6e8c50..326b2ac51fbf 100644
> --- a/test/py/u_boot_console_base.py
> +++ b/test/py/u_boot_console_base.py
> @@ -304,7 +304,17 @@ class ConsoleBase(object):
>              # Wait for something U-Boot will likely never send. This will
>              # cause the console output to be read and logged.
>              self.p.expect(['This should never match U-Boot output'])
> -        except u_boot_spawn.Timeout:
> +        except:
> +            # We expect a timeout, since U-Boot won't print what we waited
> +            # for. Squash it when it happens.
> +            #
> +            # Squash any other exception too. This function is only used to
> +            # drain (and log) the U-Boot console output after a failed test.
> +            # The U-Boot process will be restarted, or target board reset, once
> +            # this function returns. So, we don't care about detecting any
> +            # additional errors, so they're squashed so that the rest of the
> +            # post-test-failure cleanup code can continue operation, and
> +            # correctly terminate any log sections, etc.
>              pass
>          finally:
>              self.p.timeout = orig_timeout
> 
This patch only covers this single usage of the expect(). Expect() is
called from many test cases directly. In all these cases we still will
not catch OSErrror if the U-Boot binary terminates breaking the pipes.

That is why in my patch I put the except statement into the except()
method itself.

What problem do you see in catching the exception where it is arising?

Best regards

Heinrich

[U-Boot] [PATCH V2] test/py: ignore console read exceptions after test failure

[Stephen Warren]

On 09/20/2018 11:50 PM, Heinrich Schuchardt wrote:
> On 09/21/2018 12:55 AM, Stephen Warren wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> After a test has failed, test/py drains the U-Boot console log to ensure
>> that any relevant output is captured. At this point, we don't care about
>> detecting any additional errors, since the test is already known to have
>> failed, and U-Boot will be restarted. To ensure that the test cleanup code
>> is not interrupted, and can correctly terminate the log sections for the
>> failed test, ignore any exception that occurs while reading the U-Boot
>> console output during this limited period of time.
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>> ---
>> v2:
>> * Combine the two except cases into one, since one was a superset of the
>>    other.
>> * Fix TAB-vs-spaces issue in identation.
>> ---
>>   test/py/u_boot_con

sole_base.py | 12 +++++++++++-
>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py
>> index a14bad6e8c50..326b2ac51fbf 100644
>> --- a/test/py/u_boot_console_base.py
>> +++ b/test/py/u_boot_console_base.py
>> @@ -304,7 +304,17 @@ class ConsoleBase(object):
>>               # Wait for something U-Boot will likely never send. This will
>>               # cause the console output to be read and logged.
>>               self.p.expect(['This should never match U-Boot output'])
>> -        except u_boot_spawn.Timeout:
>> +        except:
>> +            # We expect a timeout, since U-Boot won't print what we waited
>> +            # for. Squash it when it happens.
>> +            #
>> +            # Squash any other exception too. This function is only used to
>> +            # drain (and log) the U-Boot console output after a failed test.
>> +            # The U-Boot process will be restarted, or target board reset, once
>> +            # this function returns. So, we don't care about detecting any
>> +            # additional errors, so they're squashed so that the rest of the
>> +            # post-test-failure cleanup code can continue operation, and
>> +            # correctly terminate any log sections, etc.
>>               pass
>>           finally:
>>               self.p.timeout = orig_timeout
>>
> This patch only covers this single usage of the expect(). Expect() is
> called from many test cases directly. In all these cases we still will
> not catch OSErrror if the U-Boot binary terminates breaking the pipes.

That is deliberate since this approach is correct.

> That is why in my patch I put the except statement into the except()
> method itself.
> 
> What problem do you see in catching the exception where it is arising?

Because it ignores the exception. The correct approach is to propagate 
the exception, fail the test, and then clean up. Ignoring the exception 
and returning nothing from the expect logic may or may not actually lead 
to detecting the error, depending on how the individual test code ends 
up checking the result from the expect call. Many tests don't, because 
they know that if an error occurs, and exception will be thrown through 
the test code and fail it without the test code having to explicitly 
check for errors.

[U-Boot] [PATCH V2] test/py: ignore console read exceptions after test failure

[Stephen Warren]

On 09/21/2018 09:39 AM, Stephen Warren wrote:
> On 09/20/2018 11:50 PM, Heinrich Schuchardt wrote:
>> On 09/21/2018 12:55 AM, Stephen Warren wrote:
>>> From: Stephen Warren <swarren@nvidia.com>
>>>
>>> After a test has failed, test/py drains the U-Boot console log to ensure
>>> that any relevant output is captured. At this point, we don't care about
>>> detecting any additional errors, since the test is already known to have
>>> failed, and U-Boot will be restarted. To ensure that the test cleanup 
>>> code
>>> is not interrupted, and can correctly terminate the log sections for the
>>> failed test, ignore any exception that occurs while reading the U-Boot
>>> console output during this limited period of time.
>>>
>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>>> ---
>>> v2:
>>> * Combine the two except cases into one, since one was a superset of the
>>>    other.
>>> * Fix TAB-vs-spaces issue in identation.
>>> ---
>>>   test/py/u_boot_console_base.py | 12 +++++++++++-
>>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/test/py/u_boot_console_base.py 
>>> b/test/py/u_boot_console_base.py
>>> index a14bad6e8c50..326b2ac51fbf 100644
>>> --- a/test/py/u_boot_console_base.py
>>> +++ b/test/py/u_boot_console_base.py
>>> @@ -304,7 +304,17 @@ class ConsoleBase(object):
>>>               # Wait for something U-Boot will likely never send. 
>>> This will
>>>               # cause the console output to be read and logged.
>>>               self.p.expect(['This should never match U-Boot output'])
>>> -        except u_boot_spawn.Timeout:
>>> +        except:
>>> +            # We expect a timeout, since U-Boot won't print what we 
>>> waited
>>> +            # for. Squash it when it happens.
>>> +            #
>>> +            # Squash any other exception too. This function is only 
>>> used to
>>> +            # drain (and log) the U-Boot console output after a 
>>> failed test.
>>> +            # The U-Boot process will be restarted, or target board 
>>> reset, once
>>> +            # this function returns. So, we don't care about 
>>> detecting any
>>> +            # additional errors, so they're squashed so that the 
>>> rest of the
>>> +            # post-test-failure cleanup code can continue operation, 
>>> and
>>> +            # correctly terminate any log sections, etc.
>>>               pass
>>>           finally:
>>>               self.p.timeout = orig_timeout
>>>
>> This patch only covers this single usage of the expect(). Expect() is
>> called from many test cases directly. In all these cases we still will
>> not catch OSErrror if the U-Boot binary terminates breaking the pipes.
> 
> That is deliberate since this approach is correct.
> 
>> That is why in my patch I put the except statement into the except()
>> method itself.
>>
>> What problem do you see in catching the exception where it is arising?
> 
> Because it ignores the exception. The correct approach is to propagate 
> the exception, fail the test, and then clean up. Ignoring the exception 
> and returning nothing from the expect logic may or may not actually lead 
> to detecting the error, depending on how the individual test code ends 
> up checking the result from the expect call. Many tests don't, because 
> they know that if an error occurs, and exception will be thrown through 
> the test code and fail it without the test code having to explicitly 
> check for errors.

Tom, do you plan to apply this patch? Thanks.

[U-Boot] [U-Boot, V2] test/py: ignore console read exceptions after test failure

[Tom Rini]

On Thu, Sep 20, 2018 at 04:55:03PM -0600, Stephen Warren wrote:

> From: Stephen Warren <swarren@nvidia.com>
> 
> After a test has failed, test/py drains the U-Boot console log to ensure
> that any relevant output is captured. At this point, we don't care about
> detecting any additional errors, since the test is already known to have
> failed, and U-Boot will be restarted. To ensure that the test cleanup code
> is not interrupted, and can correctly terminate the log sections for the
> failed test, ignore any exception that occurs while reading the U-Boot
> console output during this limited period of time.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20181006/c469c366/attachment.sig>