Ping - Re: RFR 8078812, Test RMI with client and servers as modules
Felix Yang
felix.yang at oracle.com
Fri Apr 29 05:08:10 UTC 2016
Hi Stuart,
thanks a lot for the comments!
-Felix
On 2016/4/28 9:06, Stuart Marks wrote:
> Hi Felix,
>
> I finally got a chance to take a good look at this. Again, sorry for
> the delays. I was able to reproduce the failure and find a fix for it.
> There are also some structural issues with the test that will require
> some improvements.
>
> * * *
>
> First is the way the test fixture is set up. The test now has a single
> copy of the source code for the client, server, and app, and they're
> compiled once. This is good. Unfortunately, the tests modify the
> filesystem in order to set up different cases for the classes' modular
> structure.
>
> The problem is that makes the tests dependent on each other. The
> Test-NG directives enforce this execution order, but it makes them
> really hard to work with. For example, if I were to disable test #2,
> this would cause test #3 to fail since #3 depends on side effects from
> test #2. Also, if test #2 were to fail, it's hard to debug, since test
> #3 modifies the fixture after test #2 runs, potentially obscuring any
> problems that #2 might have run into.
>
> Ideally the test should set up a single test fixture and not modify
> it, allowing the different cases to be run independently. Offhand I'm
> not sure of the best way to do this.
>
> One approach that might be worth exploring is to create several
> different jar files from the class files; the jar files would have the
> classes in different modular configurations. Then, each test would
> invoke a JVM with arguments to put the different jar files on the
> classpath or the module path. I haven't investigated this approach,
> though, so I don't know if it's practical.
>
> Other approaches may be viable as well. It's probably worth discussing
> and prototyping an approach first before you spend too much time
> implementing any particular scheme.
It was chosen to compile once considering efficiency. If we prefer each
test can be executed separately, it looks enough to just change
"@BeforeTest" to "@BeforeMethod" and update tests slightly.
With jar files, may I understand it is a different scenario with
automatic modules. I suggest to add a new test in future enhancement.
See http://cr.openjdk.java.net/~xiaofeya/8078812/webrev.02
-Felix
>
> * * *
>
> Moving on to the actual test failure, the message from
> testUnnamedApp() is as follows:
>
> ==================================================
> Command line:
> [/Users/src/jdk-dev/jdk9-dev/build/macosx-x86_64-normal-server-fastdebug/jdk/bin/java
> -ea -esa -mp mods
> -Djava.rmi.server.codebase=file:/Users/src/jdk-dev/jdk9-dev/jdk/testoutput/JTwork/java/rmi/module/ModuleTest/./mods/mserver/
> -cp classes testpkg.DummyApp 65071 ]
>
> Error: A JNI error has occurred, please check your installation and
> try again
> Exception in thread "main" java.lang.NoClassDefFoundError:
> serverpkg/Hello
> at java.lang.Class.getDeclaredMethods0(java.base/Native Method)
> at
> java.lang.Class.privateGetDeclaredMethods(java.base/Class.java:2937)
> at
> java.lang.Class.privateGetMethodRecursive(java.base/Class.java:3282)
> at java.lang.Class.getMethod0(java.base/Class.java:3252)
> at java.lang.Class.getMethod(java.base/Class.java:1961)
> at
> sun.launcher.LauncherHelper.validateMainClass(java.base/LauncherHelper.java:648)
> at
> sun.launcher.LauncherHelper.checkAndLoadMain(java.base/LauncherHelper.java:499)
> Caused by: java.lang.ClassNotFoundException: serverpkg.Hello
> at
> jdk.internal.loader.BuiltinClassLoader.loadClass(java.base/BuiltinClassLoader.java:366)
> at
> jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(java.base/ClassLoaders.java:184)
> at java.lang.ClassLoader.loadClass(java.base/ClassLoader.java:419)
> ... 7 more
>
> test ModuleTest.testUnnamedApp(): failure
> java.lang.AssertionError: expected [0] but found [1]
> [remainder of stack trace elided]
> ==================================================
>
> The main class is testpkg.DummyApp, but the launcher fails with
> NoClassDefFoundError on serverpkg/Hello. This was a bit of a puzzle,
> but I finally figured out that this is occurring during the
> verification of the DummyApp class. DummyApp depends on Hello, which
> can't be found, so it fails verification before DummyApp.main() gets
> called.
>
> It's also strange that it says "A JNI error has occurred" but that
> might be a red herring.
>
> In any case the real issue is that serverpkg.Hello can't be found. The
> reason it can't be found is that it's in a module. That module is on
> the module path, but the module isn't resolved by default. Thus its
> classes aren't visible to classes in the unnamed module.
>
> To add modules to the set of resolved modules, use the -addmods
> option. In this case, testUnnamedApp() has the client and server
> classes in modules, so it needs to have the arguments
>
> -addmods mclient,mserver
>
> added to its command line. Since the mclient module explicitly depends
> on the mserver module, this could instead just e
>
> -addmods mclient
>
> The testUnnamedAppandClient() method has only the server in a named
> module, so it needs to have
>
> -addmods mserver
>
> on its command line.
>
> With these changes, all the tests pass.
Thanks for figuring this out. But do you think it is worth to
investigate the " NoClassDefFoundError " and "A JNI error has occurred".
The root cause is not well-indicated.
-Felix
>
> * * *
>
> There are a couple other things that could be improved as well. I'll
> just mention them here (so I don't forget) but we should work on these
> later:
>
> 1) The registry and codebase stuff is probably unnecessary and can be
> removed. These might be interesting test cases for the future. It's
> not clear to me that we need to support modules in the RMI codebase,
> but at least there should be tests for cases we think are important.
Excuse me, could you explain what to remove in detail? I agree the
scenarios can be various beyond the sanity ones.
-Felix
>
> 2) In DummyApp, if creating the Client fails (e.g., because of
> NoClassDefFoundError), that error will propagate out of the main()
> method, bypassing the calls to System.exit(). The Server object has
> already been exported, which will cause the JVM to live indefinitely.
> This causes the test to hang and eventually time out, and to leave
> stale JVM processes after the test run.
There is explicit System.exit(-1) for error situation, so I didn't
observe such stale process. Any way, I updated it slightly.
-Felix
>
> In any case, I think the next step is to come up with a good design
> for varying the modular structure of the classes for the different
> test cases, without modifying the filesystem. Let's work on that, and
> we can work on the other stuff later.
>
> s'marks
>
>
>
>
>
> On 4/14/16 1:45 AM, Felix Yang wrote:
>> 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
>>>>>
>>>
>>
More information about the jigsaw-dev
mailing list