On Tue, 21 Apr 2026 15:29:25 GMT, Jaikiran Pai <[email protected]> wrote:

>> Can I please get  a review of this test-only change which updates the 
>> `test/jdk/tools/jar/JarExtractTest.java` test to use the jtreg scratch 
>> directory for its temporary files?
>> 
>> In addition to that change, the `testExtractNoDestDirWithPFlag` is a bit 
>> different and that one intentionally expects to write out to a directory 
>> that starts with a leading slash in its path. We use `/tmp` directory in 
>> that test intentionally. In this PR, that test method now cleans up the 
>> `/tmp/` directory after the test is done.
>> 
>> I've looked through the discussions in the PR for JDK-8173970, where this 
>> test was introduced and I've verified that this change doesn't impact any 
>> original expectations of this test. I've also verified that the test 
>> continues to pass with this change on all platforms in our CI.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Jaikiran Pai has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - remove toAbsolutePath() usage in test
>  - add code comments in test to explain why we use /tmp/ directory for 
> testExtractNoDestDirWithPFlag

test/jdk/tools/jar/JarExtractTest.java line 302:

> 300:         } catch (IOException ioe) {
> 301:             Assumptions.abort("skipping test, since /tmp cannot be 
> written to: " + ioe);
> 302:             return;

This return sentence seems unreachable,

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

PR Review Comment: https://git.openjdk.org/jdk/pull/30847#discussion_r3122440208

Reply via email to