> On Oct 27, 2015, at 12:17 AM, Mandy Chung <[email protected]> wrote: > > >> On Oct 26, 2015, at 10:13 AM, Alexandre (Shura) Iline >> <[email protected]> wrote: >> >> Hi. >> >> Could you please take a look on the suggested change in tests belonging to >> jdk_core test group. >> >> Pls notice that the dependencies which would later be removed by fixing >> JDK-8139430 we not added in this change. >> >> http://cr.openjdk.java.net/~shurailine/8140336/webrev.01/ > > Thanks for the patch. > > This change is against jake/jdk repo. I assume you intend to integrate this > change to jdk9/dev/jdk. You can separate into two changesets, one for > jdk9/dev/jdk and the other for test/jdk/jigsaw tests that I can push your > change to test/jdk/jigsaw tests to jake.
I have created two separate reviews: http://cr.openjdk.java.net/~shurailine/8140336/webrev.jdk9.02/ http://cr.openjdk.java.net/~shurailine/8140336/webrev.jake.02/ > > test/java/lang/ProcessHandle/TEST.properties > - which test references types in jdk.management? If there is only a couple > of tests depending on jdk.management, better to use @modules in those > individual tests instead. The dependency is coming through java/lang/ProcessHandle/JavaChild.java. Three of five tests in the dir use it: InfoTest.java, OnExitTest.java, TreeTest.java. Yes, I could update just the three tests, but since JTReg will compile all the classes in the dir, the compilation for the rest of the tests will fail if no jdk.management present. Instead I would rather have the fix as it is right now and have a separate bug to fix this later. Which is what I was going to do. This is, effectively, the opposite of what I was suggesting with regards to JDK-8139430. The difference is that in the case of java/lang/ProcessHandle tests, I will only need to update two tests twice and in the case of JDK-8139430 it’s a _lot_ of tests. > > Is java.management module pulled in due to the test library in the following > tests? > test/java/lang/instrument/RedefineBigClass.sh > test/java/lang/instrument/RedefineMethodInBacktrace.sh > test/java/lang/instrument/RetransformBigClass.sh > test/java/lang/instrument/NativeMethodPrefixAgent.java The dependency is coming through test/java/lang/instrument/NMTHelper.java, test/java/lang/instrument/RedefineMethodInBacktraceApp.java, test/java/lang/instrument/NativeMethodPrefixApp.java > > It’s okay to remove java.management dependency later by JDK-8139430. At the > same time, would it be easier not to update the tests that require > java.management due to the test library and let JDK-8139430 fixes it? See above. I would rather push this and get on fixing JDK-8139430. Shura > > Mandy > >
