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

[Stephen Warren]

On 09/19/2018 10:45 AM, Heinrich Schuchardt wrote:
> 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.

I was describing the fact that not all Python exception types are a 
subclass of Exception so "except Exception" won't catch some of them. 
However, that doesn't appear to be relevant here. I'll send an 
alternative patch in a minute.