On Thu, 9 Apr 2026 10:22:42 GMT, Mahendra Chhipa <[email protected]> wrote:
> Following tests are refactored to use JUnit Jupiter API :
> sun/net/ext/ExtendedSocketOptionsTest.java
> sun/net/www/protocol/file/DirPermissionDenied.java
> sun/net/www/protocol/jar/MultiReleaseJarURLConnection.java
> sun/net/www/protocol/jrt/Basic.java
> sun/net/www/protocol/http/TestTransparentNTLM.java
> sun/net/www/protocol/http/HttpHeaderParserTest.java
> sun/net/www/http/RequestMethodCheck/RequestMethodEquality.java
> sun/net/www/http/KeepAliveStreamCleaner/java.base/sun/net/www/http/KeepAliveStreamCleanerTest.java
> sun/net/www/http/KeepAliveStreamCleaner/KeepAliveStreamCleanerTestDriver.java
> sun/net/www/MessageHeaderTest.java
> sun/net/spi/DefaultProxySelectorTest.java
Hopefully these comments are useful, if not please let me know. They are only
suggestions; feel free to ignore them, I am not an OpenJDK member.
test/jdk/sun/net/www/MessageHeaderTest.java line 62:
> 60: @Test
> 61: public void ntlmNegotiateTest () throws Exception {
> 62: String expected[] = {
Not directly related to JUnit changes, but use canonical way to write array
type here?
- String expected[]
+ String[] expected
test/jdk/sun/net/www/protocol/file/DirPermissionDenied.java line 80:
> 78: } catch (Exception e) {
> 79: throw new RuntimeException("Failed " + e);
> 80: }
Should these use `assertThrows(IOException.class, ...)` (in case they do
actually expect an exception)?
test/jdk/sun/net/www/protocol/http/HttpHeaderParserTest.java line 480:
> 478: msg, values.size(),
> otherValues.size()));
> 479: if (!(values.containsAll(otherValues) &&
> otherValues.containsAll(values)))
> 480: assertTrue(false, format("Lists are unequal [%s]
> [%s]", values, otherValues));
Use JUnit's `fail(...)` instead of `assertTrue(false, ...)`?
test/jdk/sun/net/www/protocol/jrt/Basic.java line 92:
> 90: try {
> 91: int b = uc.getInputStream().read();
> 92: assertTrue(b != -1);
Use `assertNotEquals`?
test/jdk/sun/net/www/protocol/jrt/Basic.java line 113:
> 111: try {
> 112: Object obj = url.getContent();
> 113: assertTrue(obj != null);
Use `assertNotNull`?
-------------
PR Review: https://git.openjdk.org/jdk/pull/30645#pullrequestreview-4089646132
PR Review Comment: https://git.openjdk.org/jdk/pull/30645#discussion_r3064387115
PR Review Comment: https://git.openjdk.org/jdk/pull/30645#discussion_r3064288491
PR Review Comment: https://git.openjdk.org/jdk/pull/30645#discussion_r3064317536
PR Review Comment: https://git.openjdk.org/jdk/pull/30645#discussion_r3064374273
PR Review Comment: https://git.openjdk.org/jdk/pull/30645#discussion_r3064372688
