You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2022/01/16 23:46:41 UTC
[GitHub] [logging-log4j2] ppkarwasz opened a new pull request #706: Common unit tests for Log4j 1.x configuration factories
ppkarwasz opened a new pull request #706:
URL: https://github.com/apache/logging-log4j2/pull/706
`Log4j1ConfigurationFactoryTest` contains many interesting tests that fail if we apply them to the `PropertiesConfigurationFactory` and `XmlConfigurationFactory` that replaced `Log4j1ConfigurationFactory`.
This PR adapts those tests to the other two factories and provides 11 test XML configuration files. It was split from #704 for simpler analysis. Obviously many of the new tests **fail**.
--
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
[GitHub] [logging-log4j2] garydgregory edited a comment on pull request #706: Common unit tests for Log4j 1.x configuration factories
Posted by GitBox <gi...@apache.org>.
garydgregory edited a comment on pull request #706:
URL: https://github.com/apache/logging-log4j2/pull/706#issuecomment-1014655999
Hi @ppkarwasz
Failing a build is not going to be acceptable IMO. Because different team members work at different times and availability, like me, I don't want to bring in code that breaks the build, I might have to step away and work on something else, then we would be left with a failing project build. Each PR should pass the build. If a PR fixes a bug, it should come with a test that fails without the main change.
TY.
--
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
[GitHub] [logging-log4j2] garydgregory commented on a change in pull request #706: Common unit tests for Log4j 1.x configuration factories
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #706:
URL: https://github.com/apache/logging-log4j2/pull/706#discussion_r786004843
##########
File path: log4j-1.2-api/src/test/java/org/apache/log4j/config/AbstractLog4j1ConfigurationTest.java
##########
@@ -0,0 +1,264 @@
+/*
+ * 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.log4j.config;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.URISyntaxException;
+import java.nio.file.FileSystemException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.log4j.layout.Log4j1XmlLayout;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.core.Appender;
+import org.apache.logging.log4j.core.Layout;
+import org.apache.logging.log4j.core.LifeCycle.State;
+import org.apache.logging.log4j.core.appender.ConsoleAppender;
+import org.apache.logging.log4j.core.appender.ConsoleAppender.Target;
+import org.apache.logging.log4j.core.appender.FileAppender;
+import org.apache.logging.log4j.core.appender.NullAppender;
+import org.apache.logging.log4j.core.appender.RollingFileAppender;
+import org.apache.logging.log4j.core.appender.rolling.CompositeTriggeringPolicy;
+import org.apache.logging.log4j.core.appender.rolling.DefaultRolloverStrategy;
+import org.apache.logging.log4j.core.appender.rolling.RolloverStrategy;
+import org.apache.logging.log4j.core.appender.rolling.SizeBasedTriggeringPolicy;
+import org.apache.logging.log4j.core.appender.rolling.TimeBasedTriggeringPolicy;
+import org.apache.logging.log4j.core.appender.rolling.TriggeringPolicy;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.LoggerConfig;
+import org.apache.logging.log4j.core.layout.HtmlLayout;
+import org.apache.logging.log4j.core.layout.PatternLayout;
+
+public abstract class AbstractLog4j1ConfigurationTest {
+
+ abstract Configuration getConfiguration(String configResourcePrefix) throws URISyntaxException, IOException;
Review comment:
@ppkarwasz
Use spaces, no tabs. Follow the style of the file you edit.
--
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
[GitHub] [logging-log4j2] garydgregory commented on pull request #706: Common unit tests for Log4j 1.x configuration factories
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #706:
URL: https://github.com/apache/logging-log4j2/pull/706#issuecomment-1014655999
Hi @ppkarwasz
Failing a build is not going to be acceptable IMO. Because different team members work at different times and availability, like me, I don't want to bring in code that breaks the build, I might have to step away and work on something else, then we would be left with a failing project build. Each PR should pass the build. If a PR fixes a bug, it should come with a test that fails without the main change.
TY.
--
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
[GitHub] [logging-log4j2] ppkarwasz closed pull request #706: Common unit tests for Log4j 1.x configuration factories
Posted by GitBox <gi...@apache.org>.
ppkarwasz closed pull request #706:
URL: https://github.com/apache/logging-log4j2/pull/706
--
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
[GitHub] [logging-log4j2] ppkarwasz commented on pull request #706: Common unit tests for Log4j 1.x configuration factories
Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on pull request #706:
URL: https://github.com/apache/logging-log4j2/pull/706#issuecomment-1014676863
I'll move the tests to the PRs that actually fix bugs, then. Thanks.
--
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