RFR 8224137: Analyze and port invocation tests to jtreg and co-locate to jdk repo

[Harold Seigel]

Hi David,

Please see comments inline.

On 6/24/2019 4:15 PM, David Holmes wrote:
> Hi Harold,
>
> On 24/06/2019 1:04 pm, Harold Seigel wrote:
>> Hi David,
>>
>> Thanks for pointing this out.  My intent is to push the current tests 
>> and then create an enhancement to improve them.  I'd like to add C1 
>> and Graal testing as part of the improvements.
>
> Okay. Wasn't sure why you went for the -Xint / -Xcomp additions at 
> this stage? :)
I plan to also test c1 and graal but would like to start with these.
>
>> Also, note that passing TRUE as the first argument to 
>> ProcessBuild.createJavaProcessBuilder() adds test.vm.opts and 
>> test.java.opts to the java arguments.
>
> Okay so your explicit Xint and Xcomp can conflict with what gets 
> passed through by jtreg. I'm not sure which order flags will be 
> presented, so even if "last flag wins" it's not clear exactly what 
> will get tested. In particular using Xcomp when jtreg passes in Graal 
> flags, can cause issues.
Is there an open bug for the Xcomp / Graal flags issue?
>
> So maybe for this initial push explicit mode selection should be avoided?

I think it would be better to pass FALSE as the first argument to 
ProcessBuild.createJavaProcessBuilder(). Then I will know what is 
getting tested.

Thanks, Harold

>
> Thanks,
> David
> -----
>
>> Harold
>>
>> On 6/21/2019 9:18 PM, David Holmes wrote:
>>> Hi Harold,
>>>
>>> Not a review ...
>>>
>>> On 21/06/2019 10:55 am, Harold Seigel wrote:
>>>> Hi Coleen,
>>>>
>>>> Can you take one more look?
>>>>
>>>> http://cr.openjdk.java.net/~hseigel/bug_8224137.3/webrev/
>>>>
>>>> I changed three files, invokespecialTests.java, 
>>>> invokeinterfaceTests.java, and invokevirtualTests.java to 
>>>> explicitly specify -Xint and -Xcomp to the sub-tests.
>>>
>>> If you want to check all the compilation environments you need to be 
>>> more elaborate than just using -Xcomp. You may need to test C1 only 
>>> and C2 only. And then you need to consider Graal as well.
>>>
>>> IIUC from a brief look at these tests they will never pass through 
>>> the VM flags given to jtreg - is that correct?
>>>
>>> Thanks,
>>> David
>>>
>>>> Thanks again!
>>>>
>>>> Harold
>>>>
>>>> On 6/20/2019 6:37 PM, coleen.phillimore at oracle.com wrote:
>>>>> Great, thanks. Looks good!
>>>>> Coleen
>>>>>
>>>>> On 6/20/19 1:00 PM, Harold Seigel wrote:
>>>>>> Hi Coleen,
>>>>>>
>>>>>> Thanks for reviewing this change!
>>>>>>
>>>>>> Please review this updated webrev:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~hseigel/bug_8224137.2/webrev/index.html
>>>>>>
>>>>>> The -XDignore.symbol.file has been removed and TEST.groups has 
>>>>>> been changed.  Otherwise, it is the same as the previous webrev.
>>>>>>
>>>>>> Mgmt said to leave the copyright years as 2009, 2019, so I did 
>>>>>> not change those.
>>>>>> Thanks! Harold
>>>>>>
>>>>>> On 6/19/2019 5:41 PM, coleen.phillimore at oracle.com wrote:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~hseigel/bug_8224137/webrev/test/hotspot/jtreg/runtime/InvocationTests/invokeinterfaceTests.java.html 
>>>>>>>
>>>>>>>
>>>>>>>   35  * @compile -XDignore.symbol.file 
>>>>>>> invokeinterface/Checker.java invokeinterface/ClassGenerator.java
>>>>>>>
>>>>>>>
>>>>>>> Do these still need -XDignore.symbol.file ?
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~hseigel/bug_8224137/webrev/test/hotspot/jtreg/TEST.groups.udiff.html 
>>>>>>>
>>>>>>>
>>>>>>> Can you specify all the tests in the directory by directory? like:
>>>>>>>
>>>>>>> + -runtime/InvocationTests \
>>>>>>>
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~hseigel/bug_8224137/webrev/test/hotspot/jtreg/runtime/InvocationTests/shared/Caller.java.html 
>>>>>>>
>>>>>>>
>>>>>>>    2  * Copyright (c) 2009, 2019, Oracle and/or its affiliates. 
>>>>>>> All rights reserved.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Some of the copyrights say 2009, which is when the tests were 
>>>>>>> written but I think we're supposed to have the original date 
>>>>>>> when they're added to the repository.
>>>>>>>
>>>>>>> Well done getting these tests into jtreg and the repository!
>>>>>>> Thanks,
>>>>>>> Coleen
>>>>>>>
>>>>>>> On 6/17/19 2:57 PM, Harold Seigel wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Please review this JDK-14 change to move the invocation tests 
>>>>>>>> written by Vladimir Ivanov into the JDK repo and make them 
>>>>>>>> runnable using JTReg.
>>>>>>>>
>>>>>>>> This webrev adds three tests, invokeinterfaceTests.java, 
>>>>>>>> invokespecialTests.java, and invokevirtualTests.java. Each 
>>>>>>>> tests run its set of sub-tests twice, once using class file 
>>>>>>>> version 51 and once using the current class file version.
>>>>>>>>
>>>>>>>> Open Webrev: 
>>>>>>>> http://cr.openjdk.java.net/~hseigel/bug_8224137/webrev/
>>>>>>>>
>>>>>>>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8224137
>>>>>>>>
>>>>>>>> The tests were tested on Linux-x64, Solaris, Windows, and Mac 
>>>>>>>> OS X.
>>>>>>>>
>>>>>>>> The original tests can be found attached to JDK-8163974 
>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8163974>. Besides the 
>>>>>>>> changes needed for JTReg and adding copyrights, I made the 
>>>>>>>> following additional changes.
>>>>>>>>
>>>>>>>> 1. The tests now use the JDK's asm support instead of providing 
>>>>>>>> its own
>>>>>>>>    asm libraries.
>>>>>>>> 2. Only sub-test failures are written to the .jtr files. 
>>>>>>>> Writing all
>>>>>>>>    sub-test results caused JTReg to truncate the output.
>>>>>>>> 3. Changed src/invokeinterface/Checker.java to skip private 
>>>>>>>> methods
>>>>>>>>    when looking for an overloading method.
>>>>>>>>
>>>>>>>> The tests contain "TODO" comments and other thing needing 
>>>>>>>> clean-up.  These will be addressed in a future RFE.
>>>>>>>>
>>>>>>>> I put the tests into hs-tier3 because the 
>>>>>>>> invokeInterfaceTests.java test can run for up to 10 minutes (on 
>>>>>>>> Mac).  The other two tests take only 1-2 minutes.  Is there a 
>>>>>>>> better tier for these tests?
>>>>>>>>
>>>>>>>> Thanks, Harold
>>>>>>>>
>>>>>>>
>>>>>