[GitHub] jmeter pull request #342: More edge cases for changeCase function and slight...
Github user asfgit closed the pull request at: https://github.com/apache/jmeter/pull/342 ---
[GitHub] jmeter pull request #342: More edge cases for changeCase function and slight...
Github user ham1 commented on a diff in the pull request: https://github.com/apache/jmeter/pull/342#discussion_r153054513 --- Diff: src/functions/org/apache/jmeter/functions/ChangeCase.java --- @@ -127,19 +129,14 @@ public String getReferenceKey() { return DESC; } -private static String camel(String str, boolean isFirstCapitalized) { -StringBuilder builder = new StringBuilder(str.length()); -String[] tokens = NOT_ALPHANUMERIC_REGEX.split(str); -for (int i = 0; i < tokens.length; i++) { -String lowerCased = StringUtils.lowerCase(tokens[i]); -if(i == 0) { -builder.append(isFirstCapitalized ? StringUtils.capitalize(lowerCased): -lowerCased); -} else { -builder.append(StringUtils.capitalize(lowerCased)); -} +private static String camel(String input, boolean uncapitalizeFirst) { +List tokens = Arrays.asList(SPLIT_CHARS.split(input)).stream() +.filter(s -> !s.isEmpty()).map(StringUtils::lowerCase).map(StringUtils::capitalize) --- End diff -- I generally prefer each item on a new line - I had to read this line twice as I missed the map in the middle on first pass, whereas I think: ```java .filter(StringUtils::isNotEmpty) .map(StringUtils::lowerCase) .map(StringUtils::capitalize) ``` is just that bit easier to read and doesn't get in the way of comprehending the code. ---
[GitHub] jmeter pull request #342: More edge cases for changeCase function and slight...
Github user ham1 commented on a diff in the pull request: https://github.com/apache/jmeter/pull/342#discussion_r153049075 --- Diff: test/src/org/apache/jmeter/functions/TestChangeCaseExamples.java --- @@ -0,0 +1,117 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.jmeter.functions; + +import static org.junit.Assert.*; + +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.stream.Collectors; + +import org.apache.jmeter.engine.util.CompoundVariable; +import org.apache.jmeter.functions.ChangeCase.ChangeCaseMode; +import org.apache.jmeter.junit.JMeterTestCase; +import org.apache.jmeter.samplers.SampleResult; +import org.apache.jmeter.threads.JMeterContext; +import org.apache.jmeter.threads.JMeterContextService; +import org.apache.jmeter.threads.JMeterVariables; +import org.hamcrest.CoreMatchers; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +@RunWith(Parameterized.class) +public class TestChangeCaseExamples extends JMeterTestCase { + +@Parameters +public static Collection data() { +return Arrays.asList(new Object[][] { { " spaces before and after ", +new String[] { " SPACES BEFORE AND AFTER ", --- End diff -- I agree. See the PR for an example in `HTTPUtilsSpec` (and smaller examples in a few of the other tests). ---
[GitHub] jmeter pull request #342: More edge cases for changeCase function and slight...
Github user FSchumacher commented on a diff in the pull request: https://github.com/apache/jmeter/pull/342#discussion_r153048358 --- Diff: test/src/org/apache/jmeter/functions/TestChangeCaseExamples.java --- @@ -0,0 +1,117 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.jmeter.functions; + +import static org.junit.Assert.*; + +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.stream.Collectors; + +import org.apache.jmeter.engine.util.CompoundVariable; +import org.apache.jmeter.functions.ChangeCase.ChangeCaseMode; +import org.apache.jmeter.junit.JMeterTestCase; +import org.apache.jmeter.samplers.SampleResult; +import org.apache.jmeter.threads.JMeterContext; +import org.apache.jmeter.threads.JMeterContextService; +import org.apache.jmeter.threads.JMeterVariables; +import org.hamcrest.CoreMatchers; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +@RunWith(Parameterized.class) +public class TestChangeCaseExamples extends JMeterTestCase { + +@Parameters +public static Collection data() { +return Arrays.asList(new Object[][] { { " spaces before and after ", +new String[] { " SPACES BEFORE AND AFTER ", --- End diff -- This would be a perfect showcase for spock, don't you think? ---
[GitHub] jmeter pull request #342: More edge cases for changeCase function and slight...
Github user FSchumacher commented on a diff in the pull request: https://github.com/apache/jmeter/pull/342#discussion_r153048353 --- Diff: test/src/org/apache/jmeter/functions/TestChangeCaseExamples.java --- @@ -0,0 +1,117 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.jmeter.functions; + +import static org.junit.Assert.*; --- End diff -- Right, but we don't enforce it yet and there are more in our test code base. I will have a look at my eclipse settings. ---
[GitHub] jmeter pull request #342: More edge cases for changeCase function and slight...
Github user ham1 commented on a diff in the pull request: https://github.com/apache/jmeter/pull/342#discussion_r153048260 --- Diff: test/src/org/apache/jmeter/functions/TestChangeCaseExamples.java --- @@ -0,0 +1,117 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.jmeter.functions; + +import static org.junit.Assert.*; + +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.stream.Collectors; + +import org.apache.jmeter.engine.util.CompoundVariable; +import org.apache.jmeter.functions.ChangeCase.ChangeCaseMode; +import org.apache.jmeter.junit.JMeterTestCase; +import org.apache.jmeter.samplers.SampleResult; +import org.apache.jmeter.threads.JMeterContext; +import org.apache.jmeter.threads.JMeterContextService; +import org.apache.jmeter.threads.JMeterVariables; +import org.hamcrest.CoreMatchers; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +@RunWith(Parameterized.class) +public class TestChangeCaseExamples extends JMeterTestCase { + +@Parameters +public static Collection data() { +return Arrays.asList(new Object[][] { { " spaces before and after ", +new String[] { " SPACES BEFORE AND AFTER ", --- End diff -- One per line or all on one line would make this easier to read, also a comment regarding which mode corresponds to which position would be helpful for future reference (if/when it changes). ---
[GitHub] jmeter pull request #342: More edge cases for changeCase function and slight...
Github user ham1 commented on a diff in the pull request: https://github.com/apache/jmeter/pull/342#discussion_r153048235 --- Diff: test/src/org/apache/jmeter/functions/TestChangeCaseExamples.java --- @@ -0,0 +1,117 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.jmeter.functions; + +import static org.junit.Assert.*; --- End diff -- No `*` imports :) ---
[GitHub] jmeter pull request #342: More edge cases for changeCase function and slight...
GitHub user FSchumacher opened a pull request: https://github.com/apache/jmeter/pull/342 More edge cases for changeCase function and slight behaviour changes. ## Description Addition of more examples to test with the different change case modes. We now really split on all non alpha numeric characters - even unicode ones. The input strings for the camel case modes get trimmed before further actions. A further question is, whether we should trim the input in the other modes as well? For example "` space in front`" + `capitalize` = "`Space in front`" or "` space in front`"? ## Motivation and Context The handling of whitespace and other non alphanumeric characters was not well defined. ## How Has This Been Tested? unit tests are added and run ## Screenshots (if appropriate): ## Types of changes - Bug fix (non-breaking change which fixes an issue) ## Checklist: - [x] My code follows the [code style][style-guide] of this project. - [x] I have updated the documentation accordingly. [style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines You can merge this pull request into a Git repository by running: $ git pull https://github.com/FSchumacher/jmeter camel-case-examples Alternatively you can review and apply these changes as the patch at: https://github.com/apache/jmeter/pull/342.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #342 commit 3d576bfca9eb2503f7ba04c083f7e3d9167751b9 Author: Felix Schumacher Date: 2017-11-25T13:47:53Z More edge cases for changeCase function and slight behaviour changes. Addition of more examples to test with the different change case modes. We now really split on all non alpha numeric characters - even unicode ones. The input strings for the camel case modes get trimmed before further actions. ---