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

[David Holmes]

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? :)

> 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.

So maybe for this initial push explicit mode selection should be avoided?

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
>>>>>>>
>>>>>>
>>>>