[Lldb-commits] [PATCH] D13362: Enhance test runner timeout logic to detect successful completion of spawned process when stdout/stderr are shared to still-existing child processes

[Todd Fiala via lldb-commits]

tfiala added a comment.

In http://reviews.llvm.org/D13362#259543, @labath wrote:

> In http://reviews.llvm.org/D13362#258972, @tfiala wrote:
>
> > I've fixed:
> >  https://llvm.org/bugs/show_bug.cgi?id=25019
> >
> > I think for now I am not interested in trying to tackle the intent of this change as it unduly complicates the timeout detection logic.
> >
> > I am okay with saying:
> >  "If you run a process http://reviews.llvm.org/P1, and that process creates child processes C1..CN, and shares the stdout/stderr file handles from http://reviews.llvm.org/P1 to C1..CN, and if http://reviews.llvm.org/P1 exits, we don't detect the exit until all stdout/stderr handles shared with C1..CN are closed."  That's just a bad test if it is leaving children around.  It will time out.
>
>
> Sounds good to me. This motivates people to write correct tests, which I think is good. My main concern was not leaving those children around after we time out, which I believe you fixed already.


Yep, great thanks!

I just closed the bugzilla ticket for this feature as WONTFIX.  If we really want this later, we can revisit.


http://reviews.llvm.org/D13362