Question about the bug https://bugs.openjdk.java.net/browse/JDK-8031179
yiming.wang at oracle.com
Fri Jan 24 07:14:07 UTC 2014
Thanks for the suggestion! sorry for reply this mail late as i was busy
on other tasks
The webrev has been in the internal review process. Based on the
suggestion, here is a summary of changes:
1. Add othervm options to tests:
2. Remove java/rmi and sun/rmi from othervm.dirs of TEST.ROOT
3. Filed a another bug https://bugs.openjdk.java.net/browse/JDK-8032629
On 2014/1/18 8:45, Stuart Marks wrote:
> Hi Eric,
> Thanks for doing the analysis of the tests that need /othervm. The
> list of tests that don't need /othervm looks good. One subtlety is
> this one:
> Most activation tests do need /othervm, but this one doesn't, since
> all it does is create an ActivationGroupDesc instance, which has no
> side effects. This is unusual for the activation APIs, since most of
> them are quite intertwined with the rest of the activation subsystem.
> So, for this test, the lack of /othervm warrants a comment explaining
> why /othervm is unnecessary.
> Regarding merging of the tests
> java/rmi/MarshalledObject/compare/Compare.java and HashCode.java, this
> is fairly subtle and not obviously wrong, but it's not obviously right
> either. Note that different jtreg tests -- even in agentvm or samevm
> mode -- load the test class in a fresh classloader, which means that
> the statics are reinitialized. This provides some degree of isolation
> of the tests even when they're reusing the same JVM. By contrast,
> calling the two different methods from within the same test exposes
> the second one to initializations left from the first one. In addition
> (though I'm not too concerned about this) if the first test fails the
> second won't be run at all. Merging these tests is kind of out of
> scope for this particular bug, and since I wasn't fully able to
> convince myself that the merged tests have the same semantics as the
> unmerged tests, I'd prefer not to see the merging of these tests as
> part of this changeset.
> (Some cleanup of these two tests is probably warranted eventually. I'd
> take a different approach of merging and refactoring the sources but
> keeping them as separate test runs, using two @run tags. But we should
> work on that separately. Meanwhile, the overhead of having these as
> separate tests is minimal, especially if they're not run in /othervm
> I don't think the removal of java/rmi/Naming from exclusiveAccess.dirs
> is safe at this point. The DefaultRegistryPort.java test uses the
> default registry port, not a unique one. Conceptually it would need to
> be converted to use the TestLibrary unique port stuff, but the test is
> actually about using the default port. So, the solution here isn't
> In addition, the
> java/rmi/Naming/legalRegistryNames/LegalRegistryNames.java test also
> uses the default registry port. It too will need to converted before
> java/rmi/Naming is removed from exclusiveAccess.dirs.
> The java/rmi/Naming/LookupIPv6.java test was converted to use the
> TestLibrary unique port technique, but only partially. The registry is
> created on the unique port, but the Naming.lookup() call on the last
> line of the test doesn't provide a port number, so it does the lookup
> on the default port instead. This will cause the test to fail in
> almost all cases.
> I have to ask, did you run this test with your modifications?
> (Well, the test would pass if IPv6 is not enabled on the machine
> running the test, but it only passes because the part of the test that
> creates and uses the registry is bypassed entirely if IPv6 is not
> enabled. If you're modifying code, you need to take responsibility for
> ensuring that the code being modified is actually being run and is
> doing what you expect.)
> LookupIPv6.java also needs to have these lines added to the test tags:
> * @bug 4402708
> + * @library ../testlibrary
> + * @build TestLibrary
> * @run main/othervm -Djava.net.preferIPv6Addresses=true LookupIPv6
> (Their absence will cause the test also to fail in a clean build, but
> the test will find a TestLibrary class if one had been compiled by a
> previous test that had required it.)
> Maybe we should separate the othervm changes (removal of the rmi dirs
> from othervm.dirs, and addition of /othervm) from the
> exclusiveAccess.dirs changes. A separate bug would be filed for
> exclusiveAccess.dirs. I know this means the bug count won't go down,
> but dealing with exclusiveAccess.dirs means that the logic of various
> tests will need to be change to use the unique port mechanism. This is
> a fair chunk of work and it's logically distinct from the othervm work.
> If we also remove the merging of the MarshalledObject tests, I think
> we can proceed with the othervm changes.
> Eric, can you make these changes and send out another webrev?
> On 1/16/14 7:18 AM, Eric Wang wrote:
>> Hi Stuart,
>> I have generated a webrev for review, can you please help to check
>> whether the changes are OK.
>> Also need your help to confirm that the rest 9 tests in previous mail
>> don't need the /othervm option indeed.
>> On 2014/1/16 22:55, Eric Wang wrote:
>>> Hi Stuart,
>>> I'm working on the bug
>>> https://bugs.openjdk.java.net/browse/JDK-8031179 to add /othervm
>>> option to rmi tests, so they can removed from the item othervm.dir
>>> and exclusiveAccess.dirs.
>>> By searching the directories java/rmi, sun/rmi and java/rmi/Naming,
>>> there are 14 tests without othervm option, but only 5 tests need to
>>> the /othervm tag. they are:
>>> Tests need /othervm option:
>>> Tests don't need the /othervm option:
>>> Also i have a bit confuse about the test Compare.java and
>>> HashCode.java in java/rmi/MarshalledObject/compare directory, as
>>> they should be merged into one test so that we don't need to add
>>> additional /othervm option for 2 test.
More information about the core-libs-dev