On Thu, 12 Mar 2026 12:15:00 GMT, Daniel Fuchs <[email protected]> wrote:
>> Looking at assertCompletion(), which is nearly 60 lines of code, I'd find it
>> impossible to reason about what's actually being tested here. All these test
>> cases expect failure, and all by `NamingException`, which should be being
>> thrown by the non-test code (which here seems to be the closure you're
>> passing in).
>>
>> Can you just do:
>>
>> NamingException e = assertThrows(NamingException.class, () -> new
>> InitialDirContext(env));
>>
>> and get the same result? And if not, why not (i.e. what's tangled up in the
>> test code that makes this not work)?
>
> This is checking timeouts. We want to check that NamingException is thrown,
> and that it's thrown within the expected time frame - not too early and not
> too late.
> We obviously don't want to repeat this logic in every test.
Does inlining the "complex _assert_ method" help? Like so:
Index: test/jdk/com/sun/jndi/ldap/LdapTimeoutTest.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/test/jdk/com/sun/jndi/ldap/LdapTimeoutTest.java
b/test/jdk/com/sun/jndi/ldap/LdapTimeoutTest.java
--- a/test/jdk/com/sun/jndi/ldap/LdapTimeoutTest.java (revision Staged)
+++ b/test/jdk/com/sun/jndi/ldap/LdapTimeoutTest.java (date 1773329518733)
@@ -35,7 +35,6 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.function.Executable;
import javax.naming.Context;
import javax.naming.NamingException;
@@ -186,11 +185,10 @@
env.put(Context.PROVIDER_URL, urlTo(server));
server.start();
server.starting().join();
- Executable completion =
+ NamingException e = assertThrows(NamingException.class,
() -> assertCompletion(CONNECT_MILLIS,
- 2 * CONNECT_MILLIS + TOLERANCE,
- () -> new InitialDirContext(env));
- NamingException e = assertThrows(NamingException.class,
completion);
+ 2 * CONNECT_MILLIS + TOLERANCE,
+ () -> new InitialDirContext(env)));
String msg = e.getMessage();
assertTrue(msg != null && msg.contains("timeout")
&& msg.contains(String.valueOf(CONNECT_MILLIS)),
@@ -210,11 +208,10 @@
InitialDirContext ctx = new InitialDirContext(env);
SearchControls scl = new SearchControls();
scl.setSearchScope(SearchControls.SUBTREE_SCOPE);
- Executable completion =
+ NamingException e = assertThrows(NamingException.class,
() -> assertCompletion(READ_MILLIS,
- READ_MILLIS + TOLERANCE,
- () ->
ctx.search("ou=People,o=JNDITutorial", "(objectClass=*)", scl));
- NamingException e = assertThrows(NamingException.class,
completion);
+ READ_MILLIS + TOLERANCE,
+ () -> ctx.search("ou=People,o=JNDITutorial",
"(objectClass=*)", scl)));
String msg = e.getMessage();
assertTrue(msg != null && msg.contains("timeout")
&& msg.contains(String.valueOf(READ_MILLIS)),
@@ -231,11 +228,10 @@
env.put(Context.PROVIDER_URL, urlTo(server));
server.start();
server.starting().join();
- Executable completion =
+ NamingException e = assertThrows(NamingException.class,
() -> assertCompletion(CONNECT_MILLIS,
- 2 * CONNECT_MILLIS + TOLERANCE,
- () -> new InitialDirContext(env));
- NamingException e = assertThrows(NamingException.class,
completion);
+ 2 * CONNECT_MILLIS + TOLERANCE,
+ () -> new InitialDirContext(env)));
String msg = e.getMessage();
assertTrue(msg != null && msg.contains("timeout")
&& msg.contains(String.valueOf(CONNECT_MILLIS)),
@@ -257,11 +253,10 @@
env.put(Context.PROVIDER_URL, urlTo(server));
server.start();
server.starting().join();
- Executable completion =
+ NamingException e = assertThrows(NamingException.class,
() -> assertCompletion(CONNECT_MILLIS,
- 2 * CONNECT_MILLIS + TOLERANCE,
- () -> new InitialDirContext(env));
- NamingException e = assertThrows(NamingException.class,
completion);
+ 2 * CONNECT_MILLIS + TOLERANCE,
+ () -> new InitialDirContext(env)));
String msg = e.getMessage();
assertTrue(msg != null && msg.contains("timeout")
&& msg.contains(String.valueOf(CONNECT_MILLIS)),
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30203#discussion_r2925537062
