* [U-Boot] [PATCH 1/1] test/py: catch errors occurring when reading the console @ 2018-09-18 17:21 Heinrich Schuchardt 2018-09-18 17:23 ` Stephen Warren 2018-09-18 22:05 ` Alexander Graf 0 siblings, 2 replies; 10+ messages in thread From: Heinrich Schuchardt @ 2018-09-18 17:21 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH 1/1] test/py: catch errors occurring when reading the console 2018-09-18 17:21 [U-Boot] [PATCH 1/1] test/py: catch errors occurring when reading the console Heinrich Schuchardt @ 2018-09-18 17:23 ` Stephen Warren 2018-09-19 0:43 ` Heinrich Schuchardt 2018-09-18 22:05 ` Alexander Graf 1 sibling, 1 reply; 10+ messages in thread From: Stephen Warren @ 2018-09-18 17:23 UTC (permalink / raw) To: u-boot 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH 1/1] test/py: catch errors occurring when reading the console 2018-09-18 17:23 ` Stephen Warren @ 2018-09-19 0:43 ` Heinrich Schuchardt 2018-09-19 15:24 ` Stephen Warren 2018-09-19 15:29 ` Simon Glass 0 siblings, 2 replies; 10+ messages in thread From: Heinrich Schuchardt @ 2018-09-19 0:43 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH 1/1] test/py: catch errors occurring when reading the console 2018-09-19 0:43 ` Heinrich Schuchardt @ 2018-09-19 15:24 ` Stephen Warren 2018-09-19 16:45 ` Heinrich Schuchardt 2018-09-19 15:29 ` Simon Glass 1 sibling, 1 reply; 10+ messages in thread From: Stephen Warren @ 2018-09-19 15:24 UTC (permalink / raw) To: u-boot 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH 1/1] test/py: catch errors occurring when reading the console 2018-09-19 15:24 ` Stephen Warren @ 2018-09-19 16:45 ` Heinrich Schuchardt 2018-09-20 19:06 ` Stephen Warren 0 siblings, 1 reply; 10+ messages in thread From: Heinrich Schuchardt @ 2018-09-19 16:45 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH 1/1] test/py: catch errors occurring when reading the console 2018-09-19 16:45 ` Heinrich Schuchardt @ 2018-09-20 19:06 ` Stephen Warren 0 siblings, 0 replies; 10+ messages in thread From: Stephen Warren @ 2018-09-20 19:06 UTC (permalink / raw) To: u-boot 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH 1/1] test/py: catch errors occurring when reading the console 2018-09-19 0:43 ` Heinrich Schuchardt 2018-09-19 15:24 ` Stephen Warren @ 2018-09-19 15:29 ` Simon Glass 1 sibling, 0 replies; 10+ messages in thread From: Simon Glass @ 2018-09-19 15:29 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH 1/1] test/py: catch errors occurring when reading the console 2018-09-18 17:21 [U-Boot] [PATCH 1/1] test/py: catch errors occurring when reading the console Heinrich Schuchardt 2018-09-18 17:23 ` Stephen Warren @ 2018-09-18 22:05 ` Alexander Graf 1 sibling, 0 replies; 10+ messages in thread From: Alexander Graf @ 2018-09-18 22:05 UTC (permalink / raw) To: u-boot 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() > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH 1/1] test/py: catch errors occurring when reading the console @ 2018-09-20 18:19 Heinrich Schuchardt 2018-09-20 19:07 ` Stephen Warren 0 siblings, 1 reply; 10+ messages in thread From: Heinrich Schuchardt @ 2018-09-20 18:19 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [U-Boot] [PATCH 1/1] test/py: catch errors occurring when reading the console 2018-09-20 18:19 Heinrich Schuchardt @ 2018-09-20 19:07 ` Stephen Warren 0 siblings, 0 replies; 10+ messages in thread From: Stephen Warren @ 2018-09-20 19:07 UTC (permalink / raw) To: u-boot 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-09-20 19:07 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-18 17:21 [U-Boot] [PATCH 1/1] test/py: catch errors occurring when reading the console Heinrich Schuchardt 2018-09-18 17:23 ` Stephen Warren 2018-09-19 0:43 ` Heinrich Schuchardt 2018-09-19 15:24 ` Stephen Warren 2018-09-19 16:45 ` Heinrich Schuchardt 2018-09-20 19:06 ` Stephen Warren 2018-09-19 15:29 ` Simon Glass 2018-09-18 22:05 ` Alexander Graf 2018-09-20 18:19 Heinrich Schuchardt 2018-09-20 19:07 ` Stephen Warren
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.