On Wed, 30 Sep 2020 17:26:18 GMT, Brent Christian <bchri...@openjdk.org> wrote:

>> test/jdk/java/util/jar/JarFile/LargeManifestOOMTest.java line 60:
>> 
>>> 58:         final OutOfMemoryError oome = 
>>> Assert.expectThrows(OutOfMemoryError.class, () -> jar.getManifest());
>>> 59:         // additionally verify that the OOM was for the right/expected 
>>> reason
>>> 60:         if (!"Required array size too large".equals(oome.getMessage())) 
>>> {
>> 
>> I wasn't too sure if I should add this additional check on the message of 
>> the `OutOfMemoryError`, but decided to do it
>> anyway, given that from what I remember there were some discussions in the 
>> `core-libs-dev` list a while back on the
>> exact messages that such OOMs should throw. So I guessed that it might be OK 
>> to rely on those messages in the tests
>> within this project. However, I am open to removing specific check if it's 
>> considered unnecessary.
>
> I think it's fine either way.

If you are going to validate the message, which I probably would not,  it would 
be important to make sure it document
if the message is changed in JarFile::getBytes, that the test needs updated.  
Otherwise it will be easy to miss.

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

PR: https://git.openjdk.java.net/jdk/pull/323

Reply via email to