[test] RFR 8209771: jdk.test.lib.Utils::runAndCheckException error
Weijun Wang
weijun.wang at oracle.com
Tue Aug 21 06:33:22 UTC 2018
Hi David
> On Aug 21, 2018, at 1:02 PM, David Holmes <david.holmes at oracle.com> wrote:
>
> Hi Max,
>
> On 21/08/2018 12:53 PM, Weijun Wang wrote:
>> Please take a review at
>> https://bugs.openjdk.java.net/browse/JDK-8209771
>> http://cr.openjdk.java.net/~weijun/8209771/webrev.00/
>> There is an error in the implementation of the method, and I also think ThrowingRunnable is more developer friendly here.
>
> ThrowingRunnable is really only needed for a Runnable that might throw checked exceptions. It really doesn't make any difference here.
By using ThrowingRunnable I can put a one-line lambda expression that might throw a checked exception, otherwise I'll have to wrap it into a RuntimeException. That's what I meant developer-friendly.
>
> The actual logic fix seems okay. You could initialize throwable to null and avoid the assignment in the try block.
Sure.
>
>> The methods are only used in hotspot so I'm asking for the code review here.
>
> Mostly by the JVMCI compiler folk too.
In fact, mostly by tests in hotspot/jtreg/compiler/jvmci. I thought it's a part of hotspot. Anyway, I've added hotspot-compiler-dev at .
> I didn't even know these utilities existed. I've written tons of test code that checks for expected and unexpected exceptions. :)
Me too. This morning I had a little free time to wander around jdk.test.lib and notice these.
>
>> I also think in filterException(), filter.apply(null) should be called if test.run() has not thrown any exception, but this is a behavior change and I cannot be sure.
>
> That would be a different function I think. filterException seems to me just to be a generalized catch clause to detect if a specific kind of exception occurred - not to check that some specific exception should have occurred.
Maybe. I won't touch it.
Thanks
Max
>
> Thanks,
> David
>
>> Please include my name in the recipient list as I'm not in the mail list.
>> This will be a noreg-self.
>> Thanks
>> Max
>>
More information about the hotspot-compiler-dev
mailing list