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
test/jdk/sun/net/ext/ExtendedSocketOptionsTest.java line 50:
> 48: * @run junit/othervm ExtendedSocketOptionsTest
> 49: * @run junit/othervm ExtendedSocketOptionsTest
> 50: * @run junit/othervm ExtendedSocketOptionsTest
Since we're modifying `@run` lines we could take the opportunity to replace the
class name with `${test.main.class}`, when it matches the file name.
test/jdk/sun/net/ext/ExtendedSocketOptionsTest.java line 95:
> 93: assertEquals(2,
> GetInstanceTask.extendedSocketOptionsInstances.size());
> 94: for (final Object inst :
> GetInstanceTask.extendedSocketOptionsInstances) {
> 95: assertSame(inst, extSocketOptions,
> "sun.net.ext.ExtendedSocketOptions#getInstance()" +
shouldn't `inst` be the second argument here, not the first?
test/jdk/sun/net/spi/DefaultProxySelectorTest.java line 50:
> 48: try {
> 49: selector.select(null);
> 50: fail("select() was expected to fail for null URI");
we could simplify and use `assertThrows` here, and in other places/files as well
test/jdk/sun/net/spi/DefaultProxySelectorTest.java line 86:
> 84: try {
> 85: selector.select(uri);
> 86: fail("select() was expected to fail for URI " + uri);
Same here. Use assertThrows, even consider removind this method.
test/jdk/sun/net/www/http/RequestMethodCheck/RequestMethodEquality.java line
115:
> 113: // If both connectTimeout values are equal, it means the
> test retrieved the same broken
> 114: // HttpClient from the cache and is trying to re-use it.
> 115: assertNotEquals(originalConnectTimeout,
> cachedConnectTimeout, "Both connectTimeout values are equal.\nThis means the
> test is reusing a broken HttpClient rather than creating a new one.");
arguments should be swapped. Could you make a pass over all the files and
double check this please?
The argument order for JUnit is `(expected, actual [, msg])`
test/jdk/sun/net/www/protocol/http/HttpHeaderParserTest.java line 58:
> 56:
> 57: public class HttpHeaderParserTest {
> 58: public static Object[][] responses() {
Consider simplifying the method to use `List.of` and simply return
`List<String>`
test/jdk/sun/net/www/protocol/http/HttpHeaderParserTest.java line 370:
> 368: }
> 369:
> 370: public static Object[][] errors() {
Consider simplifying the method to simply use `List.of(...)` and return
`List<String>`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30645#discussion_r3058505058
PR Review Comment: https://git.openjdk.org/jdk/pull/30645#discussion_r3058591572
PR Review Comment: https://git.openjdk.org/jdk/pull/30645#discussion_r3058515050
PR Review Comment: https://git.openjdk.org/jdk/pull/30645#discussion_r3058601238
PR Review Comment: https://git.openjdk.org/jdk/pull/30645#discussion_r3058615496
PR Review Comment: https://git.openjdk.org/jdk/pull/30645#discussion_r3058562719
PR Review Comment: https://git.openjdk.org/jdk/pull/30645#discussion_r3058553984
