[GitHub] jmeter pull request #342: More edge cases for changeCase function and slight...

2017-12-09 Thread asfgit
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...

2017-11-25 Thread ham1
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...

2017-11-25 Thread ham1
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...

2017-11-25 Thread FSchumacher
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...

2017-11-25 Thread FSchumacher
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...

2017-11-25 Thread ham1
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...

2017-11-25 Thread ham1
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...

2017-11-25 Thread FSchumacher
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.




---