[GitHub] [struts] JCgH4164838Gh792C124B5 commented on a change in pull request #457: Fix json validation interceptor test

[GitBox <gi...@apache.org>]

JCgH4164838Gh792C124B5 commented on a change in pull request #457:
URL: https://github.com/apache/struts/pull/457#discussion_r546279045



##########
File path: plugins/json/src/test/java/org/apache/struts2/json/JSONValidationInterceptorTest.java
##########
@@ -66,8 +67,23 @@ public void testValidationFails() throws Exception {
         String json = stringWriter.toString();
 
         String normalizedActual = TestUtils.normalize(json, true);
-
         //json
+        // fix the order of "Tooshort", and "Thisisnoemail"

Review comment:
       Instead of using custom code to force the JSON string to have the order the test currently expects, I would like to suggest an alternative using the AssertJ API capabilities. 
   
   Add the following two imports:
   ```
   import static org.assertj.core.api.Assertions.anyOf;
   import org.assertj.core.api.Condition;
   ```
   
   Replace the custom ordering code with this block of code (or something similar) between the //json and //execution comments:
   ```
           //json
           Condition<String> textValidationOrder1 = new Condition<>(s -> s.contains("\"text\":[\"Tooshort\",\"Thisisnoemail\"]"), "textValidationOrder1");
           Condition<String> textValidationOrder2 = new Condition<>(s -> s.contains("\"text\":[\"Thisisnoemail\",\"Tooshort\"]"), "textValidationOrder2");
           assertThat(normalizedActual)
                   .contains("\"errors\":[\"Generalerror\"]")
                   .contains("\"fieldErrors\":{")
                   .contains("\"value\":[\"Minvalueis-1\"]")
                   .contains("\"password\":[\"Passwordisn'tcorrect\"]");
           assertThat(normalizedActual).is(anyOf(textValidationOrder1, textValidationOrder2));
           //execution
   ```
    Others might have different suggestions, or alternative ways to use the AssertJ API to handle the possibility of the order changing for the validation annotations.

##########
File path: plugins/json/src/test/java/org/apache/struts2/json/JSONValidationInterceptorTest.java
##########
@@ -33,6 +33,7 @@
 import org.apache.struts2.interceptor.validation.AnnotationValidationInterceptor;
 import org.apache.struts2.interceptor.validation.SkipValidation;
 import org.apache.struts2.util.TestUtils;
+import org.assertj.core.internal.bytebuddy.asm.Advice;

Review comment:
       Appears to be an unused import (possibly used in an older revision ?)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org