RFR: 8378595: Refactor miscellaneous tests under test/jdk/java/net/httpclient from TestNG to JUnit [v2]

Daniel Fuchs dfuchs at openjdk.org
Wed Feb 25 12:03:22 UTC 2026


On Wed, 25 Feb 2026 06:50:46 GMT, SendaoYan <syan at openjdk.org> wrote:

>> Daniel Fuchs has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision:
>> 
>>  - Review feedback
>>  - Merge branch 'master' into junit-httpclient-misc-8378595
>>  - 8378595: Refactor miscellaneous tests under test/jdk/java/net/httpclient from TestNG to JUnit
>
> test/jdk/java/net/httpclient/PathSubscriber/BodyHandlerOfFileDownloadTest.java line 116:
> 
>> 114:                 {  http3URI,   defaultFsPath,  MSG,  true   },
>> 115:                 {  http3URI,   defaultFsPath,  MSG,  false  },
>> 116: 
> 
> Could we remove this empty line

Done.

> test/jdk/java/net/httpclient/security/filePerms/FileProcessorPermissionTest.java line 38:
> 
>> 36: 
>> 37: import org.junit.jupiter.api.Test;
>> 38: import static org.junit.jupiter.api.Assertions.*;
> 
> This import "import static org.junit.jupiter.api.Assertions.*;" seems useless.

Done

> test/jdk/java/net/httpclient/security/filePerms/SecurityBeforeFile.java line 44:
> 
>> 42: import org.junit.jupiter.params.ParameterizedTest;
>> 43: import org.junit.jupiter.params.provider.MethodSource;
>> 44: import static org.junit.jupiter.api.Assertions.*;
> 
> In this file, seems that we can import "org.junit.jupiter.api.Assertions.fail" explicitly rather than import all the assertion method.

Done.

> test/jdk/java/net/httpclient/security/filePerms/SecurityBeforeFile.java line 79:
> 
>> 77:         try {
>> 78:             BodyHandlers.ofFileDownload(p, openOptions);
>> 79:             fail("UNEXPECTED, file " + p.toString() + " exists?");
> 
> Seems that use assertThrows better than try-fail-catch
> 
> 
>         assertThrows(FileNotFoundException.class, () -> {
>             BodyPublishers.ofFile(p);
>         });

I'd rather keep things the way they are as they provide additional logging and diagnosis compare to plain usage of `assertThrows`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29900#discussion_r2852610082
PR Review Comment: https://git.openjdk.org/jdk/pull/29900#discussion_r2852609527
PR Review Comment: https://git.openjdk.org/jdk/pull/29900#discussion_r2852608937
PR Review Comment: https://git.openjdk.org/jdk/pull/29900#discussion_r2852608365


More information about the net-dev mailing list