Ping - Re: RFR 8078812, Test RMI with client and servers as modules

[Felix Yang]

Hi Stuart,
      any comment on the new tests and issue?

Thanks,
Felix
On 2016/4/8 15:52, Felix Yang wrote:
> 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
>>>
>