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.
