RFR(M): 8185436: jtreg: introduce keyword to disable cds tests

[Lindenmaier, Goetz]

Hi,

I made new webrevs implementing the change with @requires:
http://cr.openjdk.java.net/~goetz/wr17/8185436-cdsKey/webrev.02/
http://cr.openjdk.java.net/~goetz/wr17/8185436-cdsKey/webrev.02-hs/

I also changed the bug description and synopsis.

For the jtreg runner I would propose to set the property test.jdk
so that it is available in VMProps.  Igor also ran into this issue.

Best regards,
  Goetz.


> -----Original Message-----
> From: Mikhailo Seledtsov [mailto:mikhailo.seledtsov at oracle.com]
> Sent: Montag, 31. Juli 2017 22:19
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
> Cc: hotspot-runtime-dev at openjdk.java.net
> Subject: Re: RFR(M): 8185436: jtreg: introduce keyword to disable cds tests
> 
> Hi Goetz,
> 
> I have an idea on how to address your second use case.
> The idea is to define a special test property (e.g.
> test.cds.disable.cds.support) which will override logic inside the
> VMProps.vmCDSSupported(). If this property is defined to "true" in test
> invocation command then vmCDSSupported() returns false (CDS is disabled,
> not supported), and all tests marked with "@requires vm.cds.supported"
> will be skipped.
> 
> How to use it:
> jtreg -Dtest.cds.disable.cds.support=true <tests-to-run>
> E.g.:    jtreg -Dtest.cds.disable.cds.support=true
> hs/hotspot/test/runtime/SharedArchiveFile/ArchiveDoesNotExist.java
> 
> I prototyped this approach, it works for me. I have attached the diff.
> Let me know whether this works for your use case, or if you have any
> questions.
> 
> 
> Thank you,
> Mikhailo
> 
> 
> On 7/31/17, 1:45 AM, Lindenmaier, Goetz wrote:
> > Hi Mikhailo,
> >
> > Basically I'm fine with using the @requires property.
> > But is there a way to overrule the outcome of the method
> > implemented In VMProps.java computing the property?
> > I have two use cases for the key I want to introduce.
> >
> > First, our internal VM (we are Oracle licensees) is compiled without
> > CDS support. Thus we don't want to run the CDS tests. Currently
> > we have them all listed in the ProblemList, but that's not nice, especially
> > because we have to adapt it whenever a new test is added.
> > As I understand, the @requires property works fine, here.
> >
> > Second, we also test the two ports we contributed (ppc and s390). These
> contain
> > rudimentary cds support and so far passed all tests.  Unfortunately it broke
> > lately in jdk10.  Instead of fixing it (our people are working on finishing our
> > internal Java 9 port) I would like to switch off all cds tests.
> > As I can set the key on the command line of jtreg, I easily can do that.
> > Is there a way to do similar with the @requires property?
> >
> > Best regards,
> >    Goetz.
> >
> >
> >
> >> -----Original Message-----
> >> From: Mikhailo Seledtsov [mailto:mikhailo.seledtsov at oracle.com]
> >> Sent: Freitag, 28. Juli 2017 23:53
> >> To: Lindenmaier, Goetz<goetz.lindenmaier at sap.com>
> >> Cc: hotspot-runtime-dev at openjdk.java.net
> >> Subject: Re: RFR(M): 8185436: jtreg: introduce keyword to disable cds
> tests
> >>
> >> Hi Goetz,
> >>
> >>     I am a HotSpot SQE Engineer at Oracle. I have discussed your proposed
> >> fix with Igor Ignatyev (also VM SQE Engineer), and we have the following
> >> feedback on this change.
> >>
> >> 1. As part of streamlining and simplifying SQE process and the use of
> >> test tools we have narrowed down the test selection mechanisms.
> >>
> >> 2. Our preferred test selection mechanism is use of "@requires" and a
> >> corresponding test/jtreg-ext/requires/VMProps.java. Even though JTREG
> >> supports use of "@key",  we prefer the use of "@requires" as a first
> choice.
> >>
> >> 3. If it is not possible to use "@requires" for a given situation then
> >> use "@key" mechanism. We would ask you if you could explore the
> >> possibility of implementing this change via @requires first.
> >>
> >>
> >> Here are several hints that may help:
> >>
> >> 1. Take a look at<top>/test/jtreg-ext/requires/VMProps.java. The value
> >> of a given "requires property" is evaluated inside this file and placed
> >> into a map (see public call() method). Add your evaluation code here,
> >> and then follow the pattern used for other properties. Create a property
> >> (e.g. vm.cds.supported, with values of true/false). Create a method that
> >> evaluates the property value (e.g. isCDSSupported() or similar).
> >>
> >> 2. The method could use several options to evaluate whether CDS is
> >> supported.
> >>       A. WhiteBox API. Create a new WB test API method which can return
> >> true if CDS_ compiler flag is defined, otherwise false.
> >>           Call WB API from VMProps.java. See
> >> WB.getBooleanVMFlag("EnableJVMCI") as an example. Or create your
> own
> >> WB.isCDSSupported()
> >>           WhiteBox.java resides in test/lib/sun/hotspot/WhiteBox.java
> >>
> >>       B. Another options is to evaluate by running VM with sharing on and
> >> checking the return (may be not as reliable as option A)
> >>       C. Other ideas welcome.
> >>
> >> 3. Include "@requres vm.cds.supported == true" to the appropriate tests.
> >>
> >> Let me know if you have any questions.
> >>
> >>
> >> Best regards,
> >> Mikhailo
> >>
> >>
> >> On 7/28/17, 12:58 AM, Lindenmaier, Goetz wrote:
> >>> Hi
> >>>
> >>> we compile the VM without CDS support. Thus the CDS tests
> >>> fail. This change introduces a keyword 'cds' and marks
> >>> the tests accordingly.
> >>> This change also fixes the keywords specified in
> >> gc/g1/TestSharedArchiveWithPreTouch.java.
> >>> There may only be one @key keyword in the test specification.
> >>> In runtime/CompressedOops/CompressedClassPointers.java only one
> test
> >>> case required CDS. I changed this sub case to succeed if CDS is not
> >>> available.
> >>>
> >>> Please review this change. I please need a sponsor.
> >>> http://cr.openjdk.java.net/~goetz/wr17/8185436-cdsKey/webrev.01/
> >>>
> >>> Best regards,
> >>>     Goetz.