RFR(S/M) : 8243565 : some gc tests use 'test.java.opts' and not 'test.vm.opts'

[Stefan Karlsson]

On 2020-04-25 01:06, Igor Ignatyev wrote:
> http://cr.openjdk.java.net/~iignatyev/8243565/webrev.00/
>> 335 lines changed: 12 ins; 191 del; 132 mod;
> 
> Hi all,
> 
> could you please review the patch which updates a dozen of gc tests to pass both 'test.java.opts' and 'test.vm.opts' to JVM under test? to achieve that, the patch removes code which was reading test.java.opts and joing it w/ other flags, and instead uses ProcessTools.createJavaProcessBuilder(boolean addTestVmAndJavaOptions=true, String...), , which prepends values of both properties. in some tests, it led to removal of diagnostic output, this output is redundant as ProcessTools/OutputAnalyzer not only prints out information about failed processes, but also dumps it in working directory (see 8174768).
> 
> the patch also removes redundant @modules, and replaces @run main w/ @run driver in all these tests as there is no need of the "main" test classes to be run w/ any of external flags. also in order to reduce confusion $XOpts variables were renamed to $XFlags.
> from JBS:
>> a dozen of gc tests use 'test.java.opts' to get external vm flags, yet actually external flags are split b/w 'test.java.opts' and 'test.vm.opts' and in the majority of cases all external flags are set listed in 'test.vm.opts'. so the tests should be updated to read both.
> 
> webrev: http://cr.openjdk.java.net/~iignatyev/8243565/webrev.00

The code changes looks good. I don't understand the @module parts, so 
that needs to be reviewed by someone else.

StefanK

> JBS: https://bugs.openjdk.java.net/browse/JDK-8243565
> 
> Thanks,
> -- Igor
>