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

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

[Heinrich Schuchardt]

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@gmx.de>
Reviewed-by: Simon Glass <sjg@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
---
 test/py/u_boot_spawn.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py
index b011a3e3da..7ee876b101 100644
--- a/test/py/u_boot_spawn.py
+++ b/test/py/u_boot_spawn.py
@@ -181,6 +181,9 @@ class Spawn(object):
                 # unlimited substitutions, but in practice the version of
                 # Python in Ubuntu 14.04 appears to default to count=2!
                 self.buf = self.re_vt100.sub('', self.buf, count=1000000)
+        except OSError, EOFError:
+            # Reading the the console may result in an error. Catch it.
+            pass
         finally:
             if self.logfile_read:
                 self.logfile_read.flush()
-- 
2.18.0

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

[Stephen Warren]

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@gmx.de>
> Reviewed-by: Simon Glass <sjg@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.

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

[Alexander Graf]

On 18.09.18 10:21, 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.

I agree with Stephen that the actual error this is trying to fix is not
obvious from the description. Could you please clarify what exactly is
going wrong and why this is the correct fix for it?


Thanks,

Alex

> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Reviewed-by: Simon Glass <sjg@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
> ---
>  test/py/u_boot_spawn.py | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py
> index b011a3e3da..7ee876b101 100644
> --- a/test/py/u_boot_spawn.py
> +++ b/test/py/u_boot_spawn.py
> @@ -181,6 +181,9 @@ class Spawn(object):
>                  # unlimited substitutions, but in practice the version of
>                  # Python in Ubuntu 14.04 appears to default to count=2!
>                  self.buf = self.re_vt100.sub('', self.buf, count=1000000)
> +        except OSError, EOFError:
> +            # Reading the the console may result in an error. Catch it.
> +            pass
>          finally:
>              if self.logfile_read:
>                  self.logfile_read.flush()
> 

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

[Heinrich Schuchardt]

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@gmx.de>
>> Reviewed-by: Simon Glass <sjg@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.

An example can be seen at the end of
https://api.travis-ci.org/v3/job/429566431/log.txt .
I have copied the relevant part of the output below.

Please, observe that the output that has occurred until the segmentation
fault is not displayed. Instead we see lines with "INTERNAL ERROR>".

The same can be observed when executing

make mrproper && make tests

Further below I have copied the output with the patch. Here the OSError
is caught and the py tests continue as expected.

Best regards

Heinrich

=== Output without the patch ===

test/py/tests/test_efi_selftest.py F
INTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File
"/tmp/venv/local/lib/python2.7/site-packages/_pytest/main.py", line 178,
in wrap_session
INTERNALERROR>     session.exitstatus = doit(config, session) or 0
INTERNALERROR>   File
"/tmp/venv/local/lib/python2.7/site-packages/_pytest/main.py", line 215,
in _main
INTERNALERROR>     config.hook.pytest_runtestloop(session=session)
INTERNALERROR>   File
"/tmp/venv/local/lib/python2.7/site-packages/pluggy/hooks.py", line 258,
in __call__
INTERNALERROR>     return self._hookexec(self, self._nonwrappers +
self._wrappers, kwargs)
INTERNALERROR>   File
"/tmp/venv/local/lib/python2.7/site-packages/pluggy/manager.py", line
67, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook, methods, kwargs)
INTERNALERROR>   File
"/tmp/venv/local/lib/python2.7/site-packages/pluggy/manager.py", line
61, in <lambda>
INTERNALERROR>     firstresult=hook.spec_opts.get('firstresult'),
INTERNALERROR>   File
"/tmp/venv/local/lib/python2.7/site-packages/pluggy/callers.py", line
201, in _multicall
INTERNALERROR>     return outcome.get_result()
INTERNALERROR>   File
"/tmp/venv/local/lib/python2.7/site-packages/pluggy/callers.py", line
77, in get_result
INTERNALERROR>     _reraise(*ex)  # noqa
INTERNALERROR>   File
"/tmp/venv/local/lib/python2.7/site-packages/pluggy/callers.py", line
180, in _multicall
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File
"/tmp/venv/local/lib/python2.7/site-packages/_pytest/main.py", line 236,
in pytest_runtestloop
INTERNALERROR>     item.config.hook.pytest_runtest_protocol(item=item,
nextitem=nextitem)
INTERNALERROR>   File
"/tmp/venv/local/lib/python2.7/site-packages/pluggy/hooks.py", line 258,
in __call__
INTERNALERROR>     return self._hookexec(self, self._nonwrappers +
self._wrappers, kwargs)
INTERNALERROR>   File
"/tmp/venv/local/lib/python2.7/site-packages/pluggy/manager.py", line
67, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook, methods, kwargs)
INTERNALERROR>   File
"/tmp/venv/local/lib/python2.7/site-packages/pluggy/manager.py", line
61, in <lambda>
INTERNALERROR>     firstresult=hook.spec_opts.get('firstresult'),
INTERNALERROR>   File
"/tmp/venv/local/lib/python2.7/site-packages/pluggy/callers.py", line
201, in _multicall
INTERNALERROR>     return outcome.get_result()
INTERNALERROR>   File
"/tmp/venv/local/lib/python2.7/site-packages/pluggy/callers.py", line
77, in get_result
INTERNALERROR>     _reraise(*ex)  # noqa
INTERNALERROR>   File
"/tmp/venv/local/lib/python2.7/site-packages/pluggy/callers.py", line
180, in _multicall
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File
"/home/travis/build/agraf/u-boot/test/py/conftest.py", line 577, in
pytest_runtest_protocol
INTERNALERROR>     console.drain_console()
INTERNALERROR>   File
"/home/travis/build/agraf/u-boot/test/py/u_boot_console_base.py", line
306, in drain_console
INTERNALERROR>     self.p.expect(['This should never match U-Boot output'])
INTERNALERROR>   File
"/home/travis/build/agraf/u-boot/test/py/u_boot_spawn.py", line 174, in
expect
INTERNALERROR>     c = os.read(self.fd, 1024)
INTERNALERROR> OSError: [Errno 5] Input/output error


=== Output with the patch ===

$ make tests
./test/run
+make O=/home/user/workspace/u-boot-build/denx/build-sandbox -s
sandbox_defconfig
+make O=/home/user/workspace/u-boot-build/denx/build-sandbox -s -j8
===========================================================================
test session starts
============================================================================
platform linux2 -- Python 2.7.15+, pytest-3.6.4, py-1.6.0, pluggy-0.6.0
rootdir: /home/user/workspace/u-boot-build/denx/test/py, inifile: pytest.ini
collected 411 items



test/py/tests/test_000_version.py .

                   [  0%]
test/py/tests/test_avb.py sssss

                   [  0%]
test/py/tests/test_bind.py ..

                   [  0%]
test/py/tests/test_dfu.py s

                   [  0%]
test/py/tests/test_efi_loader.py .sssss

                   [  0%]
test/py/tests/test_efi_selftest.py FFFFF

                   [  0%]
test/py/tests/test_env.py ............

                   [  0%]
test/py/tests/test_fit.py F

                   [  0%]
test/py/tests/test_fpga.py sssssssssssssssssssssssssss

                   [  0%]
test/py/tests/test_gpt.py sssssss

                   [  0%]
test/py/tests/test_help.py .

                   [  0%]
test/py/tests/test_hush_if_test.py
.......................................................
                                                         [  0%]
test/py/tests/test_log.py ..

                   [  0%]
test/py/tests/test_md.py ..

                   [  0%]
test/py/tests/test_mmc_rd.py s

                   [  0%]
test/py/tests/test_net.py .sssss

                   [  0%]
test/py/tests/test_ofplatdata.py s

                   [  0%]
test/py/tests/test_sandbox_exit.py ..

                   [  0%]
test/py/tests/test_sf.py ssss

                   [  0%]
test/py/tests/test_shell_basics.py ....

                   [  0%]
test/py/tests/test_sleep.py .

                   [  0%]
test/py/tests/test_tpm2.py ...........

                   [  0%]
test/py/tests/test_ums.py s

                   [  0%]
test/py/tests/test_unknown_cmd.py .

                   [  0%]
test/py/tests/test_ut.py
........................................................................................................................................................................
[  0%]
test/py/tests/test_vboot.py F[sudo] password for user:

                                                               [  0%]
test/py/tests/test_fs/test_basic.py
sssssssssssssssssssssssssssssssssssssss
                                                        [  0%]
test/py/tests/test_fs/test_ext.py ssssssssssssssssss

                   [  0%]
test/py/tests/test_fs/test_mkdir.py ssssssssssss

                   [  0%]
test/py/tests/test_fs/test_unlink.py ssssssssssssss

=================================================================================
FAILURES
=================================================================================
____________________________________________________________________________
test_efi_selftest
_____________________________________________________________________________

u_boot_console = <u_boot_console_sandbox.ConsoleSandbox object at
0x7fb32ef7cfd0>

    @pytest.mark.buildconfigspec('cmd_bootefi_selftest')
    def test_efi_selftest(u_boot_console):
        """
        Run bootefi selftest
        """

        u_boot_console.run_command(cmd='setenv efi_selftest')
        u_boot_console.run_command(cmd='bootefi selftest',
wait_for_prompt=False)
        m = u_boot_console.p.expect(['Summary: 0 failures', 'Press any
key'])
        if m != 0:
>               raise Exception('Failures occurred during the EFI selftest')
E     Exception: Failures occurred during the EFI selftest

test/py/tests/test_efi_selftest.py:19: Exception
---------------------------------------------------------------------------
Captured stdout call
---------------------------------------------------------------------------
=> setenv efi_selftest
=> => bootefi selftest
Scanning disk mmc2.blk...
^[[44;172R______________________________________________________________________
test_efi_selftest_device_tree
_______________________________________________________________________

u_boot_console = <u_boot_console_sandbox.ConsoleSandbox object at
0x7fb32ef7cfd0>

    @pytest.mark.buildconfigspec('cmd_bootefi_selftest')
    @pytest.mark.buildconfigspec('of_control')
    def test_efi_selftest_device_tree(u_boot_console):
        u_boot_console.run_command(cmd='setenv efi_selftest list')
>       output = u_boot_console.run_command('bootefi selftest')

test/py/tests/test_efi_selftest.py:30:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <u_boot_console_sandbox.ConsoleSandbox object@0x7fb32ef7cfd0>,
cmd = '', wait_for_echo = True, send_nl = True, wait_for_prompt = True

    def run_command(self, cmd, wait_for_echo=True, send_nl=True,
            wait_for_prompt=True):
        """Execute a command via the U-Boot console.

            The command is always sent to U-Boot.

            U-Boot echoes any command back to its output, and this function
            typically waits for that to occur. The wait can be disabled
by setting
            wait_for_echo=False, which is useful e.g. when sending CTRL-C to
            interrupt a long-running command such as "ums".

            Command execution is typically triggered by sending a newline
            character. This can be disabled by setting send_nl=False,
which is
            also useful when sending CTRL-C.

            This function typically waits for the command to finish
executing, and
            returns the console output that it generated. This can be
disabled by
            setting wait_for_prompt=False, which is useful when invoking
a long-
            running command such as "ums".

            Args:
                cmd: The command to send.
                wait_for_echo: Boolean indicating whether to wait for
U-Boot to
                    echo the command text back to its output.
                send_nl: Boolean indicating whether to send a newline
character
                    after the command string.
                wait_for_prompt: Boolean indicating whether to wait for the
                    command prompt to be sent by U-Boot. This typically
occurs
                    immediately after the command has been executed.

            Returns:
                If wait_for_prompt == False:
                    Nothing.
                Else:
                    The output from U-Boot during command execution. In
other
                    words, the text U-Boot emitted between the point it
echod the
                    command string and emitted the subsequent command
prompts.
            """

        if self.at_prompt and \
                self.at_prompt_logevt != self.logstream.logfile.cur_evt:
            self.logstream.write(self.prompt, implicit=True)

        try:
            self.at_prompt = False
            if send_nl:
                cmd += '\n'
            while cmd:
                # Limit max outstanding data, so UART FIFOs don't overflow
                chunk = cmd[:self.max_fifo_fill]
                cmd = cmd[self.max_fifo_fill:]
                self.p.send(chunk)
                if not wait_for_echo:
                    continue
                chunk = re.escape(chunk)
                chunk = chunk.replace('\\\n', '[\r\n]')
                m = self.p.expect([chunk] + self.bad_patterns)
                if m != 0:
                    self.at_prompt = False
                    raise Exception('Bad pattern found on console: ' +
                                    self.bad_pattern_ids[m - 1])
            if not wait_for_prompt:
                return
            m = self.p.expect([self.prompt_compiled] + self.bad_patterns)
            if m != 0:
                self.at_prompt = False
                raise Exception('Bad pattern found on console: ' +
>                               self.bad_pattern_ids[m - 1])
E                               TypeError: unsupported operand type(s)
for -: 'NoneType' and 'int'

test/py/u_boot_console_base.py:207: TypeError
--------------------------------------------------------------------------
Captured stdout setup
---------------------------------------------------------------------------
/u-boot

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

[Stephen Warren]

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@gmx.de>
>>> Reviewed-by: Simon Glass <sjg@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.

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

[Simon Glass]

Hi Heinrich,

On 18 September 2018 at 18:43, Heinrich Schuchardt <xypron.glpk@gmx.de> 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@gmx.de>
>>> Reviewed-by: Simon Glass <sjg@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.

Your explanation makes sense to me. Did I miss Stephen's objection? What is it?

Can you please add the above paragraph or part of it to the commit
message and to a code comment?

Regards,
Simon

[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@gmx.de>
>>>> Reviewed-by: Simon Glass <sjg@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

[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@gmx.de>
>>>>> Reviewed-by: Simon Glass <sjg@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.

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

[Stephen Warren]

On 09/20/2018 12:19 PM, Heinrich Schuchardt wrote:
> Spawn.exept has a try block without 'except'.
> 
> When the py test is running it is connected via pipes to the U-Boot
> process. If the U-Boot process ends prematurely, e.g. due to a
> segmentation fault, the pipes are broken. Trying to read from a broken
> pipe results in an OSError. Catch the exception and treat the broken pipe
> like any other end of output. By returning None expect() indicates that
> the output does not match.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> I suggest that Alex takes the patch because we need it when working on
> the efi-next branch.
> 
> v3
> 	add more comments

Adding comments doesn't solve the fundamental problem with this patch. 
I'll send an alternative patch in a minute. That patch will hide 
exceptions only during a very specific portion of time, rather than just 
ignoring all of them.

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

[Heinrich Schuchardt]

Spawn.exept has a try block without 'except'.

When the py test is running it is connected via pipes to the U-Boot
process. If the U-Boot process ends prematurely, e.g. due to a
segmentation fault, the pipes are broken. Trying to read from a broken
pipe results in an OSError. Catch the exception and treat the broken pipe
like any other end of output. By returning None expect() indicates that
the output does not match.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
I suggest that Alex takes the patch because we need it when working on
the efi-next branch.

v3
	add more comments
v2
	replace TAB by spaces
	fix type in subject
---
 test/py/u_boot_spawn.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/test/py/u_boot_spawn.py b/test/py/u_boot_spawn.py
index b011a3e3da..adc1d00287 100644
--- a/test/py/u_boot_spawn.py
+++ b/test/py/u_boot_spawn.py
@@ -181,6 +181,14 @@ class Spawn(object):
                 # unlimited substitutions, but in practice the version of
                 # Python in Ubuntu 14.04 appears to default to count=2!
                 self.buf = self.re_vt100.sub('', self.buf, count=1000000)
+        except OSError, EOFError:
+            # When the py test is running it is connected via pipes to the
+            # U-Boot process. If the U-Boot process ends prematurely, e.g. due
+            # to a segmentation fault, the pipes are broken. Trying to read
+            # from a broken pipe results in an OSError. Treat the broken pipe
+            # like any other end of output. By returning None expect()
+            # indicates that the output does not match.
+            pass
         finally:
             if self.logfile_read:
                 self.logfile_read.flush()
-- 
2.18.0