RFR: fix the bug with failure when multispace is a separator in --jvmargs argument and similar annotations.

[Ivan Gerasimov]

On 21.06.2013 18:12, Aleksey Shipilev wrote:
> I like that suggestion better. Fixed Sergey's patch to use [ ]+, and
> still committed the regression tests. Pushed.
>
> Thanks everyone!
> -Aleksey.
>
> P.S. Ivan, please consider filling out OCA, as per:
>   http://openjdk.java.net/contribute/
I believe I don't need that one as I'm an Oracle employee.

>
> On 06/21/2013 05:28 PM, Ivan Gerasimov wrote:
>> As the argument to .split() is a regexp, wouldn't it be better to use "[
>> ]+" as a delimiter?
>>
>> Sincerely,
>> Ivan
>>
>> On 21.06.2013 17:04, Sergey Kuksenko wrote:
>>> fix the bug with failure when multiply spaces is a separator in
>>> --jvmargs argument and similar annotations.
>>> ----- patch starts here---------------------------
>>> diff -r 9d5036308216
>>> jmh-core-it/src/test/java/org/openjdk/jmh/it/fork/ForkedJvmArgsTest3.java
>>> --- /dev/null    Thu Jan 01 00:00:00 1970 +0000
>>> +++
>>> b/jmh-core-it/src/test/java/org/openjdk/jmh/it/fork/ForkedJvmArgsTest3.java
>>> Fri Jun 21 17:01:58 2013 +0400
>>> @@ -0,0 +1,74 @@
>>> +/**
>>> + * Copyright (c) 2005, 2013, Oracle and/or its affiliates. All rights
>>> reserved.
>>> + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>> + *
>>> + * This code is free software; you can redistribute it and/or modify it
>>> + * under the terms of the GNU General Public License version 2 only, as
>>> + * published by the Free Software Foundation.  Oracle designates this
>>> + * particular file as subject to the "Classpath" exception as provided
>>> + * by Oracle in the LICENSE file that accompanied this code.
>>> + *
>>> + * This code is distributed in the hope that it will be useful, but
>>> WITHOUT
>>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
>>> + * version 2 for more details (a copy is included in the LICENSE file
>>> that
>>> + * accompanied this code).
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> version
>>> + * 2 along with this work; if not, write to the Free Software
>>> Foundation,
>>> + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
>>> + *
>>> + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA
>>> 94065 USA
>>> + * or visit www.oracle.com if you need additional information or have
>>> any
>>> + * questions.
>>> + */
>>> +package org.openjdk.jmh.it.fork;
>>> +
>>> +import org.junit.Assert;
>>> +import org.junit.Test;
>>> +import org.openjdk.jmh.Main;
>>> +import org.openjdk.jmh.annotations.BenchmarkMode;
>>> +import org.openjdk.jmh.annotations.Fork;
>>> +import org.openjdk.jmh.annotations.GenerateMicroBenchmark;
>>> +import org.openjdk.jmh.annotations.Measurement;
>>> +import org.openjdk.jmh.annotations.Mode;
>>> +import org.openjdk.jmh.annotations.Warmup;
>>> +import org.openjdk.jmh.it.Fixtures;
>>> +
>>> +import java.util.concurrent.TimeUnit;
>>> +
>>> +/**
>>> + * tests multiply spaces in jvmArgs annotations.
>>> + * @author Sergey Kuksenko (sergey.kuksenko at oracle.com)
>>> + */
>>> + at BenchmarkMode(Mode.All)
>>> +public class ForkedJvmArgsTest3 {
>>> +
>>> +    @GenerateMicroBenchmark
>>> +    @Warmup(iterations = 0)
>>> +    @Measurement(iterations = 1, time = 100, timeUnit =
>>> TimeUnit.MILLISECONDS)
>>> +    @Fork(jvmArgs = "-Dtest1  -Dtest3")
>>> +    public void test1() {
>>> +        Fixtures.work();
>>> +        Assert.assertNotNull(System.getProperty("test1"));
>>> +        Assert.assertNull(System.getProperty("test2"));
>>> +        Assert.assertNotNull(System.getProperty("test3"));
>>> +    }
>>> +
>>> +    @GenerateMicroBenchmark
>>> +    @Warmup(iterations = 0)
>>> +    @Measurement(iterations = 1, time = 100, timeUnit =
>>> TimeUnit.MILLISECONDS)
>>> +    @Fork(jvmArgs = "-Dtest2  -Dtest3")
>>> +    public void test2() {
>>> +        Fixtures.work();
>>> +        Assert.assertNull(System.getProperty("test1"));
>>> +        Assert.assertNotNull(System.getProperty("test2"));
>>> +        Assert.assertNotNull(System.getProperty("test3"));
>>> +    }
>>> +
>>> +    @Test
>>> +    public void invoke() {
>>> +        Main.testMain(Fixtures.getTestMask(this.getClass()) + " -foe");
>>> +    }
>>> +
>>> +}
>>> diff -r 9d5036308216
>>> jmh-core/src/main/java/org/openjdk/jmh/runner/options/BaseOptions.java
>>> ---
>>> a/jmh-core/src/main/java/org/openjdk/jmh/runner/options/BaseOptions.java
>>> Fri Jun 14 22:43:17 2013 +0400
>>> +++
>>> b/jmh-core/src/main/java/org/openjdk/jmh/runner/options/BaseOptions.java
>>> Fri Jun 21 17:01:58 2013 +0400
>>> @@ -138,7 +138,9 @@
>>>               String opImage = fieldToCommandLineImage(f);
>>>               if (opImage != null && !opImage.isEmpty()) {
>>>                   for (String s : opImage.split(" ")) {
>>> -                    sb.add(s);
>>> +                    if(s!=null && !s.isEmpty()) {
>>> +                        sb.add(s);
>>> +                    }
>>>                   }
>>>               }
>>>           }
>>> diff -r 9d5036308216
>>> jmh-core/src/main/java/org/openjdk/jmh/runner/options/HarnessOptions.java
>>> ---
>>> a/jmh-core/src/main/java/org/openjdk/jmh/runner/options/HarnessOptions.java
>>> Fri Jun 14 22:43:17 2013 +0400
>>> +++
>>> b/jmh-core/src/main/java/org/openjdk/jmh/runner/options/HarnessOptions.java
>>> Fri Jun 21 17:01:58 2013 +0400
>>> @@ -33,6 +33,7 @@
>>>   import org.openjdk.jmh.runner.BenchmarkRecord;
>>>   import org.openjdk.jmh.runner.CompilerHints;
>>>   import org.openjdk.jmh.runner.options.handlers.ForkOptionHandler;
>>> +import org.openjdk.jmh.util.internal.CollectionUtils;
>>>
>>>   import java.io.File;
>>>   import java.lang.management.ManagementFactory;
>>> @@ -192,9 +193,9 @@
>>>           }
>>>
>>>           if (getJvmArgs() != null) { // use supplied jvm args if given
>>> in cmd line
>>> -            command.addAll(Arrays.asList(getJvmArgs().split(" ")));
>>> +            CollectionUtils.addAllNotEmpty(command,
>>> Arrays.asList(getJvmArgs().split(" ")));
>>>           } else if (annJvmArgs != null) { // use jvm args supplied in
>>> annotation which shuns implicit args
>>> -            command.addAll(Arrays.asList(annJvmArgs.split(" ")));
>>> +            CollectionUtils.addAllNotEmpty(command,
>>> Arrays.asList(annJvmArgs.split(" ")));
>>>           } else {
>>>               // else use same jvm args given to this runner
>>>               RuntimeMXBean RuntimemxBean =
>>> ManagementFactory.getRuntimeMXBean();
>>> diff -r 9d5036308216
>>> jmh-core/src/main/java/org/openjdk/jmh/util/internal/CollectionUtils.java
>>> ---
>>> a/jmh-core/src/main/java/org/openjdk/jmh/util/internal/CollectionUtils.java
>>> Fri Jun 14 22:43:17 2013 +0400
>>> +++
>>> b/jmh-core/src/main/java/org/openjdk/jmh/util/internal/CollectionUtils.java
>>> Fri Jun 21 17:01:58 2013 +0400
>>> @@ -26,6 +26,7 @@
>>>
>>>   import org.openjdk.jmh.util.AnnotationUtils;
>>>
>>> +import java.util.Collection;
>>>   import java.util.HashMap;
>>>   import java.util.List;
>>>   import java.util.Map;
>>> @@ -103,4 +104,13 @@
>>>           }
>>>           return list;
>>>       }
>>> +
>>> +    public static List<String> addAllNotEmpty(List<String> list,
>>> Collection<String> toAdd) {
>>> +        for(String s : toAdd) {
>>> +            if (s != null && !s.isEmpty()) {
>>> +                list.add(s);
>>> +            }
>>> +        }
>>> +        return list;
>>> +    }
>>>   }
>>>
>>>
>
>