RFR(S) : 8242471 : remove "temporarily" restrictions of nsk/jdi/Argument/value/value004

[Alex Menkov]

LGTM

--alex

On 04/13/2020 10:35, Igor Ignatyev wrote:
> sure.
> 
> @alias, can I get a 2nd review for this patch?
> 
> Thanks,
> -- Igor
> 
>> On Apr 9, 2020, at 9:45 PM, Chris Plummer <chris.plummer at oracle.com 
>> <mailto:chris.plummer at oracle.com>> wrote:
>>
>> Hi Igor,
>>
>> I think it's best to get another review.
>>
>> thanks,
>>
>> Chris
>>
>> On 4/9/20 8:49 PM, Igor Ignatev wrote:
>>> Thanks Chris! Would you consider this trivial or should I wait for 
>>> another review?
>>>
>>> — Igor
>>>
>>>> On Apr 9, 2020, at 8:35 PM, Chris Plummer <Chris.Plummer at oracle.com> 
>>>> wrote:
>>>>
>>>> 
>>>> Hi Igor,
>>>>
>>>> The changes looks good.
>>>>
>>>> thanks,
>>>>
>>>> Chris
>>>>
>>>> On 4/9/20 4:13 PM, Igor Ignatyev wrote:
>>>>> Hi Chris,
>>>>>
>>>>> I looked what transport got skipped on Windows, and it's dt_shmem, 
>>>>> as I didn't see any reasons why this test should not be run w/ this 
>>>>> transport, I looked deeper and I found the place which needs to be 
>>>>> updated to make the test work w/ dt_shmem. so now the test doesn't 
>>>>> skip any transports (and thus the removed warning isn't even 
>>>>> partially true) and only skips connector other than 
>>>>> c.s.j.RawCommandLineLaunch, to make it cleaner the test now logs 
>>>>> the skipped connectors, e.g. on my mac, I'm getting the following 
>>>>> in .jtr
>>>>>> skipping com.sun.jdi.CommandLineLaunch (defaults: 
>>>>>> home=/Users/iignatye/ws/jdk/jdk/build/macosx-x64/images/jdk, 
>>>>>> options=, main=, suspend=true, quote=", vmexec=java)
>>>>>
>>>>> I've retested the patch on the same platforms as before and now it 
>>>>> passes on windows as well.
>>>>>
>>>>> incremental webrev: 
>>>>> http://cr.openjdk.java.net/~iignatyev//8242471/webrev.0-1/index.html
>>>>> full webrev: 
>>>>> http://cr.openjdk.java.net/~iignatyev//8242471/webrev.01/index.html
>>>>>
>>>>> Thanks,
>>>>> -- Igor
>>>>>
>>>>>> On Apr 9, 2020, at 3:15 PM, Chris Plummer 
>>>>>> <chris.plummer at oracle.com <mailto:chris.plummer at oracle.com>> wrote:
>>>>>>
>>>>>> Hi Igor,
>>>>>>
>>>>>>   89             log.display("Connector's transport is: " + 
>>>>>> c.transport().name());
>>>>>>
>>>>>> Would be nice if you printed each transport's name (even the 
>>>>>> skipped ones). That way when reading the log after getting 
>>>>>> SkippedException you can see which transports were attempted.
>>>>>>
>>>>>> You removed the following:
>>>>>>
>>>>>>   50  * ================================================
>>>>>>   51  * WARNING:
>>>>>>   52  *         Temporarily the test is prepared only for
>>>>>>   53  * Sparc.Solaris.dt_socket-transport of RawCommandLineLaunch
>>>>>>   54  *         connector
>>>>>>   55  * ================================================
>>>>>>
>>>>>> Isn't this still somewhat true? It's still requires dt_socket, but 
>>>>>> no longer solaris-sparc. This is part of the reason I asked above 
>>>>>> to print which transports are attempted. I'm curious what the 
>>>>>> skipped transport is on Windows.
>>>>>>
>>>>>> thanks,
>>>>>>
>>>>>> Chris
>>>>>>
>>>>>> On 4/9/20 2:43 PM, Igor Ignatyev wrote:
>>>>>>> http://cr.openjdk.java.net/~iignatyev/8242471/webrev.00/
>>>>>>>> 38 lines changed: 8 ins; 23 del; 7 mod;
>>>>>>> Hi all,
>>>>>>>
>>>>>>> could you please review this small patch for 
>>>>>>> nsk/jdi/Argument/value/value004 test?
>>>>>>> from JBS:
>>>>>>>> nsk/jdi/Argument/value/value004 test has restrictions to be run 
>>>>>>>> only on solaris-sparc and w/ dt_socket transport, solaris-sparc 
>>>>>>>> restriction seems to be not valid any more. the test also should 
>>>>>>>> be updated to use skipped exception if there are no suitable 
>>>>>>>> connectors available.
>>>>>>>
>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8242471
>>>>>>> testing: the changed test on {linux,windows,macosx}-x64 and 
>>>>>>> solaris-sparcv9, w/ the test being skipped on windows-x64
>>>>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8242471/webrev.00/
>>>>>>>
>>>>>>> Thanks,
>>>>>>> -- Igor
>>>>>>
>>>>>>
>>>>>
>>>>
>>
>