RFR 8028141 intermittent test failure of LocalManagementTest

[roger riggs]

Hi,

The new utilities are useful though not directly for this fix because 
the code
needs to append to the classpath, not just the command arguments.

Minor comments on the new utilities, for future consideration.
  - In the getCommandLine impl; java.util.StringJoiner may be useful 
though not much shorter.
  - A bit more esoteric but using the latest lambda/streams, the method 
could be implemented as:

     public static String getCommandLine(ProcessBuilder pb) {

         return pb.command().stream().collect(Collectors.joining(" ")).toString();

     }

  - getTestOpts:93, 105 could supply a String[opts.size()] so the string 
array does not needs to be reallocated.

  - The mapping between lists and arrays seems awkward here and in 
ProcessTools.
    Some cleanup might be able to treat that more consistently and be 
easier to read.

Thanks, Roger


On 11/19/2013 3:05 AM, taras ledkov wrote:
> Dis you see the review: 
> http://cr.openjdk.java.net/~ykantser/8023138/webrev.00/test/lib/testlibrary/jdk/testlibrary/Utils.java.sdiff.html
> (e.g. please take a look at the Utils.addTestJavaOpts)
>
> What do you think about common solution for test properties inheritance
> (for ProcessTools utils) because it is the frequent pattern.
>
> On 19.11.2013 1:59, roger riggs wrote:
>> Please review this fix to improve the reliability of the
>> jmxRemote/LocalManagementTest and CustomLauncherTest. The solution may
>> apply to other tests that fail with ClassNotFound.
>>
>> The tests did not include the jdk.testlibrary in the classpath when it
>> spawns a new process. The failure mode is dependent on the order that
>> tests had been run.
>>
>> Also note that if a test depends on classes in the jdk.testlibrary it
>> should include @build <classname> to ensure they are built in the
>> @library directory
>>
>> Webrev:
>> http://cr.openjdk.java.net/~rriggs/webrev-localmanagement-8028141/
>>
>> Thanks, Roger
>>
>>
>