Re: [logging-log4j2] 02/02: Use final and simpler hashcode generation through Objects.

2022-01-12 Thread Carter Kozak
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.

2022-01-12 Thread Matt Sicker
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.

2022-01-12 Thread Gary Gregory
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.

2022-01-12 Thread Carter Kozak
+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

2022-01-12 Thread Gary Gregory
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

2022-01-12 Thread Ralph Goers
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.

2022-01-12 Thread Gary Gregory
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.

2022-01-12 Thread Matt Sicker
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.

2022-01-12 Thread Gary Gregory
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?

2022-01-12 Thread Carter Kozak
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