Fixes #1209 soft assertions did not handle correctly formatting error…

… messages with double %
assertj · Mar 30, 2018 · 3488096 · 3488096
1 parent e279dc0
commit 3488096
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 21 deletions.
diff --git a/src/main/java/org/assertj/core/error/ShouldBeEqualByComparingFieldByFieldRecursively.java b/src/main/java/org/assertj/core/error/ShouldBeEqualByComparingFieldByFieldRecursively.java
@@ -13,6 +13,7 @@
 package org.assertj.core.error;
 
 import static java.lang.String.format;
+import static org.assertj.core.util.Strings.escapePercent;
 import static org.assertj.core.util.Strings.join;
 
 import java.util.ArrayList;
@@ -71,8 +72,4 @@ private static String describeDifference(Difference difference, Representation r
                   escapePercent(otherFieldValueRepresentation));
   }
 
-  protected static String escapePercent(String value) {
-    return value == null ? null : value.replace("%", "%%");
-  }
-
 }
diff --git a/src/main/java/org/assertj/core/util/Strings.java b/src/main/java/org/assertj/core/util/Strings.java
@@ -85,11 +85,20 @@ public static String concat(Object... objects) {
    */
   public static String formatIfArgs(String message, Object... args) {
     return Arrays.isNullOrEmpty(args)
-            // here we need to format %n but not other % since we do not have arguments. %% is formatted to % so
-            // replacing % into %% and then format it will have no effect. Nevertheless, we want to format %n
-            // correctly so we replace all % to %% except if they are followed by a 'n'.
-            ? format(message.replaceAll("%([^n])","%%$1"))
-            : format (message, args);
+        // here we need to format %n but not other % since we do not have arguments.
+        // => we replace all % to %% except if they are followed by a 'n'.
+        ? format(escapePercentExceptWhenFollowedBy_n(message))
+        : format(message, args);
+  }
+
+  /**
+   * Escape any {@code %} to {@code %%} to avoid interpreting it in {@link String#format(String, Object...)}.
+   *  
+   * @param value the String to escape
+   * @return the escaped String 
+   */
+  public static String escapePercent(String value) {
+    return value == null ? null : value.replace("%", "%%");
   }
 
   /**
@@ -227,6 +236,15 @@ public String to(String s) {
     }
   }
 
-  private Strings() {
+  // change %%n back to %n which could have been done by calling escapePercent
+  private static String escapePercentExceptWhenFollowedBy_n(String message) {
+    return revertEscapingPercent_n(escapePercent(message));
   }
+
+  private static String revertEscapingPercent_n(String value) {
+    return value == null ? null : value.replace("%%n", "%n");
+  }
+
+  private Strings() {}
+
 }
diff --git a/src/test/java/org/assertj/core/api/SoftAssertionsTest.java b/src/test/java/org/assertj/core/api/SoftAssertionsTest.java
@@ -1608,4 +1608,19 @@ public void soft_assertions_should_work_with_zipSatisfy() {
     assertThat(errorsCollected.get(0)).hasMessageContaining("John");
   }
 
+  @Test
+  public void bug_1209() {
+    // GIVEN
+    String string = "%%E";
+    // WHEN
+    softly.assertThat(string).matches("fffff");
+    // THEN
+    // THEN
+    List<Throwable> errorsCollected = softly.errorsCollected();
+    assertThat(errorsCollected).hasSize(1);
+    assertThat(errorsCollected.get(0)).hasMessageContaining("%%E")
+                                      .hasMessageContaining("to match pattern")
+                                      .hasMessageContaining("fffff");
+  }
+
 }
diff --git a/src/test/java/org/assertj/core/error/MessageFormatter_format_Test.java b/src/test/java/org/assertj/core/error/MessageFormatter_format_Test.java
@@ -12,22 +12,33 @@
  */
 package org.assertj.core.error;
 
-import static org.assertj.core.test.ExpectedException.none;
+import static com.tngtech.junit.dataprovider.DataProviders.$;
+import static com.tngtech.junit.dataprovider.DataProviders.$$;
+import static java.lang.String.format;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.*;
-
+import static org.assertj.core.presentation.StandardRepresentation.STANDARD_REPRESENTATION;
+import static org.assertj.core.test.ExpectedException.none;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
 
-import org.assertj.core.description.*;
-import org.assertj.core.presentation.Representation;
-import org.assertj.core.presentation.StandardRepresentation;
+import org.assertj.core.description.Description;
+import org.assertj.core.description.TextDescription;
 import org.assertj.core.test.ExpectedException;
-import org.junit.*;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import com.tngtech.java.junit.dataprovider.DataProvider;
+import com.tngtech.java.junit.dataprovider.DataProviderRunner;
+import com.tngtech.java.junit.dataprovider.UseDataProvider;
 
 /**
  * Tests for <code>{@link MessageFormatter#format(Description, String, Object...)}</code>.
  * 
  * @author Alex Ruiz
  */
+@RunWith(DataProviderRunner.class)
 public class MessageFormatter_format_Test {
 
   @Rule
@@ -59,9 +70,29 @@ public void should_throw_error_if_args_array_is_null() {
   @Test
   public void should_format_message() {
     Description description = new TextDescription("Test");
-    Representation representation = new StandardRepresentation();
-    String s = messageFormatter.format(description, representation, "Hello %s", "World");
+    String s = messageFormatter.format(description, STANDARD_REPRESENTATION, "Hello %s", "World");
     assertThat(s).isEqualTo("[Test] Hello \"World\"");
     verify(descriptionFormatter).format(description);
   }
+
+  @Test
+  @UseDataProvider("messages")
+  public void should_format_message_and_correctly_escape_percentage(String input, String formatted) {
+    // GIVEN
+    Description description = new TextDescription("Test");
+    // WHEN
+    String finalMessage = messageFormatter.format(description, STANDARD_REPRESENTATION, input);
+    // THEN
+    assertThat(finalMessage).isEqualTo("[Test] " + formatted);
+  }
+
+  @DataProvider
+  public static Object[][] messages() {
+    return $$($("%E", "%E"),
+              $("%%E", "%%E"),
+              $("%%%E", "%%%E"),
+              $("%n", format("%n")),
+              $("%%%n%E", "%%" + format("%n") + "%E"),
+              $("%%n", "%" + format("%n")));
+  }
 }
diff --git a/src/test/java/org/assertj/core/error/ShouldMatchPattern_create_Test.java b/src/test/java/org/assertj/core/error/ShouldMatchPattern_create_Test.java
@@ -15,9 +15,10 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.error.ShouldMatchPattern.shouldMatch;
 
-import org.assertj.core.description.*;
+import org.assertj.core.description.TextDescription;
 import org.assertj.core.presentation.StandardRepresentation;
-import org.junit.*;
+import org.junit.Before;
+import org.junit.Test;
 
 /**
  * Tests for <code>{@link ShouldMatchPattern#create(org.assertj.core.description.Description, org.assertj.core.presentation.Representation)}</code>.
@@ -38,4 +39,11 @@ public void should_create_error_message() {
     String message = factory.create(new TextDescription("Test"), new StandardRepresentation());
     assertThat(message).isEqualTo(String.format("[Test] %nExpecting:%n \"Yoda\"%nto match pattern:%n \"Luke\""));
   }
+
+  @Test
+  public void should_create_error_message_escaping_percent() {
+    factory = shouldMatch("%%E", "fffff");
+    String message = factory.create(new TextDescription("Test"), new StandardRepresentation());
+    assertThat(message).isEqualTo(String.format("[Test] %nExpecting:%n \"%%%%E\"%nto match pattern:%n \"fffff\""));
+  }
 }