[llvm-dev] [lit] RFC: Per test timeout

[Jonathan Roelofs via llvm-dev]

On 11/14/15 3:10 AM, Dan Liew via llvm-dev wrote:
> Hi,
>
> A feature I've wanted in lit for a while is a having a timeout per
> test. Attached
> are patches that implement this idea.

Cool, I hope this succeeds. I tried implementing per-test timeouts 
before, and couldn't get it to work in all cases. The review eventually 
fizzled out, and I abandoned it.

Here's that old review: http://reviews.llvm.org/D6584 Perhaps you can 
cannibalize testcases from it.

>
> I'm e-mailing llvm-dev rather than llvm-commits
> because I want to gather more feedback on my initial implementation and
> hopefully some answers to some unresolved issues with my implementation.
>
> Currently in lit you can set a global timeout for
> all of the tests but not for each individual test.
>
> The attached patches add
>
> * Support for a new ResultCode called TIMEOUT

When I implemented mine, one of the first revisions had this, but then 
@ddunbar said he'd prefer I didn't add a new ResultCode.

> * A new command line option --max-individual-test-time

I think you should call it `--timeout=`, and then say in the description 
that it's a per-test timeout.

> * Support for running external and internal ShTests with a per test timeout
> * Support for running GTests with a per test timeout
>
> I wanted to get some initial feedback on the implementation.
>
> * If a timeout is requested the Python psutil module is required.
>    This module does not ship with Python but is availble via pip
>    (or on Linux your distribution's package manager). How do people feel
>    about this? I don't like adding extra dependencies but this module
>    makes it really easy to kill a process and all its children recursively
>    in a platform neutral way. Note that if a per test timeout is not requested
>    then the psutil module is not imported and lit acts just like it did
> before my patches.

This must be the missing piece... I couldn't get my implementation to 
work without resorting to Python 3.x features (which is incompatible 
with a 2.x minimum version).

>
> * If the platform running lit doesn't have psutil installed and a
> timeout is requested
>    an exception will be thrown. Should we provide a more friendly error message?

Yes.

>    If so where should this go in lit's code?

Not sure off the top of my head, but probably somewhere near the rest of 
the argument parsing stuff. I'll have a look later.

>
> * I've not tested these patches on OSX or Windows. Is anyone on those
> platforms willing
>    to give them a try?

Yes, I'll give it a whirl on Darwin early next week.


Mind squashing your two patches, throwing them on Phabricator, and 
cc-ing llvm-commits?


Thanks!

Jon

>
> * The behaviour of --max-individual-test-time is a little at odds with
> the behaviour of --max-time. --max-time will mark unexecuted tests as
> UNRESOLVED whereas --max-individual-test-time will mark a test that
> ran out of time as TIMEOUT. Is this okay?
>
> * @Chris Matthews. Does the code that emits xunit xml files need to
> change in anyway?
>
> * @Daniel Dunbar. If these changes (in some form) end up being
> committed would you be happy
>    to push a new release of lit to PyPy?
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>

-- 
Jon Roelofs
jonathan at codesourcery.com
CodeSourcery / Mentor Embedded