RFR [XS] : 8229182: runtime/containers/docker/TestMemoryAwareness.java test fails on SLES12

Severin Gehwolf sgehwolf at redhat.com
Tue Aug 27 14:33:36 UTC 2019


On Tue, 2019-08-27 at 14:20 +0000, Baesken, Matthias wrote:
> Hello  ,   my suggestions was " appendJavaOpts "   :
> 

OK, got it now. Looking at the webrev this wasn't clear as it didn't do
that.

>      In case we want to be safe,  I suggest to
> add  an  additional  method   " appendJavaOpts "   
> 
>      opts.appendJavaOpts("-Xmx" + javaHeapSize);
> 
>      to DockerRunOptions ;  appendJavaOpts   would append  the
> options  after  the  testJavaOpts    (so Xmx can be overwritten ).
> 
> But if  overrideJavaOpts    is preferred  that fine with me too ...

I'd prefer a more telling name than "append". I'd ask myself when
trying to use it: Does it append to regular java options? How is it
different from addJavaOpts()? You get the idea. I'd prefer
overrideJavaOpts for that reason.

Perhaps addOverrideOpts()? Either way, the javadoc should make it clear
what adding options there means.

Thanks,
Severin

> Best regards, Matthias
> 
> 
> > On Tue, 2019-08-27 at 13:49 +0000, Baesken, Matthias wrote:
> > > http://cr.openjdk.java.net/~mbaesken/webrevs/8229182.3/
> > 
> > OK, so, in summary, the constraints are as follows:
> > 
> >  * We don't want to turn of appending global test java options as
> > these
> >    could possibly contain local options to a specific test
> > environment.
> >    It would be a surprise if docker tests wouldn't have them.
> >  * Just setting Java options is not enough as the intention is to
> > set
> >    them for the Docker test, but could potentially be overridden by
> >    global test options (this issue).
> > 
> > Given we'd like to keep that general design, maybe it would make
> > sense
> > to create a 3'rd, separate set of flags, neither java options nor
> > test
> > options would manage to override. Call it "overrideJavaOpts" or
> > some
> > such. At least that would make it clear that these options if set
> > have
> > the final say.
> > 
> > Thoughts?
> > 
> > Thanks,
> > Severin



More information about the hotspot-dev mailing list