<p dir="ltr">Hi,<br></p>
<p dir="ltr">> 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.<br>
><br>
> Here's that old review: <a href="http://reviews.llvm.org/D6584">http://reviews.llvm.org/D6584</a> Perhaps you can cannibalize testcases from it.<br></p>
<p dir="ltr">Thanks for that. I'll take a look.<br>
><br>
>><br>
>> I'm e-mailing llvm-dev rather than llvm-commits<br>
>> because I want to gather more feedback on my initial implementation and<br>
>> hopefully some answers to some unresolved issues with my implementation.<br>
>><br>
>> Currently in lit you can set a global timeout for<br>
>> all of the tests but not for each individual test.<br>
>><br>
>> The attached patches add<br>
>><br>
>> * Support for a new ResultCode called TIMEOUT<br>
><br>
><br>
> 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.</p>
<p dir="ltr">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.</p>
<p dir="ltr">><br>
>> * A new command line option --max-individual-test-time<br>
><br>
><br>
> I think you should call it `--timeout=`, and then say in the description that it's a per-test timeout.</p>
<p dir="ltr">I agree a shorter name would be nicer. I'm worried about it being confused with --max-time though.</p>
<p dir="ltr">><br>
>> * Support for running external and internal ShTests with a per test timeout<br>
>> * Support for running GTests with a per test timeout<br>
>></p>
<p dir="ltr">><br>
> 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).</p>
<p dir="ltr">I did do brief testing with Python 2.7. It seemed to work okay.</p>
<p dir="ltr">><br>
>><br>
>> * If the platform running lit doesn't have psutil installed and a<br>
>> timeout is requested<br>
>>    an exception will be thrown. Should we provide a more friendly error message?<br>
><br>
><br>
> Yes.</p>
<p dir="ltr">Okay. I'll add this before submitting to phabricator.<br></p>
<p dir="ltr">><br>
> Mind squashing your two patches, throwing them on Phabricator, and cc-ing llvm-commits?<br></p>
<p dir="ltr">I'm travelling so I can't do it right now but will do tomorrow.</p>
<p dir="ltr">Thanks,<br>
Dan.</p>