RFR(s): 8169721: [TESTBUG] com/sun/jndi tests have undeclared dependency on java.naming module

[Sergei Kovalev]

Hi Daniel,

Thank you for feedback.

You absolutely right about modules. To have an ability to distinguish 
environment issue from test pass I've added a warning message how you 
recommended. At least it will have a visible effect.

http://cr.openjdk.java.net/~skovalev/8169721/webrev.01/

Looks like I need another approval for one more reviewer. Isn't it?

-- 
With best regards,
Sergei


17.11.16 18:19, Daniel Fuchs wrote:
> Hi Sergey,
>
> I added a System.out.println in UnbindIdempotent (see diff below)
> to double check which type of context object was expected:
>
> Got context: class com.sun.jndi.rmi.registry.RegistryContext
>
> This lets me think that this test does not depend on java.naming
> but rather on jdk.naming.rmi - and that if you simply link with
> java.naming then the test might simply pass without doing nothing
> because it gets a NamingException at line 54.
>
> Could you please double check this (verify what the test actually
> does with --limit-mods and your current change)?
> You might also want to keep the traces I added (below) to improve
> diagnosis in case of test failures.
>
> The other changes look good - I think - though I am not
> the usual maintainer of this area.
>
> best regards,
>
> -- daniel
>
> diff --git 
> a/test/com/sun/jndi/rmi/registry/RegistryContext/UnbindIdempotent.java 
> b/test/com/sun/jndi/rmi/registry/RegistryContext/UnbindIdempotent.java
> --- 
> a/test/com/sun/jndi/rmi/registry/RegistryContext/UnbindIdempotent.java
> +++ 
> b/test/com/sun/jndi/rmi/registry/RegistryContext/UnbindIdempotent.java
> @@ -49,8 +49,10 @@
>
>          try {
>              rctx = (Context)ictx.lookup("rmi://localhost:" + 
> Integer.toString(registryPort));
> +            System.out.println("Got context: " + rctx.getClass());
>          } catch (NamingException e) {
>              // Unable to set up for test.
> +        System.out.println("Warning: Can't run test: " + e);
>              return;
>          }
>
>
> On 15/11/16 14:34, Sergei Kovalev wrote:
>> Hi Team,
>>
>> Please review a small fix for tests.
>>
>> BugID: https://bugs.openjdk.java.net/browse/JDK-8169721
>> Web review: http://cr.openjdk.java.net/~skovalev/8169721/webrev.00/
>>
>> Issue: most tests has missed dependency om java.naming module.
>> Solution: add missed module declaration for tests. If this possible, add
>> module declaration to TEST.properties file to avoid modification of each
>> individual test.
>>