RFR (S): 8164660: MinimalVM is not tested with GC tests

[Thomas Schatzl]

Hi Dima,

On Mon, 2016-08-29 at 16:08 +0300, Dmitry Fazunenko wrote:
> Hi Thomas,
> 
> On 29.08.2016 11:18, Thomas Schatzl wrote:
> > 
> > Hi Dima,
> > 
> > On Fri, 2016-08-26 at 19:43 +0300, Dmitry Fazunenko wrote:
> > > 
> > > Hello,
> > > 
> > > I'm looking for reviewers for a simple change  needed for better
> > > selection of tests applicable for minimal VM.
> > > There are two sort of changes:
> > > - removing '@modules java.management' (if it's not needed)
> > > - adding @requires vm.gc.X  (to those tests which ignore external
> > > flags
> > > and keep passing even if conflicting collector is given)
> > > 
> > > https://bugs.openjdk.java.net/browse/JDK-8164660
> > > http://cr.openjdk.java.net/~dfazunen/8164660/webrev.00/
> >    previously tests in the hotspot/test/gc/g1 directory were
> > excluded by
> > the TEST.groups file. So why do they need the @requires tag now?
> TEST.groups is not used for test selection (filtering non applicable 
> tests out) for 1+ year.
> Adding @requires vm.gc.G1 to those tests is not absolutely needed, it
> just helps to not run those tests which ignore external flag.

Okay :)

> 
> > 
> > 
> > TestStringSymbolTableStats.java is still missing the @requires tag
> > after this change.
> My fault, thanks for catching this.
> Fixed:
> http://cr.openjdk.java.net/~dfazunen/8164660/webrev.01/
> 
> 

Looks good. Thanks.

> 
> > 
> > 
> > Also, I think that none of the tests where the change adds the
> > @requires vm.gc.g1, needs java.management.
>
> It's another story...
> A test may depend on java.management if a dependent library uses it.
> In our case ProcessTools imports
> java.lang.management.ManagementFactory, 
> so each test using ProcessTools does potentially need the 
> java.management module.
> But 'import' doesn't mean 'use'. In case of ProcessTools the only
> tests 
> invoking ProcessTools.executeTestJvmAllArgs really need
> java.management.
> I found only two such hotspot tests:
>      compiler/jvmci/compilerToVM/DebugOutputTest.java
>      compiler/compilercontrol/share/scenario/Executor.java
> For other tests which don't use java.lang.management directly
> dependency on the java.management module could be removed.
> But this change is outside of the scope of 8164660: MinimalVM is not 
> tested with GC tests
> 
> I will submit a separate CR for implementing this.

Okay.

Ship it :)

Thomas