vy commented on code in PR #3263: URL: https://github.com/apache/logging-log4j2/pull/3263#discussion_r1866548995
########## log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryCustomizationTest.java: ########## @@ -24,34 +24,36 @@ import org.apache.logging.log4j.message.MessageFactory; import org.apache.logging.log4j.message.ParameterizedMessageFactory; import org.junit.jupiter.api.Test; -import org.junitpioneer.jupiter.ClearSystemProperty; import org.junitpioneer.jupiter.SetSystemProperty; +@SetSystemProperty( + key = "log4j2.messageFactory", + value = "org.apache.logging.log4j.core.LoggerMessageFactoryCustomizationTest$TestMessageFactory") +@SetSystemProperty( + key = "log4j2.flowMessageFactory", + value = "org.apache.logging.log4j.core.LoggerMessageFactoryCustomizationTest$TestFlowMessageFactory") class LoggerMessageFactoryCustomizationTest { @Test - @ClearSystemProperty(key = "log4j2.messageFactory") - @ClearSystemProperty(key = "log4j2.flowMessageFactory") void arguments_should_be_honored() { - final LoggerContext loggerContext = - new LoggerContext(LoggerMessageFactoryCustomizationTest.class.getSimpleName()); - final Logger logger = new Logger( - loggerContext, "arguments_should_be_honored", new TestMessageFactory(), new TestFlowMessageFactory()); - assertTestMessageFactories(logger); + try (LoggerContext loggerContext = + new LoggerContext(LoggerMessageFactoryCustomizationTest.class.getSimpleName())) { + Logger logger = new Logger( + loggerContext, + "arguments_should_be_honored", Review Comment: You can use `@TestInfo` instead of hardcoding the method name. (Same goes for the other test methods too.) ########## log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryCustomizationTest.java: ########## @@ -24,34 +24,36 @@ import org.apache.logging.log4j.message.MessageFactory; import org.apache.logging.log4j.message.ParameterizedMessageFactory; import org.junit.jupiter.api.Test; -import org.junitpioneer.jupiter.ClearSystemProperty; import org.junitpioneer.jupiter.SetSystemProperty; +@SetSystemProperty( Review Comment: By making this test-scoped, aren't you changing the `arguments_should_be_honored()` test? That is, * Before: there are no property-provided configurations, but only arguments passed to `Logger::new` * New: there are property-provided configurations, and also arguments passed to `Logger::new` In the new case, you cannot know which configuration source has kicked in. ########## log4j-api-test/src/test/java/org/apache/logging/log4j/util/PropertiesUtilTest.java: ########## @@ -208,6 +208,28 @@ void testBadPropertySource() { } } + @Test + @ResourceLock(value = Resources.SYSTEM_PROPERTIES, mode = ResourceAccessMode.READ) + @Issue("https://github.com/apache/logging-log4j2/issues/3252") + void testRecursivePropertySource() { Review Comment: Shall we also verify the status logger warning? -- 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-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org