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

[Igor Ignatyev]

> On Apr 27, 2020, at 3:02 AM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
> 
> 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

thanks for review Stefan!

I can try to explain @modules part to you : 
 - in most cases I removed '@modules java.base/jdk.internal.misc' which was adding --add-export flag so jdk.internal.misc package was available for a test. if a test really depended on classes from jdk.internal.misc, it would fail to compile
- in TestHumongousCodeCacheRoots, I also removed '@modules java.management' which simply was telling jtreg to run this test only if java.management module exists and is available. the test doesn't use any of API provided by  java.management (directly or indirectly) so this tag can be safely removed.

all these @modules were needed before, b/c some of generally used jdk.test.lib. classes have dependencies on jdk.internal.misc package and java.management module, but these dependencies got removed by 8164737 and 8164944, and there are 8185163 and 8178416 to clean up @modules in our tests. so if you prefer, I can exclude @modules changes from 8243565 and send them out as a fix for another RFE.

-- Igor

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