You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by gg...@apache.org on 2022/01/12 17:01:10 UTC
[logging-log4j2] branch release-2.x updated (0fc41eb -> 0f3510a)
This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a change to branch release-2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git.
from 0fc41eb [LOG4J2-3330] Configurator.setLevel not fetching the correct LoggerContext.
new e3fd782 Minor bullet-proofing for WIP.
new b4a892a Our convention is to make test classes public.
new 68a131f Increase timeout to attempt to fix GitHub action builds.
new 0f3510a Remove duplicate data carried in ConfigurationSource.
The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../log4j/core/config/AbstractConfiguration.java | 2 +-
.../log4j/core/config/ConfigurationSource.java | 99 +++++++++-------------
.../logging/log4j/core/net/SmtpManagerTest.java | 2 +-
.../core/net/SocketAppenderReconnectTest.java | 4 +-
4 files changed, 44 insertions(+), 63 deletions(-)
[logging-log4j2] 03/04: Increase timeout to attempt to fix GitHub action builds.
Posted by gg...@apache.org.
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 68a131f2ad3b82ab9b3140aa0da5154c0b7de259
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Wed Jan 12 12:00:15 2022 -0500
Increase timeout to attempt to fix GitHub action builds.
---
.../org/apache/logging/log4j/core/net/SocketAppenderReconnectTest.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 87f30dd..21ce372 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
@@ -201,7 +201,7 @@ public class SocketAppenderReconnectTest {
private static void awaitUntilSucceeds(final Runnable runnable) {
// These figures are collected via trial-and-error; nothing scientific to look for here.
final long pollIntervalMillis = 1_000L;
- final long timeoutSeconds = 15L;
+ final long timeoutSeconds = 60L;
await()
.pollInterval(pollIntervalMillis, TimeUnit.MILLISECONDS)
.atMost(timeoutSeconds, TimeUnit.SECONDS)
[logging-log4j2] 01/04: Minor bullet-proofing for WIP.
Posted by gg...@apache.org.
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 e3fd782471c97b0b7af3fb7089c90bd658224daf
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Wed Jan 12 11:58:00 2022 -0500
Minor bullet-proofing for WIP.
---
.../org/apache/logging/log4j/core/config/AbstractConfiguration.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java
index 5c072f1..d981768 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java
@@ -251,7 +251,7 @@ public abstract class AbstractConfiguration extends AbstractFilterable implement
protected void initializeWatchers(Reconfigurable reconfigurable, ConfigurationSource configSource,
int monitorIntervalSeconds) {
- if (configSource.getFile() != null || configSource.getURL() != null) {
+ if (configSource != null && (configSource.getFile() != null || configSource.getURL() != null)) {
if (monitorIntervalSeconds > 0) {
watchManager.setIntervalSeconds(monitorIntervalSeconds);
if (configSource.getFile() != null) {
[logging-log4j2] 04/04: Remove duplicate data carried in ConfigurationSource.
Posted by gg...@apache.org.
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 0f3510ab5f2371705b2f5cf52e9807fb5804052a
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Wed Jan 12 12:01:05 2022 -0500
Remove duplicate data carried in ConfigurationSource.
Deprecated unused API.
---
.../log4j/core/config/ConfigurationSource.java | 99 +++++++++-------------
1 file changed, 40 insertions(+), 59 deletions(-)
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationSource.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationSource.java
index 5e6ef9b..be931c5 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationSource.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationSource.java
@@ -61,9 +61,6 @@ public class ConfigurationSource {
public static final ConfigurationSource COMPOSITE_SOURCE = new ConfigurationSource(Constants.EMPTY_BYTE_ARRAY, null, 0);
private static final String HTTPS = "https";
- private final File file;
- private final URL url;
- private final String location;
private final InputStream stream;
private volatile byte[] data;
private volatile Source source;
@@ -75,15 +72,13 @@ public class ConfigurationSource {
* Constructs a new {@code ConfigurationSource} with the specified input stream that originated from the specified
* file.
*
- * @param stream the input stream
+ * @param stream the input stream, the caller is responsible for closing this resource.
* @param file the file where the input stream originated
*/
public ConfigurationSource(final InputStream stream, final File file) {
this.stream = Objects.requireNonNull(stream, "stream is null");
- this.file = Objects.requireNonNull(file, "file is null");
- this.location = file.getAbsolutePath();
- this.url = null;
this.data = null;
+ this.source = new Source(file);
long modified = 0;
try {
modified = file.lastModified();
@@ -97,40 +92,36 @@ public class ConfigurationSource {
* Constructs a new {@code ConfigurationSource} with the specified input stream that originated from the specified
* url.
*
- * @param stream the input stream
+ * @param stream the input stream, the caller is responsible for closing this resource.
* @param url the URL where the input stream originated
*/
public ConfigurationSource(final InputStream stream, final URL url) {
this.stream = Objects.requireNonNull(stream, "stream is null");
- this.url = Objects.requireNonNull(url, "URL is null");
- this.location = url.toString();
- this.file = null;
this.data = null;
this.lastModified = 0;
+ this.source = new Source(url);
}
/**
* Constructs a new {@code ConfigurationSource} with the specified input stream that originated from the specified
* url.
*
- * @param stream the input stream
+ * @param stream the input stream, the caller is responsible for closing this resource.
* @param url the URL where the input stream originated
* @param lastModified when the source was last modified.
*/
public ConfigurationSource(final InputStream stream, final URL url, long lastModified) {
this.stream = Objects.requireNonNull(stream, "stream is null");
- this.url = Objects.requireNonNull(url, "URL is null");
- this.location = url.toString();
- this.file = null;
this.data = null;
this.lastModified = lastModified;
+ this.source = new Source(url);
}
/**
* Constructs a new {@code ConfigurationSource} with the specified input stream. Since the stream is the only source
* of data, this constructor makes a copy of the stream contents.
*
- * @param stream the input stream
+ * @param stream the input stream, the caller is responsible for closing this resource.
* @throws IOException if an exception occurred reading from the specified stream
*/
public ConfigurationSource(final InputStream stream) throws IOException {
@@ -141,21 +132,18 @@ public class ConfigurationSource {
Objects.requireNonNull(source, "source is null");
this.data = Objects.requireNonNull(data, "data is null");
this.stream = new ByteArrayInputStream(data);
- this.file = source.getFile();
- this.url = source.getURI().toURL();
- this.location = source.getLocation();
this.lastModified = lastModified;
+ this.source = source;
}
private ConfigurationSource(final byte[] data, final URL url, long lastModified) {
this.data = Objects.requireNonNull(data, "data is null");
this.stream = new ByteArrayInputStream(data);
- this.file = null;
- this.url = url;
- this.location = null;
this.lastModified = lastModified;
- if ( url == null ) {
- this.data = data;
+ if (url == null) {
+ this.data = data;
+ } else {
+ this.source = new Source(url);
}
}
@@ -186,9 +174,21 @@ public class ConfigurationSource {
* @return the configuration source file, or {@code null}
*/
public File getFile() {
- return file;
+ return source == null ? null : source.getFile();
}
+ private boolean isFile() {
+ return source == null ? false : source.getFile() != null;
+ }
+
+ private boolean isURL() {
+ return source == null ? false : source.getURI() != null;
+ }
+
+ private boolean isLocation() {
+ return source == null ? false : source.getLocation() != null;
+ }
+
/**
* Returns the configuration source URL, or {@code null} if this configuration source is based on a file or has
* neither a file nor an URL.
@@ -196,9 +196,13 @@ public class ConfigurationSource {
* @return the configuration source URL, or {@code null}
*/
public URL getURL() {
- return url;
+ return source == null ? null : source.getURL();
}
+ /**
+ * @deprecated Not used internally, no replacement. TODO remove and make source final.
+ */
+ @Deprecated
public void setSource(Source source) {
this.source = source;
}
@@ -216,30 +220,7 @@ public class ConfigurationSource {
* @return The URI.
*/
public URI getURI() {
- URI sourceURI = null;
- if (url != null) {
- try {
- sourceURI = url.toURI();
- } catch (final URISyntaxException ex) {
- /* Ignore the exception */
- }
- }
- if (sourceURI == null && file != null) {
- sourceURI = file.toURI();
- }
- if (sourceURI == null && location != null) {
- try {
- sourceURI = new URI(location);
- } catch (final URISyntaxException ex) {
- // Assume the scheme was missing.
- try {
- sourceURI = new URI("file://" + location);
- } catch (final URISyntaxException uriEx) {
- /* Ignore the exception */
- }
- }
- }
- return sourceURI;
+ return source == null ? null : source.getURI();
}
/**
@@ -257,7 +238,7 @@ public class ConfigurationSource {
* @return a string describing the configuration source file or URL, or {@code null}
*/
public String getLocation() {
- return location;
+ return source == null ? null : source.getLocation();
}
/**
@@ -276,14 +257,14 @@ public class ConfigurationSource {
* @throws IOException if a problem occurred while opening the new input stream
*/
public ConfigurationSource resetInputStream() throws IOException {
- if (source != null) {
+ if (source != null && data != null) {
return new ConfigurationSource(source, data, this.lastModified);
- } else if (file != null) {
- return new ConfigurationSource(new FileInputStream(file), file);
- } else if (url != null && data != null) {
+ } else if (isFile()) {
+ return new ConfigurationSource(new FileInputStream(getFile()), getFile());
+ } else if (isURL() && data != null) {
// Creates a ConfigurationSource without accessing the URL since the data was provided.
- return new ConfigurationSource(data, url, modifiedMillis == 0 ? lastModified : modifiedMillis);
- } else if (url != null) {
+ return new ConfigurationSource(data, getURL(), modifiedMillis == 0 ? lastModified : modifiedMillis);
+ } else if (isURL()) {
return fromUri(getURI());
} else if (data != null) {
return new ConfigurationSource(data, null, lastModified);
@@ -293,8 +274,8 @@ public class ConfigurationSource {
@Override
public String toString() {
- if (location != null) {
- return location;
+ if (isLocation()) {
+ return getLocation();
}
if (this == NULL_SOURCE) {
return "NULL_SOURCE";
Re: [logging-log4j2] 02/04: Our convention is to make test classes public.
Posted by Gary Gregory <ga...@gmail.com>.
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 <bo...@gmail.com> 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 <gg...@apache.org> 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 <ga...@gmail.com>
> > 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.
Posted by Gary Gregory <ga...@gmail.com>.
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 <bo...@gmail.com> 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 <ga...@gmail.com>
> 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 <ck...@ckozak.net> 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 <gg...@apache.org> 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 <ga...@gmail.com>
> > > > > 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.
Posted by Matt Sicker <bo...@gmail.com>.
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 <ga...@gmail.com> 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 <ck...@ckozak.net> 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 <gg...@apache.org> 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 <ga...@gmail.com>
> > > > 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.
Posted by Gary Gregory <ga...@gmail.com>.
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 <ck...@ckozak.net> 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 <gg...@apache.org> 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 <ga...@gmail.com>
> > > 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.
Posted by Carter Kozak <ck...@ckozak.net>.
+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 <gg...@apache.org> 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 <ga...@gmail.com>
> > 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.
Posted by Matt Sicker <bo...@gmail.com>.
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 <gg...@apache.org> 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 <ga...@gmail.com>
> 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();
>
[logging-log4j2] 02/04: Our convention is to make test classes public.
Posted by gg...@apache.org.
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 <ga...@gmail.com>
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();