Christian, Ioi, Are you OK with this changes?
http://cr.openjdk.java.net/~dsamersoff/JDK-8160923/webrev.03/ -Dmitry On 2016-08-24 14:42, Dmitry Samersoff wrote: > Christian, > > Thank you for the review. > > Please see updated webrev: > > http://cr.openjdk.java.net/~dsamersoff/JDK-8160923/webrev.03/ > > I still have no ideas why this @build construction works with > @run driver but doesn't work with @run main/othervm. > > Is there a chance to have all such knowledge documented? > >> You don't need to explicitly build JpsHelper, > > I would prefer to leave it as is - it's harmless but highlights > TestJpsJar dependency. > >> would it make sense to change this to use the /test/lib ones and > > I'd tried it[1] and it doesn't work. jtreg claims that package > jdk.test.lib doesn't exist.[2] > > > 1. > http://cr.openjdk.java.net/~dsamersoff/JDK-8160923/webrev.02.bad/ > > 2. > http://cr.openjdk.java.net/~dsamersoff/JDK-8160923/webrev.02.bad/TestJpsClass.jtr > > -Dmitry > > On 2016-08-23 22:10, Christian Tornqvist wrote: >> Hi Dmitry, >> >> You don't need to explicitly build JpsHelper, >> I also noticed that >> you're using ProcessTools and OutputAnalyzer from /lib/testlibrary , >> would it make sense to change this to use the /test/lib ones and >> simply have: >> >> @library /test/lib >> >> ? >> >> Thanks, Christian -----Original Message----- From: >> hotspot-runtime-dev >> [mailto:hotspot-runtime-dev-boun...@openjdk.java.net] On Behalf Of >> Dmitry Samersoff Sent: Tuesday, August 23, 2016 3:02 PM To: Ioi Lam >> <ioi....@oracle.com>; serviceability-dev@openjdk.java.net; >> hotspot-runtime-dev <hotspot-runtime-...@openjdk.java.net> Subject: >> Re: PING! Re: RFR(XS): JDK-8160923: sun/tools/jps/TestJpsJar.java >> fails due to ClassNotFoundException: jdk.testlibrary.ProcessTools >> >> Ioi, >> >> Thank you for review. >> >> Hmm. It looks like changes below solves the problem. >> >> - * @build jdk.testlibrary.* JpsHelper JpsBase + * @build JpsHelper >> JpsBase >> >> I'm running rbt job to verify it. >> >> -Dmitry >> >> On 2016-08-23 16:10, Ioi Lam wrote: >>> Hi Dmitry, >>> >>> Why are you adding /test/lib: >>> >>> - * @library /lib/testlibrary + * @library /lib/testlibrary >>> /test/lib >>> >>> The only class used by jdk/test/sun/tools/jps/*.java in /test/lib >>> is here: >>> >>> TestJpsSanity.java:import jdk.test.lib.apps.LingeredApp; >>> >>> But TestJpsSanity.java is not use by this test -- I ran the test >>> with your patch in a clean jtreg directory and the test passed, but >>> I don't see TestJpsSanity.class, or any jdk.test.lib.* class. >>> >>> So I don't think you need to add /test/lib. >>> >>> - Ioi >>> >>> On 8/23/16 5:34 AM, Dmitry Samersoff wrote: >>>> On 2016-08-17 10:51, Dmitry Samersoff wrote: >>>>> Everybody, >>>>> >>>>> Please review the changes: >>>>> >>>>> http://cr.openjdk.java.net/~dsamersoff/JDK-8160923/webrev.01/ >>>>> >>>>> -Dmitry >>>>> >>>> >>> >> >> >> -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, >> Russia * I would love to change the world, but they won't give me the >> sources. >> > > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
