sure. @alias, can I get a 2nd review for this patch?
Thanks, -- Igor > On Apr 9, 2020, at 9:45 PM, Chris Plummer <chris.plum...@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.plum...@oracle.com> >>> <mailto:chris.plum...@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 >>>> <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 >>>> <http://cr.openjdk.java.net/~iignatyev//8242471/webrev.01/index.html> >>>> >>>> Thanks, >>>> -- Igor >>>> >>>>> On Apr 9, 2020, at 3:15 PM, Chris Plummer <chris.plum...@oracle.com >>>>> <mailto:chris.plum...@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/ >>>>>> <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 >>>>>> <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/ >>>>>> <http://cr.openjdk.java.net/~iignatyev/8242471/webrev.00/> >>>>>> >>>>>> Thanks, >>>>>> -- Igor >>>>> >>>>> >>>> >>> >
