[U-Boot] [PATCH 1/1] test/py: catch errors occurring when reading the console

[Heinrich Schuchardt]

On 09/19/2018 05:24 PM, Stephen Warren wrote:
> On 09/18/2018 06:43 PM, Heinrich Schuchardt wrote:
>> On 09/18/2018 07:23 PM, Stephen Warren wrote:
>>> On 09/18/2018 11:21 AM, Heinrich Schuchardt wrote:
>>>> Spawn.exept has a try block without 'except'.
>>>>
>>>> If no output is available an OSError may arise. Catch this exception
>>>> and
>>>> continue testing.
>>>>
>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>>> ---
>>>> I suggest that Alex takes the patch because we need it when working on
>>>> the
>>>> efi-next branch.
>>>>
>>>> v2
>>>>      replace TAB by spaces
>>>>      fix typo in subject
>>>
>>> I'll point out that I still object to this.
>>
>> On 09/18/2018 07:06 PM, Stephen Warren wrote:
>>> This doesn't make sense at all. It catches all errors and ignores them.
>>> It'll turn any error condition into a timeout (presumably, the expected
>>> data being waited for will never appear) rather than dealing with it
>>> immediately (due to the thrown exception). Why is this needed?
>>
>> When the py test is running it is connected via pipes to the U-Boot
>> process. If the U-Boot process ends due to a segmentation fault the
>> pipes are broken. Trying to read from a broken pipe results in an
>> OSError.
>>
>> Before this patch this leads to an uncaught error in the Python script.
>> The output that has occured up to this point is lost and not displayed.
>> All further tests for the configuration are not run.
> 
> Ah. I think that's because Python is throwing an error rather than an
> exception. I would have assumed that py.test caught errors as well as
> exceptions, but perhaps not. I think what we should do here is catch
> errors and translate them into a new thrown exception. That way,
> everything outside the function would treat this as just another type of
> exception, and hence still capture/flush the output, mark the test
> immediately failed rather than having to wait for a timeout, etc.
> 

Hello Stephen,

the above confuses me.

OSError is an exception. What do you mean by error in this context?

The method Spawn.expect() is used to compare actual output to expected
output. If a pipe is closed, this is obviously the end of the output.
Why should we treat it differently to any other end of output?

The except OSError statement is right at the end of the method. Why
would you expect the method to wait for a timeout?

If the OSError excepton is caught, the method returns None and the test
is known to have failed.

Best regards

Heinrich