Thanks Alex, pushed. -- Igor
> On Apr 13, 2020, at 11:30 AM, Alex Menkov <[email protected]> wrote: > > 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 <[email protected] >>> <mailto:[email protected]>> 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 <[email protected]> >>>>> 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 <[email protected] >>>>>>> <mailto:[email protected]>> 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 >>>>>>> >>>>>>> >>>>>> >>>>> >>>
