RFR(M) : 8251132 : make main classes public in vmTestbase/jit tests

[Vladimir Kozlov]

On 8/5/20 9:44 AM, Igor Ignatyev wrote:
> 
> 
>> On Aug 5, 2020, at 9:16 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>
>> Hi Igor
>>
>> We were always told to use '/othervm' only if additional VM flags are specified.
>> Also based on RFE description making classes public will allow to execute them directly by jtreg.
>>
>> So why you use  '/othervm'?
> 
> /othervm tests are run directly by jtreg, as opposed to tests which use ExecDriver, where jtreg runs ExecDriver and ExecDriver spawns a new process to run a test.
> 
> I used to /othervm to keep the tests closer to their current state, i.e. each test is run in a separate clean JVM. removing /othervm would require a bit more detail analysis on wherever these tests really require clean state, I'd prefer to do separately.

Okay. Add this to RFE comment to avoid confusion later.

> 
>>
>> Also since you cleaning all this test can you use uniform format for class declaration line.
>> I see different variations:
>>
>> public class DivTest{
>>
>> public class Filtering
>> {
>>
>> public class Robert
>>     {
>>
>> public class collapse {
>>
>> I think the last example is what we usually use.
>>
>> Code indent is also all over places.
>>
>> I understand that fixing many files by hand would be hard. But we you can do something (with script) which will not take a lot of your time we should do that.
> I guess I can run some auto-formater on all these files, yet to make it cleaner I'd prefer to do it by another RFE.

Agree.

Thanks,
Vladimir K

> 
> -- Igor
> 
>>
>> Thanks,
>> Vladimir K
>>
>> On 8/4/20 10:18 PM, Igor Ignatyev wrote:
>>> http://cr.openjdk.java.net/~iignatyev//8251132/webrev.00
>>>> 498 lines changed: 0 ins; 132 del; 366 mod;
>>> Hi all,
>>> could you please review the patch which adds public modifier to "main" test classes in vmTestbase/jit tests?
>>> from JBS:
>>>> main test classes of several vmTestbase/jit tests are package-private, as a result, jtreg can't run them directly and we had to use `driver ExecDriver --java ` to run them.
>>>>
>>>> this RFE is to make these classes public and to replace ExecDriver w/ regular `main/othervm` where appropriate.
>>> the patch also removes ExecDriver and @build in all but 6 tests. those 6 (vmTestbase/jit/t/t108--t113) compare stack traces to the golden ones, and execution them "directly" by jtreg will lead to failures due to a few extra frames from jtreg.
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8251132
>>> testing: :vmTestbase_vm_compiler on {linux,windows,macos}-x64
>>> webrev: http://cr.openjdk.java.net/~iignatyev//8251132/webrev.00
>>> Thanks,
>>> -- Igor
>