On Wed, 1 May 2024 20:27:19 GMT, Lance Andersen <lan...@openjdk.org> wrote:

>> test/jdk/java/util/Base64/TestEncodingDecodingLength.java line 75:
>> 
>>> 73:         } catch (InvocationTargetException ex) {
>>> 74:             Throwable rootEx = ex.getCause();
>>> 75:             assertEquals(OutOfMemoryError.class, rootEx.getClass(), 
>>> "00ME should be thrown");
>> 
>> Sorry if it is intentional, but I wonder if you meant "OOME" instead of 
>> "00ME" here?
>
> Good catch, you sometimes see what you want to see

That was unexpected, wonder how I did that unconsciously...

>> test/jdk/java/util/Base64/TestEncodingDecodingLength.java line 100:
>> 
>>> 98:             m.invoke(DECODER, src, -LARGE_MEM_SIZE + 1, 1);
>>> 99:         } catch (InvocationTargetException ex) {
>>> 100:             fail("Decode should neither throw NASE or OOME: " + 
>>> ex.getCause());
>> 
>> Should we check the cause is either NASE or OOME here?
>
> The original test just validated there was no exception thrown, so I think we 
> can just update the message to indicate an unexpected Exception.
> 
> I had added this as a comment but must have missed hitting the comment button

Improved the error message to be more general, (as no exception should be 
thrown). Comments clarify that the method used to throw NASE and OOME.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19036#discussion_r1586793517
PR Review Comment: https://git.openjdk.org/jdk/pull/19036#discussion_r1586793148

Reply via email to