You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by "ppkarwasz (via GitHub)" <gi...@apache.org> on 2023/08/01 04:35:26 UTC

Re: [PR] `ParameterFormatter` rewrite with corrected escape handling (logging-log4j2)

ppkarwasz commented on code in PR #1639:
URL: https://github.com/apache/logging-log4j2/pull/1639#discussion_r1280099113


##########
log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java:
##########
@@ -17,156 +17,167 @@
 package org.apache.logging.log4j.message;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.MethodSource;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.assertj.core.api.Assertions.assertThat;
 
 /**
  * Tests {@link ParameterFormatter}.
  */
 public class ParameterFormatterTest {
 
-    @Test
-    public void testCountArgumentPlaceholders() {
-        assertEquals(0, ParameterFormatter.countArgumentPlaceholders(""));
-        assertEquals(0, ParameterFormatter.countArgumentPlaceholders("aaa"));
-        assertEquals(0, ParameterFormatter.countArgumentPlaceholders("\\{}"));
-        assertEquals(1, ParameterFormatter.countArgumentPlaceholders("{}"));
-        assertEquals(1, ParameterFormatter.countArgumentPlaceholders("{}\\{}"));
-        assertEquals(2, ParameterFormatter.countArgumentPlaceholders("{}{}"));
-        assertEquals(3, ParameterFormatter.countArgumentPlaceholders("{}{}{}"));
-        assertEquals(4, ParameterFormatter.countArgumentPlaceholders("{}{}{}aa{}"));
-        assertEquals(4, ParameterFormatter.countArgumentPlaceholders("{}{}{}a{]b{}"));
-        assertEquals(5, ParameterFormatter.countArgumentPlaceholders("{}{}{}a{}b{}"));
-    }
-
-    @Test
-    public void testFormat3StringArgs() {
-        final String testMsg = "Test message {}{} {}";
-        final String[] args = { "a", "b", "c" };
-        final String result = ParameterFormatter.format(testMsg, args);
-        assertEquals("Test message ab c", result);
-    }
-
-    @Test
-    public void testFormatNullArgs() {
-        final String testMsg = "Test message {} {} {} {} {} {}";
-        final String[] args = { "a", null, "c", null, null, null };
-        final String result = ParameterFormatter.format(testMsg, args);
-        assertEquals("Test message a null c null null null", result);
-    }
-
-    @Test
-    public void testFormatStringArgsIgnoresSuperfluousArgs() {
-        final String testMsg = "Test message {}{} {}";
-        final String[] args = { "a", "b", "c", "unnecessary", "superfluous" };
-        final String result = ParameterFormatter.format(testMsg, args);
-        assertEquals("Test message ab c", result);
-    }
-
-    @Test
-    public void testFormatStringArgsWithEscape() {
-        final String testMsg = "Test message \\{}{} {}";
-        final String[] args = { "a", "b", "c" };
-        final String result = ParameterFormatter.format(testMsg, args);
-        assertEquals("Test message {}a b", result);
-    }
-
-    @Test
-    public void testFormatStringArgsWithTrailingEscape() {
-        final String testMsg = "Test message {}{} {}\\";
-        final String[] args = { "a", "b", "c" };
-        final String result = ParameterFormatter.format(testMsg, args);
-        assertEquals("Test message ab c\\", result);
-    }
-
-    @Test
-    public void testFormatStringArgsWithTrailingEscapedEscape() {
-        final String testMsg = "Test message {}{} {}\\\\";
-        final String[] args = { "a", "b", "c" };
-        final String result = ParameterFormatter.format(testMsg, args);
-        assertEquals("Test message ab c\\\\", result);
-    }
-
-    @Test
-    public void testFormatStringArgsWithEscapedEscape() {
-        final String testMsg = "Test message \\\\{}{} {}";
-        final String[] args = { "a", "b", "c" };
-        final String result = ParameterFormatter.format(testMsg, args);
-        assertEquals("Test message \\ab c", result);
-    }
-
-    @Test
-    public void testFormatMessage3StringArgs() {
-        final String testMsg = "Test message {}{} {}";
-        final String[] args = { "a", "b", "c" };
-        final StringBuilder sb = new StringBuilder();
-        ParameterFormatter.formatMessage(sb, testMsg, args, 3);
-        final String result = sb.toString();
-        assertEquals("Test message ab c", result);
-    }
-
-    @Test
-    public void testFormatMessageNullArgs() {
-        final String testMsg = "Test message {} {} {} {} {} {}";
-        final String[] args = { "a", null, "c", null, null, null };
-        final StringBuilder sb = new StringBuilder();
-        ParameterFormatter.formatMessage(sb, testMsg, args, 6);
-        final String result = sb.toString();
-        assertEquals("Test message a null c null null null", result);
-    }
-
-    @Test
-    public void testFormatMessageStringArgsIgnoresSuperfluousArgs() {
-        final String testMsg = "Test message {}{} {}";
-        final String[] args = { "a", "b", "c", "unnecessary", "superfluous" };
-        final StringBuilder sb = new StringBuilder();
-        ParameterFormatter.formatMessage(sb, testMsg, args, 5);
-        final String result = sb.toString();
-        assertEquals("Test message ab c", result);
-    }
-
-    @Test
-    public void testFormatMessageStringArgsWithEscape() {
-        final String testMsg = "Test message \\{}{} {}";
-        final String[] args = { "a", "b", "c" };
-        final StringBuilder sb = new StringBuilder();
-        ParameterFormatter.formatMessage(sb, testMsg, args, 3);
-        final String result = sb.toString();
-        assertEquals("Test message {}a b", result);
-    }
-
-    @Test
-    public void testFormatMessageStringArgsWithTrailingEscape() {
-        final String testMsg = "Test message {}{} {}\\";
-        final String[] args = { "a", "b", "c" };
-        final StringBuilder sb = new StringBuilder();
-        ParameterFormatter.formatMessage(sb, testMsg, args, 3);
-        final String result = sb.toString();
-        assertEquals("Test message ab c\\", result);
-    }
-
-    @Test
-    public void testFormatMessageStringArgsWithTrailingEscapedEscape() {
-        final String testMsg = "Test message {}{} {}\\\\";
-        final String[] args = { "a", "b", "c" };
-        final StringBuilder sb = new StringBuilder();
-        ParameterFormatter.formatMessage(sb, testMsg, args, 3);
-        final String result = sb.toString();
-        assertEquals("Test message ab c\\\\", result);
-    }
-
-    @Test
-    public void testFormatMessageStringArgsWithEscapedEscape() {
-        final String testMsg = "Test message \\\\{}{} {}";
-        final String[] args = { "a", "b", "c" };
-        final StringBuilder sb = new StringBuilder();
-        ParameterFormatter.formatMessage(sb, testMsg, args, 3);
-        final String result = sb.toString();
-        assertEquals("Test message \\ab c", result);
+    @ParameterizedTest
+    @CsvSource({
+            "0,,false,",
+            "0,,false,aaa",
+            "0,,true,\\{}",
+            "1,0,false,{}",
+            "1,0,true,{}\\{}",
+            "1,2,true,\\\\{}",
+            "2,8:10,true,foo \\{} {}{}",

Review Comment:
   Maybe we could also test `{\\}` as an alternative to `\\{}`.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

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