[Lldb-commits] [PATCH] D13124: test runner: switch to pure-Python timeout mechanism

[Todd Fiala via lldb-commits]

tfiala added a comment.

In http://reviews.llvm.org/D13124#254958, @zturner wrote:

> This is what I get.
>
> d:\src\llvm\tools\lldb\test>cd test_runner
>
> d:\src\llvm\tools\lldb\test\test_runner>cd test
>
> d:\src\llvm\tools\lldb\test\test_runner\test>c:\Python27_LLDB\x86\python_d.exe
>  process_control_tests.py
>  ..E.EE
>  ======================================================================
>  ERROR: test_hard_timeout_works (__main__.ProcessControlTimeoutTests)
>  Driver falls back to hard terminate when soft terminate is ignored.
>
>  ----------------------------------------------------------------------
>
> Traceback (most recent call last):
>
>   File "process_control_tests.py", line 177, in test_hard_timeout_works
>     options="--sleep 120"),
>   File "process_control_tests.py", line 83, in inferior_command
>     cls._suppress_soft_terminate(command)
>   File "process_control_tests.py", line 70, in _suppress_soft_terminate
>     for signum in helper.soft_terminate_signals():
>
> TypeError: 'NoneType' object is not iterable


This is going to be what appears to be a missing "is not None" check on the helper.soft_terminate_signals().  I'm expecting we'll need to figure out how Windows will want to test the "child process ignores the pleasant/nice kill attempt -- aka a soft terminate".  I doubt the right thing to do is to tell the inferior to ignore signal numbers on Windows.  In this case, here:

File "process_control_tests.py", line 83, in inferior_command

  cls._suppress_soft_terminate(command)

we'll need to figure out what Windows needs to do to tell the inferior.py script how to ignore soft terminate signals.

One option I almost did but wanted to figure out if you need it is to switch the inferior.py parameter from the sense of "ignore these signals" to "do whatever platform thing is necessary to avoid a soft terminate".  For the Posix-y systems, that would be ignoring SIGTERM and SIGQUIT.  For Windows, it would be whatever you need.  Then the test side can just add that (new) flag that says something like "--ignore-soft-terminate".  That is probably the way to go here.  You still need to figure out what needs to go in the handler for that in inferior.py, but that would be the general idea.

> ======================================================================

>  ERROR: test_soft_timeout_works_core (__main__.ProcessControlTimeoutTests)

>  Driver uses soft terminate (with core request) when process times out.

> 

>  ----------------------------------------------------------------------

> 

> Traceback (most recent call last):

> 

>   File "process_control_tests.py", line 157, in test_soft_timeout_works_core

>     self._soft_timeout_works(True)

>   File "process_control_tests.py", line 150, in _soft_timeout_works

>     helper.was_soft_terminate(driver.returncode, with_core),

>   File "..\lib\process_control.py", line 192, in was_soft_terminate

>     raise Exception("platform needs to implement")

> 

> Exception: platform needs to implement

> 

> ======================================================================

>  ERROR: test_soft_timeout_works_no_core (__main__.ProcessControlTimeoutTests)

>  Driver uses soft terminate (no core request) when process times out.

> 

>  ----------------------------------------------------------------------

> 

> Traceback (most recent call last):

> 

>   File "process_control_tests.py", line 162, in

> 

> test_soft_timeout_works_no_core

> 

>     self._soft_timeout_works(False)

>   File "process_control_tests.py", line 150, in _soft_timeout_works

>     helper.was_soft_terminate(driver.returncode, with_core),

>   File "..\lib\process_control.py", line 192, in was_soft_terminate

>     raise Exception("platform needs to implement")

> 

> Exception: platform needs to implement


This is one of the spots I need you to fill in.

> 

> 

>  ----------------------------------------------------------------------

> 

> Ran 6 tests in 10.351s

> 

> FAILED (errors=3)

>  [37558 refs]

> 

> Looking into these now.



http://reviews.llvm.org/D13124