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

[Jonathan Roelofs via llvm-dev]

On 11/15/15 9:19 AM, Dan Liew wrote:
> Hi,
>
>  > 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.
>
> Thanks for that. I'll take a look.
>  >
>  >>
>  >> 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.
>
> Okay. I'll bear that in mind. I currently want to distinguish between a
> TIMEOUT and UNRESOLVED (I'm not sure what other conditions can lead to
> this result code) so I will keep it for now.

The reason not to is that there are lots of out-of-tree scripts that 
parse the output of LIT (which was designed to match DejaGnu's output). 
It's best not to force everyone update their scripts. Therefore tests 
that run past their allotted time should be marked as FAIL.

>
>  >
>  >> * 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.
>
> I agree a shorter name would be nicer. I'm worried about it being
> confused with --max-time though.

The --help description should be good enough to distinguish them.  It'll 
be important however to keep their naming consistent across the 
variables that are used to implement this.

>
>  >
>  >> * Support for running external and internal ShTests with a per test
> timeout
>  >> * Support for running GTests with a per test timeout
>  >>
>
>  >
>  > 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).
>
> I did do brief testing with Python 2.7. It seemed to work okay.
>
>  >
>  >>
>  >> * 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.
>
> Okay. I'll add this before submitting to phabricator.
>
>  >
>  > Mind squashing your two patches, throwing them on Phabricator, and
> cc-ing llvm-commits?
>
> I'm travelling so I can't do it right now but will do tomorrow.
>
> Thanks,
> Dan.
>

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