Re: [logging-log4j2] 02/02: Use final and simpler hashcode generation through Objects.
I'd prefer if we didn't incur implicit array allocation cost generating a hash code. My preference is to keep the original implementation. -ck On Wed, Jan 12, 2022, at 08:50, ggreg...@apache.org wrote: > This is an automated email from the ASF dual-hosted git repository. > > ggregory pushed a commit to branch master > in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git > > commit 857eca786c2027469763e2fea93ed2b4a3c2b6c0 > Author: Gary Gregory > AuthorDate: Wed Jan 12 07:09:06 2022 -0500 > > Use final and simpler hashcode generation through Objects. > --- > .../main/java/org/apache/logging/log4j/core/util/Source.java | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git > a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Source.java > b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Source.java > index 9674618..f28c8f8 100644 > --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Source.java > +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/Source.java > @@ -143,15 +143,15 @@ public class Source { > } > > @Override > -public boolean equals(final Object o) { > -if (this == o) { > +public boolean equals(Object obj) { > +if (this == obj) { > return true; > } > -if (!(o instanceof Source)) { > +if (!(obj instanceof Source)) { > return false; > } > -final Source source = (Source) o; > -return Objects.equals(location, source.location); > +Source other = (Source) obj; > +return Objects.equals(location, other.location); > } > > /** > @@ -208,7 +208,7 @@ public class Source { > > @Override > public int hashCode() { > -return 31 + Objects.hashCode(location); > +return Objects.hash(location); > } > > @Override >
Re: [logging-log4j2] 02/04: Our convention is to make test classes public.
I'll note that the convention from JUnit 4 is to make them public; JUnit 5 encourages package-private tests instead for some reason, and that's the default template for JUnit 5 tests in IntelliJ. I do like consistency, though! On Wed, Jan 12, 2022 at 11:01 AM wrote: > > This is an automated email from the ASF dual-hosted git repository. > > ggregory pushed a commit to branch release-2.x > in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git > > commit b4a892acd1d37f355c27cde10ab1736cf1ebe315 > Author: Gary Gregory > AuthorDate: Wed Jan 12 11:58:30 2022 -0500 > > Our convention is to make test classes public. > --- > .../test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java| 2 > +- > .../org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java | 2 > +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > index df98702..5759cf7 100644 > --- > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > +++ > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > @@ -36,7 +36,7 @@ import org.junit.jupiter.api.Test; > /** > * Unit tests for {@link SmtpManager}. > */ > -class SmtpManagerTest { > +public class SmtpManagerTest { > > @Test > void testCreateManagerName() { > diff --git > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > index 5fa603f..87f30dd 100644 > --- > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > +++ > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > @@ -55,7 +55,7 @@ import static org.junit.jupiter.api.Assertions.*; > /** > * Tests reconnection support of {@link > org.apache.logging.log4j.core.appender.SocketAppender}. > */ > -class SocketAppenderReconnectTest { > +public class SocketAppenderReconnectTest { > > private static final Logger LOGGER = StatusLogger.getLogger(); >
Re: [logging-log4j2] 02/04: Our convention is to make test classes public.
Hi Matt, Porting to Junit 5 would be nice. I'm not sure how to deal with all our Junit 4 rules though. Gary On Wed, Jan 12, 2022, 13:07 Matt Sicker wrote: > I'll note that the convention from JUnit 4 is to make them public; > JUnit 5 encourages package-private tests instead for some reason, and > that's the default template for JUnit 5 tests in IntelliJ. I do like > consistency, though! > > On Wed, Jan 12, 2022 at 11:01 AM wrote: > > > > This is an automated email from the ASF dual-hosted git repository. > > > > ggregory pushed a commit to branch release-2.x > > in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git > > > > commit b4a892acd1d37f355c27cde10ab1736cf1ebe315 > > Author: Gary Gregory > > AuthorDate: Wed Jan 12 11:58:30 2022 -0500 > > > > Our convention is to make test classes public. > > --- > > .../test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > | 2 +- > > .../org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > index df98702..5759cf7 100644 > > --- > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > +++ > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > @@ -36,7 +36,7 @@ import org.junit.jupiter.api.Test; > > /** > > * Unit tests for {@link SmtpManager}. > > */ > > -class SmtpManagerTest { > > +public class SmtpManagerTest { > > > > @Test > > void testCreateManagerName() { > > diff --git > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > > index 5fa603f..87f30dd 100644 > > --- > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > > +++ > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > > @@ -55,7 +55,7 @@ import static org.junit.jupiter.api.Assertions.*; > > /** > > * Tests reconnection support of {@link > org.apache.logging.log4j.core.appender.SocketAppender}. > > */ > > -class SocketAppenderReconnectTest { > > +public class SocketAppenderReconnectTest { > > > > private static final Logger LOGGER = StatusLogger.getLogger(); > > >
Re: [logging-log4j2] 02/04: Our convention is to make test classes public.
+1 I prefer minimum visibility by default for the same reason I prefer to make everything final by default: It gives us more freedom to change later on. This doesn't directly apply to tests, but it's nice when a convention applies globally. Most projects don't make junit5 tests public, so there's a question of whether we want to be consistent with our own usage of junit4, or with broader usage of junit5. I prefer the latter. We could enforce it with error-prone for consistency, if desired. -ck On Wed, Jan 12, 2022, at 13:07, Matt Sicker wrote: > I'll note that the convention from JUnit 4 is to make them public; > JUnit 5 encourages package-private tests instead for some reason, and > that's the default template for JUnit 5 tests in IntelliJ. I do like > consistency, though! > > On Wed, Jan 12, 2022 at 11:01 AM wrote: > > > > This is an automated email from the ASF dual-hosted git repository. > > > > ggregory pushed a commit to branch release-2.x > > in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git > > > > commit b4a892acd1d37f355c27cde10ab1736cf1ebe315 > > Author: Gary Gregory > > AuthorDate: Wed Jan 12 11:58:30 2022 -0500 > > > > Our convention is to make test classes public. > > --- > > .../test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java| > > 2 +- > > .../org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java | > > 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git > > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > > > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > index df98702..5759cf7 100644 > > --- > > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > +++ > > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > @@ -36,7 +36,7 @@ import org.junit.jupiter.api.Test; > > /** > > * Unit tests for {@link SmtpManager}. > > */ > > -class SmtpManagerTest { > > +public class SmtpManagerTest { > > > > @Test > > void testCreateManagerName() { > > diff --git > > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > > > > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > > index 5fa603f..87f30dd 100644 > > --- > > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > > +++ > > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > > @@ -55,7 +55,7 @@ import static org.junit.jupiter.api.Assertions.*; > > /** > > * Tests reconnection support of {@link > > org.apache.logging.log4j.core.appender.SocketAppender}. > > */ > > -class SocketAppenderReconnectTest { > > +public class SocketAppenderReconnectTest { > > > > private static final Logger LOGGER = StatusLogger.getLogger(); > > >
Re: [CI][UNSTABLE] Logging/log4j/release-2.x#696 has test failures
Note that GitHub builds are green as are my local builds, both run twice as fast as this Jenkins instance which I guess must be oversubscribed, nothing bad about Jenkins itself of course. Gary On Wed, Jan 12, 2022, 13:03 Mr. Jenkins wrote: > *BUILD UNSTABLE* > Build URL > https://ci-builds.apache.org/job/Logging/job/log4j/job/release-2.x/696/ > Project: release-2.x > Date of build: Wed, 12 Jan 2022 17:01:26 + > Build duration: 1 hr 2 min and counting > * JUnit Tests * > Name: (root) Failed: 0 test(s), Passed: 1420 test(s), Skipped: 0 test(s), > Total: 1420 test(s) > Name: liquibase.ext.logging.log4j2 Failed: 0 test(s), Passed: 10 test(s), > Skipped: 0 test(s), Total: 10 test(s) > Name: org.apache.log4j Failed: 0 test(s), Passed: 164 test(s), Skipped: 0 > test(s), Total: 164 test(s) > Name: org.apache.log4j.bridge Failed: 0 test(s), Passed: 4 test(s), > Skipped: 0 test(s), Total: 4 test(s) > Name: org.apache.log4j.config Failed: 0 test(s), Passed: 78 test(s), > Skipped: 0 test(s), Total: 78 test(s) > Name: org.apache.log4j.layout Failed: 0 test(s), Passed: 4 test(s), > Skipped: 0 test(s), Total: 4 test(s) > Name: org.apache.log4j.pattern Failed: 0 test(s), Passed: 16 test(s), > Skipped: 0 test(s), Total: 16 test(s) > Name: org.apache.logging.log4j Failed: 1 test(s), Passed: 487 test(s), > Skipped: 2 test(s), Total: 490 test(s) > *- Failed: > org.apache.logging.log4j.LoggerSupplierTest.flowTracing_SupplierOfObjectArrayMessage* > Name: org.apache.logging.log4j.cassandra Failed: 0 test(s), Passed: 2 > test(s), Skipped: 0 test(s), Total: 2 test(s) > Name: org.apache.logging.log4j.configuration Failed: 0 test(s), Passed: 2 > test(s), Skipped: 0 test(s), Total: 2 test(s) > Name: org.apache.logging.log4j.core Failed: 0 test(s), Passed: 184 > test(s), Skipped: 2 test(s), Total: 186 test(s) > Name: org.apache.logging.log4j.core.appender Failed: 0 test(s), Passed: > 238 test(s), Skipped: 12 test(s), Total: 250 test(s) > Name: org.apache.logging.log4j.core.appender.db Failed: 0 test(s), Passed: > 26 test(s), Skipped: 0 test(s), Total: 26 test(s) > Name: org.apache.logging.log4j.core.appender.db.jdbc Failed: 0 test(s), > Passed: 80 test(s), Skipped: 0 test(s), Total: 80 test(s) > Name: org.apache.logging.log4j.core.appender.db.jpa Failed: 0 test(s), > Passed: 24 test(s), Skipped: 0 test(s), Total: 24 test(s) > Name: org.apache.logging.log4j.core.appender.db.jpa.converter Failed: 0 > test(s), Passed: 88 test(s), Skipped: 0 test(s), Total: 88 test(s) > Name: org.apache.logging.log4j.core.appender.mom Failed: 0 test(s), > Passed: 8 test(s), Skipped: 0 test(s), Total: 8 test(s) > Name: org.apache.logging.log4j.core.appender.mom.jeromq Failed: 0 test(s), > Passed: 8 test(s), Skipped: 0 test(s), Total: 8 test(s) > Name: org.apache.logging.log4j.core.appender.mom.kafka Failed: 0 test(s), > Passed: 16 test(s), Skipped: 0 test(s), Total: 16 test(s) > Name: org.apache.logging.log4j.core.appender.nosql Failed: 0 test(s), > Passed: 18 test(s), Skipped: 0 test(s), Total: 18 test(s) > Name: org.apache.logging.log4j.core.appender.rewrite Failed: 0 test(s), > Passed: 14 test(s), Skipped: 0 test(s), Total: 14 test(s) > Name: org.apache.logging.log4j.core.appender.rolling Failed: 0 test(s), > Passed: 203 test(s), Skipped: 2 test(s), Total: 205 test(s) > Name: org.apache.logging.log4j.core.appender.rolling.action Failed: 0 > test(s), Passed: 184 test(s), Skipped: 0 test(s), Total: 184 test(s) > Name: org.apache.logging.log4j.core.appender.routing Failed: 0 test(s), > Passed: 26 test(s), Skipped: 0 test(s), Total: 26 test(s) > Name: org.apache.logging.log4j.core.async Failed: 0 test(s), Passed: 160 > test(s), Skipped: 0 test(s), Total: 160 test(s) > Name: org.apache.logging.log4j.core.config Failed: 0 test(s), Passed: 342 > test(s), Skipped: 2 test(s), Total: 344 test(s) > Name: org.apache.logging.log4j.core.config.arbiters Failed: 0 test(s), > Passed: 12 test(s), Skipped: 0 test(s), Total: 12 test(s) > Name: org.apache.logging.log4j.core.config.builder Failed: 0 test(s), > Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s) > Name: org.apache.logging.log4j.core.config.plugins.convert Failed: 0 > test(s), Passed: 12 test(s), Skipped: 0 test(s), Total: 12 test(s) > Name: org.apache.logging.log4j.core.config.plugins.processor Failed: 0 > test(s), Passed: 10 test(s), Skipped: 0 test(s), Total: 10 test(s) > Name: org.apache.logging.log4j.core.config.plugins.util Failed: 0 test(s), > Passed: 52 test(s), Skipped: 0 test(s), Total: 52 test(s) > Name: org.apache.logging.log4j.core.config.plugins.validation.validators > Failed: 0 test(s), Passed: 30 test(s), Skipped: 0 test(s), Total: 30 test(s) > Name: org.apache.logging.log4j.core.config.properties Failed: 0 test(s), > Passed: 8 test(s), Skipped: 0 test(s), Total: 8 test(s) > Name: org.apache.logging.log4j.core.config.xml Failed: 0 test(s), Passed: > 10 test(s), Skipped: 0 test(s), Total: 10 test(s) > Name: org.apache.logging.log4j.core.filter Failed: 0 test(s), Passed: 100 > test
Re: [CI][UNSTABLE] Logging/log4j/release-2.x#696 has test failures
What is annoying is that the Jenkins builds seem to fail somewhere different every time and in places that I have never before seen fail. Ralph > On Jan 12, 2022, at 11:36 AM, Gary Gregory wrote: > > Note that GitHub builds are green as are my local builds, both run twice as > fast as this Jenkins instance which I guess must be oversubscribed, nothing > bad about Jenkins itself of course. > > Gary > > > > > On Wed, Jan 12, 2022, 13:03 Mr. Jenkins > wrote: > >> *BUILD UNSTABLE* >> Build URL >> https://ci-builds.apache.org/job/Logging/job/log4j/job/release-2.x/696/ >> Project: release-2.x >> Date of build: Wed, 12 Jan 2022 17:01:26 + >> Build duration: 1 hr 2 min and counting >> * JUnit Tests * >> Name: (root) Failed: 0 test(s), Passed: 1420 test(s), Skipped: 0 test(s), >> Total: 1420 test(s) >> Name: liquibase.ext.logging.log4j2 Failed: 0 test(s), Passed: 10 test(s), >> Skipped: 0 test(s), Total: 10 test(s) >> Name: org.apache.log4j Failed: 0 test(s), Passed: 164 test(s), Skipped: 0 >> test(s), Total: 164 test(s) >> Name: org.apache.log4j.bridge Failed: 0 test(s), Passed: 4 test(s), >> Skipped: 0 test(s), Total: 4 test(s) >> Name: org.apache.log4j.config Failed: 0 test(s), Passed: 78 test(s), >> Skipped: 0 test(s), Total: 78 test(s) >> Name: org.apache.log4j.layout Failed: 0 test(s), Passed: 4 test(s), >> Skipped: 0 test(s), Total: 4 test(s) >> Name: org.apache.log4j.pattern Failed: 0 test(s), Passed: 16 test(s), >> Skipped: 0 test(s), Total: 16 test(s) >> Name: org.apache.logging.log4j Failed: 1 test(s), Passed: 487 test(s), >> Skipped: 2 test(s), Total: 490 test(s) >> *- Failed: >> org.apache.logging.log4j.LoggerSupplierTest.flowTracing_SupplierOfObjectArrayMessage* >> Name: org.apache.logging.log4j.cassandra Failed: 0 test(s), Passed: 2 >> test(s), Skipped: 0 test(s), Total: 2 test(s) >> Name: org.apache.logging.log4j.configuration Failed: 0 test(s), Passed: 2 >> test(s), Skipped: 0 test(s), Total: 2 test(s) >> Name: org.apache.logging.log4j.core Failed: 0 test(s), Passed: 184 >> test(s), Skipped: 2 test(s), Total: 186 test(s) >> Name: org.apache.logging.log4j.core.appender Failed: 0 test(s), Passed: >> 238 test(s), Skipped: 12 test(s), Total: 250 test(s) >> Name: org.apache.logging.log4j.core.appender.db Failed: 0 test(s), Passed: >> 26 test(s), Skipped: 0 test(s), Total: 26 test(s) >> Name: org.apache.logging.log4j.core.appender.db.jdbc Failed: 0 test(s), >> Passed: 80 test(s), Skipped: 0 test(s), Total: 80 test(s) >> Name: org.apache.logging.log4j.core.appender.db.jpa Failed: 0 test(s), >> Passed: 24 test(s), Skipped: 0 test(s), Total: 24 test(s) >> Name: org.apache.logging.log4j.core.appender.db.jpa.converter Failed: 0 >> test(s), Passed: 88 test(s), Skipped: 0 test(s), Total: 88 test(s) >> Name: org.apache.logging.log4j.core.appender.mom Failed: 0 test(s), >> Passed: 8 test(s), Skipped: 0 test(s), Total: 8 test(s) >> Name: org.apache.logging.log4j.core.appender.mom.jeromq Failed: 0 test(s), >> Passed: 8 test(s), Skipped: 0 test(s), Total: 8 test(s) >> Name: org.apache.logging.log4j.core.appender.mom.kafka Failed: 0 test(s), >> Passed: 16 test(s), Skipped: 0 test(s), Total: 16 test(s) >> Name: org.apache.logging.log4j.core.appender.nosql Failed: 0 test(s), >> Passed: 18 test(s), Skipped: 0 test(s), Total: 18 test(s) >> Name: org.apache.logging.log4j.core.appender.rewrite Failed: 0 test(s), >> Passed: 14 test(s), Skipped: 0 test(s), Total: 14 test(s) >> Name: org.apache.logging.log4j.core.appender.rolling Failed: 0 test(s), >> Passed: 203 test(s), Skipped: 2 test(s), Total: 205 test(s) >> Name: org.apache.logging.log4j.core.appender.rolling.action Failed: 0 >> test(s), Passed: 184 test(s), Skipped: 0 test(s), Total: 184 test(s) >> Name: org.apache.logging.log4j.core.appender.routing Failed: 0 test(s), >> Passed: 26 test(s), Skipped: 0 test(s), Total: 26 test(s) >> Name: org.apache.logging.log4j.core.async Failed: 0 test(s), Passed: 160 >> test(s), Skipped: 0 test(s), Total: 160 test(s) >> Name: org.apache.logging.log4j.core.config Failed: 0 test(s), Passed: 342 >> test(s), Skipped: 2 test(s), Total: 344 test(s) >> Name: org.apache.logging.log4j.core.config.arbiters Failed: 0 test(s), >> Passed: 12 test(s), Skipped: 0 test(s), Total: 12 test(s) >> Name: org.apache.logging.log4j.core.config.builder Failed: 0 test(s), >> Passed: 6 test(s), Skipped: 0 test(s), Total: 6 test(s) >> Name: org.apache.logging.log4j.core.config.plugins.convert Failed: 0 >> test(s), Passed: 12 test(s), Skipped: 0 test(s), Total: 12 test(s) >> Name: org.apache.logging.log4j.core.config.plugins.processor Failed: 0 >> test(s), Passed: 10 test(s), Skipped: 0 test(s), Total: 10 test(s) >> Name: org.apache.logging.log4j.core.config.plugins.util Failed: 0 test(s), >> Passed: 52 test(s), Skipped: 0 test(s), Total: 52 test(s) >> Name: org.apache.logging.log4j.core.config.plugins.validation.validators >> Failed: 0 test(s), Passed: 30 test(s), Skipped: 0 test(s), Total: 30 test(s) >> Name: org.apache.logging.log4j.core.con
Re: [logging-log4j2] 02/04: Our convention is to make test classes public.
I happy to stick with Junit 5 convertions once we drop Junit 4, which feels like a tediuous big job :-( Gafy On Wed, Jan 12, 2022, 13:33 Carter Kozak wrote: > +1 > > I prefer minimum visibility by default for the same reason I prefer to > make everything final by default: It gives us more freedom to change later > on. This doesn't directly apply to tests, but it's nice when a convention > applies globally. > > Most projects don't make junit5 tests public, so there's a question of > whether we want to be consistent with our own usage of junit4, or with > broader usage of junit5. I prefer the latter. We could enforce it with > error-prone for consistency, if desired. > > -ck > > On Wed, Jan 12, 2022, at 13:07, Matt Sicker wrote: > > I'll note that the convention from JUnit 4 is to make them public; > > JUnit 5 encourages package-private tests instead for some reason, and > > that's the default template for JUnit 5 tests in IntelliJ. I do like > > consistency, though! > > > > On Wed, Jan 12, 2022 at 11:01 AM wrote: > > > > > > This is an automated email from the ASF dual-hosted git repository. > > > > > > ggregory pushed a commit to branch release-2.x > > > in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git > > > > > > commit b4a892acd1d37f355c27cde10ab1736cf1ebe315 > > > Author: Gary Gregory > > > AuthorDate: Wed Jan 12 11:58:30 2022 -0500 > > > > > > Our convention is to make test classes public. > > > --- > > > .../test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > | 2 +- > > > > .../org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java | 2 > +- > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > > index df98702..5759cf7 100644 > > > --- > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > > +++ > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > > @@ -36,7 +36,7 @@ import org.junit.jupiter.api.Test; > > > /** > > > * Unit tests for {@link SmtpManager}. > > > */ > > > -class SmtpManagerTest { > > > +public class SmtpManagerTest { > > > > > > @Test > > > void testCreateManagerName() { > > > diff --git > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > > > index 5fa603f..87f30dd 100644 > > > --- > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > > > +++ > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > > > @@ -55,7 +55,7 @@ import static org.junit.jupiter.api.Assertions.*; > > > /** > > > * Tests reconnection support of {@link > org.apache.logging.log4j.core.appender.SocketAppender}. > > > */ > > > -class SocketAppenderReconnectTest { > > > +public class SocketAppenderReconnectTest { > > > > > > private static final Logger LOGGER = StatusLogger.getLogger(); > > > > > >
Re: [logging-log4j2] 02/04: Our convention is to make test classes public.
Most of the JUnit tests can be mechanically translated via IntelliJ (or other tools potentially). The tests that need manual migration are ones that use an expected exception in the @Test annotation and tests that use rules. I've already ported most of the JUnit 4 rules into equivalent JUnit 5 extensions, though they work a little differently (e.g., instead of invoking a rule instance to get objects from the LoggerContext, you can add a LoggerContext parameter to your test method and have it injected). On Wed, Jan 12, 2022 at 12:46 PM Gary Gregory wrote: > > I happy to stick with Junit 5 convertions once we drop Junit 4, which feels > like a tediuous big job :-( > > Gafy > > On Wed, Jan 12, 2022, 13:33 Carter Kozak wrote: > > > +1 > > > > I prefer minimum visibility by default for the same reason I prefer to > > make everything final by default: It gives us more freedom to change later > > on. This doesn't directly apply to tests, but it's nice when a convention > > applies globally. > > > > Most projects don't make junit5 tests public, so there's a question of > > whether we want to be consistent with our own usage of junit4, or with > > broader usage of junit5. I prefer the latter. We could enforce it with > > error-prone for consistency, if desired. > > > > -ck > > > > On Wed, Jan 12, 2022, at 13:07, Matt Sicker wrote: > > > I'll note that the convention from JUnit 4 is to make them public; > > > JUnit 5 encourages package-private tests instead for some reason, and > > > that's the default template for JUnit 5 tests in IntelliJ. I do like > > > consistency, though! > > > > > > On Wed, Jan 12, 2022 at 11:01 AM wrote: > > > > > > > > This is an automated email from the ASF dual-hosted git repository. > > > > > > > > ggregory pushed a commit to branch release-2.x > > > > in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git > > > > > > > > commit b4a892acd1d37f355c27cde10ab1736cf1ebe315 > > > > Author: Gary Gregory > > > > AuthorDate: Wed Jan 12 11:58:30 2022 -0500 > > > > > > > > Our convention is to make test classes public. > > > > --- > > > > .../test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > | 2 +- > > > > > > .../org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java | 2 > > +- > > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git > > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > > > index df98702..5759cf7 100644 > > > > --- > > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > > > +++ > > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > > > @@ -36,7 +36,7 @@ import org.junit.jupiter.api.Test; > > > > /** > > > > * Unit tests for {@link SmtpManager}. > > > > */ > > > > -class SmtpManagerTest { > > > > +public class SmtpManagerTest { > > > > > > > > @Test > > > > void testCreateManagerName() { > > > > diff --git > > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > > > > index 5fa603f..87f30dd 100644 > > > > --- > > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > > > > +++ > > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > > > > @@ -55,7 +55,7 @@ import static org.junit.jupiter.api.Assertions.*; > > > > /** > > > > * Tests reconnection support of {@link > > org.apache.logging.log4j.core.appender.SocketAppender}. > > > > */ > > > > -class SocketAppenderReconnectTest { > > > > +public class SocketAppenderReconnectTest { > > > > > > > > private static final Logger LOGGER = StatusLogger.getLogger(); > > > > > > > > >
Re: [logging-log4j2] 02/04: Our convention is to make test classes public.
Good to hear. I've stubbed my toes going from Junit rules 4 to Junit 5 extensions. I don't want to take the time to do that now though. Gary On Wed, Jan 12, 2022, 14:42 Matt Sicker wrote: > Most of the JUnit tests can be mechanically translated via IntelliJ > (or other tools potentially). The tests that need manual migration are > ones that use an expected exception in the @Test annotation and tests > that use rules. I've already ported most of the JUnit 4 rules into > equivalent JUnit 5 extensions, though they work a little differently > (e.g., instead of invoking a rule instance to get objects from the > LoggerContext, you can add a LoggerContext parameter to your test > method and have it injected). > > On Wed, Jan 12, 2022 at 12:46 PM Gary Gregory > wrote: > > > > I happy to stick with Junit 5 convertions once we drop Junit 4, which > feels > > like a tediuous big job :-( > > > > Gafy > > > > On Wed, Jan 12, 2022, 13:33 Carter Kozak wrote: > > > > > +1 > > > > > > I prefer minimum visibility by default for the same reason I prefer to > > > make everything final by default: It gives us more freedom to change > later > > > on. This doesn't directly apply to tests, but it's nice when a > convention > > > applies globally. > > > > > > Most projects don't make junit5 tests public, so there's a question of > > > whether we want to be consistent with our own usage of junit4, or with > > > broader usage of junit5. I prefer the latter. We could enforce it with > > > error-prone for consistency, if desired. > > > > > > -ck > > > > > > On Wed, Jan 12, 2022, at 13:07, Matt Sicker wrote: > > > > I'll note that the convention from JUnit 4 is to make them public; > > > > JUnit 5 encourages package-private tests instead for some reason, and > > > > that's the default template for JUnit 5 tests in IntelliJ. I do like > > > > consistency, though! > > > > > > > > On Wed, Jan 12, 2022 at 11:01 AM wrote: > > > > > > > > > > This is an automated email from the ASF dual-hosted git repository. > > > > > > > > > > ggregory pushed a commit to branch release-2.x > > > > > in repository > https://gitbox.apache.org/repos/asf/logging-log4j2.git > > > > > > > > > > commit b4a892acd1d37f355c27cde10ab1736cf1ebe315 > > > > > Author: Gary Gregory > > > > > AuthorDate: Wed Jan 12 11:58:30 2022 -0500 > > > > > > > > > > Our convention is to make test classes public. > > > > > --- > > > > > > .../test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > > | 2 +- > > > > > > > > > .../org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java | 2 > > > +- > > > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git > > > > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > > > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > > > > index df98702..5759cf7 100644 > > > > > --- > > > > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > > > > +++ > > > > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SmtpManagerTest.java > > > > > @@ -36,7 +36,7 @@ import org.junit.jupiter.api.Test; > > > > > /** > > > > > * Unit tests for {@link SmtpManager}. > > > > > */ > > > > > -class SmtpManagerTest { > > > > > +public class SmtpManagerTest { > > > > > > > > > > @Test > > > > > void testCreateManagerName() { > > > > > diff --git > > > > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > > > > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > > > > > index 5fa603f..87f30dd 100644 > > > > > --- > > > > a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > > > > > +++ > > > > b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java > > > > > @@ -55,7 +55,7 @@ import static org.junit.jupiter.api.Assertions.*; > > > > > /** > > > > > * Tests reconnection support of {@link > > > org.apache.logging.log4j.core.appender.SocketAppender}. > > > > > */ > > > > > -class SocketAppenderReconnectTest { > > > > > +public class SocketAppenderReconnectTest { > > > > > > > > > > private static final Logger LOGGER = StatusLogger.getLogger(); > > > > > > > > > > > > >
Code Formatter?
Hi all, We've discussed the idea of using a code formatter before, I finally had a moment put up an example. Please take a look and provide feedback at your convenience :-) https://github.com/apache/logging-log4j2/pull/697 -ck