RFR 8078812, Test RMI with client and servers as modules

[Felix Yang]

Hi Stuart,
      thanks for the comments. I adjusted the test.  The test failed 
with NoClassDefFoundErr if client/server are in named module but dummy 
app in unnamed module. See https://bugs.openjdk.java.net/browse/JDK-8153833

New webrev: http://cr.openjdk.java.net/~xiaofeya/8078812/webrev.01/ 
<http://cr.openjdk.java.net/%7Exiaofeya/8078812/webrev.01/>

About simplifying the scenario, I replaced inheritance by moving classes 
in runtime.

Thanks,
Felix

On 2016/3/2 10:28, Stuart Marks wrote:
> Hi Felix, sorry for the delay.
>
> Several comments below.
>
> --------------------------------------------------
>
>  119         // wait for rmiregistry to be fully ready
>  120         Thread.sleep(3000);
>
> There are some RMI test utilities that will handle starting up and 
> waiting for the registry to be ready. Unfortunately they're in the 
> unnamed package (still haven't cleaned that up) so you can't use them 
> from this test.
>
> For now I'd recommend scaling the timeout by the test.timeout.factor, 
> so that this won't fail on slow systems. There's a test utility for 
> that; see Utils.adjustTimeout().
>
> --------------------------------------------------
>
> The directory "unamed" is misspelled; it has classes in the unnamed 
> module. This misspelling is reflected in variable names and values, e.g.,
>
>   59     private static final Path UNAMED_SRC_DIR = 
> Paths.get(TEST_SRC, "unamed");
>   60     private static final Path MODS_OUTPUT_DIR = Paths.get("mods");
>   61     private static final Path UNAMED_OUTPUT_DIR = 
> Paths.get("unamed");
>
> While spelling errors aren't that big a deal, since this involves file 
> and directory names, I'd recommend fixing this now. It'll just be 
> harder to fix it later.
>
> Also, "SeperateModuleClient" => "SeparateModuleClient"
>
> --------------------------------------------------
>
> Overall the structure of the test seems reasonable to test various 
> clients and servers in combinations of the same, different, and 
> unnamed modules.
>
> I'm not entirely sure what p2.SeperateModuleClient is testing. It 
> extends p1.Client and its constructor and testStub() method simply 
> call the corresponding methods on super, so it doesn't seem to be 
> testing anything different from p1.Client running against p3.Server.
>
> Also, p4.UnamedModuleClient seems to want to run the client from the 
> unnamed module, but it too extends p1.Client and simply defers all of 
> its execution to its superclass. So I don't think this actually 
> testing an RMI client call originating from an unnamed module.
>
> There is an uncomfortable amount of duplication between 
> mtest/test/DummyApp.java and p4/UnamedModuleDummyApp. I see what this 
> is trying to do, which is to test a RMI server from an unnamed module. 
> It instantiates p3.Server, which resides in a named module, but 
> exports it from an unnamed module.
>
> So I think there's some tension here. There's subclassing among the 
> clients that attempts to avoid duplication, which is good, but it 
> doesn't truly seem to be testing clients in different modules and in 
> an unnamed module. The server, on the other hand, does seem to be 
> testing things properly in different modules, at the cost of 
> duplicating all the code into another class that resides in a 
> different (unnamed) module.
>
> I'm not entirely sure what the best approach is here. Ideally you'd 
> want to have a single implementation of client, server + remote 
> interface, and application, and compile each of them once. Then since 
> you're invoking a new JVM for each test, invoke it with different 
> options to put the client, or the server, or the app, into modules, or 
> onto the classpath (to get into an unnamed module). You might have to 
> copy compiled class files to different locations so that different 
> classes can be either on the classpath or in a named module without 
> causing any conflicts.
>
> I'm willing to entertain some simplifications here as well. It might 
> be sufficient to deal with just clients and servers in 
> different/unnamed modules, and not worry about putting the application 
> into different modules. That should reduce the number of cases to cover.
>
> s'marks
>
> On 2/29/16 10:05 PM, Felix Yang wrote:
>> Ping...
>>
>> -Felix
>> On 2016/2/24 14:06, Felix Yang wrote:
>>> Hi all,
>>>       please review the new tests to use RMI in module world.
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~xiaofeya/8078812/webrev.00/
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8078812
>>>
>>> Thanks,
>>> Felix
>>