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
