On Fri, 9 Jan 2026 00:13:12 GMT, Alexander Matveev <[email protected]> wrote:

>> Sorry, my comment was incorrect: SAXException is thrown when the directory 
>> is passed into `DocumentBuilder#parse(java.io.File)`:
>> 
>> org.xml.sax.SAXParseException; systemId: 
>> file:/C:/AppData/Local/Temp/junit10902338938727647559/.jpackage.xml/; 
>> lineNumber: 1; columnNumber: 1; Premature end of file.
>> 
>> 
>> Whet it is passed to `Files#readAllBytes(Path)` it throws IOException:
>> 
>> java.nio.file.AccessDeniedException: 
>> C:\AppData\Local\Temp\junit17123875824397995430.jpackage.xml
>> 
>> 
>> This change causes failure of the AppImageFileTest#testDirectory test case at
>> https://github.com/openjdk/jdk/blob/385c4f8180d30c0e41b848eb4b2c1c8788211422/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/AppImageFileTest.java#L118:
>> 
>> org.opentest4j.AssertionFailedError: expected: <Failed to read 
>> ".jpackage.xml" file in the predefined app image 
>> "C:\AppData\Local\Temp\junit10902338938727647559"> but was: <".jpackage.xml" 
>> file in the predefined app image 
>> "C:\AppData\Local\Temp\junit10902338938727647559" contains malformed XML 
>> data>
>>      at 
>> org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
>>      at 
>> org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
>>      at 
>> org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
>>      at 
>> org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
>>      at 
>> org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
>>      at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1141)
>>      at 
>> jdk.jpackage/jdk.jpackage.internal.AppImageFileTest.testDirectory(AppImageFileTest.java:122)
>>      at [email protected]/java.lang.reflect.Method.invoke(Method.java:580)
>>      at [email protected]/java.util.ArrayList.forEach(ArrayList.java:1597)
>>      at [email protected]/java.util.ArrayList.forEach(ArrayList.java:1597)
>> 
>> 
>> AppImageFileTest#testNoSuchFile test case at
>> https://github.com/openjdk/jdk/blob/385c4f8180d30c0e41b848eb4b2c1c8788211422/test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/AppImageFileTest.java#L111
>>  also fails because of an unexpected exception type.
>> 
>> In case of a non-existant file, `DocumentBuilder#parse(java.io.File)` throws 
>> `java.io.FileNotFoundException`, and `Files#readAllBytes(Path)` throws 
>> `java.nio.file.NoSuchFileException`. This change fails 
>> AppImageFileTest#testNoSuchFile test case:
>> 
>> org.opentest4j.AssertionFailedError: expected: <".jpackage.xml" file is 
>> missing in the predefine...
>
> You need to document it or adjust unit test. It does not make sense to use 
> one approach here and different approach in `AppImageFile` below.

The test code bails out on the first unexpected exception. It doesn't transform 
internal exceptions into localized user-friendly messages, etc. Implementation 
does. Using the same API in these particular cases solves no problem. I can 
just use `Files#readAllBytes(Path)` in the test code to satisfy the request for 
the same approach. It will make no difference, but make the code bulkier than 
it needs to be.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29007#discussion_r2674409358

Reply via email to