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