[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 at 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 at 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.