You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@logging.apache.org by Volkan Yazıcı <vo...@yazi.ci> on 2022/01/03 12:30:02 UTC
Re: Master branch
Fantastic work Ralph! Please see my comments below:
On Tue, Dec 28, 2021 at 9:39 AM Ralph Goers <ra...@dslextreme.com>
wrote:
> Most of the components that were generating test jars have been split
into two modules - the main component,
> which only builds the jar, and a -test component that builds a -test jar
and then runs the unit tests.
For completeness and consistency across all modules *and* to (hopefully)
mitigate the problem you have mentioned in #2, I think we should place all
tests to their own modules. Even though we all agree that this is ugly,
IIRC, our most recent Maven and JPMS research pointed us in this direction.
I also propose backporting every "moving tests to a module" change to
`release-2.x`. Otherwise, it will be really difficult to synchronize
changes on both branches in the future.
> 1. log4j-core uses toolchains on MacOS to use Java 17. There is a bug in
javac on Macs that causes compilation
> to fail. It is fixed in Java 17 but was not back-ported to Java 11.
For one, I am fine if we set the build (not target!) JDK requirement to
Java 17 for all platforms to work around this problem. This said, I am
confused on why we still need Maven toolchains in `master`. Shouldn't we
simply remove all Maven toolchains configuration and configure
maven-compiler-plugin and maven-enforcer plugin to require 17 for build and
target 11?
> 2. In general the test jars have a second compilation step to create the
module-info.java. Unfortunately, the unit
> tests cannot be modularized because:
> a. the unit tests open the main module (i.e. they use the same
package names).
> b. the test jar requires the main module.
> c. the unit tests require the test module.
> This creates a circularity because the unit tests depend on the test
module and it requires the same packages
> as the unit tests.
Will this still pose a problem if we move all tests to their own module?
> 3. I tried building entirely with Java 17 but there were some unit tests
in log4j-core-test that fail. There is one class
> that is trying to modify a final variable in the Constant class and
it seems that the technique it is using is no
> longer valid.
> 4. log4j-jul must use Junit 4. It seems Junit 5 is hard-wired to use
java.util.logging and so it isn’t possible to set the
> system property in time to have the unit tests use Log4j’s LogManager.
Do we have tickets for these two?
> 5. JsonTemplateLayout is still creating a test-jar. I haven’t validated
if anything needs it. If it does than it will need to
> be split to have a -test module as well.
JTL test-jar is used by log4j-perf.
I have created LOG4J2-3308
<https://issues.apache.org/jira/browse/LOG4J2-3308> to address this.
> 6. I split the annotation processor into its own module -
log4j-plugin-processor. Note that with modular compiles
> annotation processors are no longer added automatically so this
should be clearer.
> 7. I have not added module-info.java files to anything beyond log4j-api,
log4j-plugins, and log4j-core. That will be
> coming next.
> 8. log4j-api-test has a few tests that seem to randomly fail. I didn’t
look into them.
Again, great work Ralph!
Thanks so much for pushing `master` further.
Re: Master branch
Posted by Ralph Goers <ra...@dslextreme.com>.
Gary,
To be clear, you are proposing that we create the same log4j-api-test and
log4j-core-test modules that exist in master? If you want to do that work I won’t object.
But I myself would prefer to focus on master at this point as much as possible so we
can get 3.0 out in a reasonable time frame. Obviously, I have Jira issues in my queue
that also need to be addressed so I will also still have to commit to release-2.x.
Ralph
> On Jan 6, 2022, at 6:53 AM, Gary Gregory <ga...@gmail.com> wrote:
>
> On Mon, Jan 3, 2022 at 7:30 AM Volkan Yazıcı <vo...@yazi.ci> wrote:
>
>> Fantastic work Ralph! Please see my comments below:
>>
>> On Tue, Dec 28, 2021 at 9:39 AM Ralph Goers <ra...@dslextreme.com>
>> wrote:
>>> Most of the components that were generating test jars have been split
>> into two modules - the main component,
>>> which only builds the jar, and a -test component that builds a -test jar
>> and then runs the unit tests.
>>
>> For completeness and consistency across all modules *and* to (hopefully)
>> mitigate the problem you have mentioned in #2, I think we should place all
>> tests to their own modules. Even though we all agree that this is ugly,
>> IIRC, our most recent Maven and JPMS research pointed us in this direction.
>>
>> I also propose backporting every "moving tests to a module" change to
>> `release-2.x`. Otherwise, it will be really difficult to synchronize
>> changes on both branches in the future.
>>
>
> I agree with Volkan. Whatever nightmare of a mess JPMS is making in master,
> it is no longer possible to cherry-pick what should be simple commits from
> release-2.x to master. Patches/diff files are not even possible, it is all
> manual, this is a f*g PITA for both development and maintenance as I am
> currently experiencing :-( This is not a good use of time.
>
> Frustrated,
> Gary
>
>
>>
>>> 1. log4j-core uses toolchains on MacOS to use Java 17. There is a bug in
>> javac on Macs that causes compilation
>>> to fail. It is fixed in Java 17 but was not back-ported to Java 11.
>>
>> For one, I am fine if we set the build (not target!) JDK requirement to
>> Java 17 for all platforms to work around this problem. This said, I am
>> confused on why we still need Maven toolchains in `master`. Shouldn't we
>> simply remove all Maven toolchains configuration and configure
>> maven-compiler-plugin and maven-enforcer plugin to require 17 for build and
>> target 11?
>>
>>> 2. In general the test jars have a second compilation step to create the
>> module-info.java. Unfortunately, the unit
>>> tests cannot be modularized because:
>>> a. the unit tests open the main module (i.e. they use the same
>> package names).
>>> b. the test jar requires the main module.
>>> c. the unit tests require the test module.
>>> This creates a circularity because the unit tests depend on the test
>> module and it requires the same packages
>>> as the unit tests.
>>
>> Will this still pose a problem if we move all tests to their own module?
>>
>>> 3. I tried building entirely with Java 17 but there were some unit tests
>> in log4j-core-test that fail. There is one class
>>> that is trying to modify a final variable in the Constant class and
>> it seems that the technique it is using is no
>>> longer valid.
>>> 4. log4j-jul must use Junit 4. It seems Junit 5 is hard-wired to use
>> java.util.logging and so it isn’t possible to set the
>>> system property in time to have the unit tests use Log4j’s LogManager.
>>
>> Do we have tickets for these two?
>>
>>> 5. JsonTemplateLayout is still creating a test-jar. I haven’t validated
>> if anything needs it. If it does than it will need to
>>> be split to have a -test module as well.
>>
>> JTL test-jar is used by log4j-perf.
>> I have created LOG4J2-3308
>> <https://issues.apache.org/jira/browse/LOG4J2-3308> to address this.
>>
>>> 6. I split the annotation processor into its own module -
>> log4j-plugin-processor. Note that with modular compiles
>>> annotation processors are no longer added automatically so this
>> should be clearer.
>>> 7. I have not added module-info.java files to anything beyond log4j-api,
>> log4j-plugins, and log4j-core. That will be
>>> coming next.
>>> 8. log4j-api-test has a few tests that seem to randomly fail. I didn’t
>> look into them.
>>
>> Again, great work Ralph!
>> Thanks so much for pushing `master` further.
>>
Re: Master branch
Posted by Gary Gregory <ga...@gmail.com>.
On Mon, Jan 3, 2022 at 7:30 AM Volkan Yazıcı <vo...@yazi.ci> wrote:
> Fantastic work Ralph! Please see my comments below:
>
> On Tue, Dec 28, 2021 at 9:39 AM Ralph Goers <ra...@dslextreme.com>
> wrote:
> > Most of the components that were generating test jars have been split
> into two modules - the main component,
> > which only builds the jar, and a -test component that builds a -test jar
> and then runs the unit tests.
>
> For completeness and consistency across all modules *and* to (hopefully)
> mitigate the problem you have mentioned in #2, I think we should place all
> tests to their own modules. Even though we all agree that this is ugly,
> IIRC, our most recent Maven and JPMS research pointed us in this direction.
>
> I also propose backporting every "moving tests to a module" change to
> `release-2.x`. Otherwise, it will be really difficult to synchronize
> changes on both branches in the future.
>
I agree with Volkan. Whatever nightmare of a mess JPMS is making in master,
it is no longer possible to cherry-pick what should be simple commits from
release-2.x to master. Patches/diff files are not even possible, it is all
manual, this is a f*g PITA for both development and maintenance as I am
currently experiencing :-( This is not a good use of time.
Frustrated,
Gary
>
> > 1. log4j-core uses toolchains on MacOS to use Java 17. There is a bug in
> javac on Macs that causes compilation
> > to fail. It is fixed in Java 17 but was not back-ported to Java 11.
>
> For one, I am fine if we set the build (not target!) JDK requirement to
> Java 17 for all platforms to work around this problem. This said, I am
> confused on why we still need Maven toolchains in `master`. Shouldn't we
> simply remove all Maven toolchains configuration and configure
> maven-compiler-plugin and maven-enforcer plugin to require 17 for build and
> target 11?
>
> > 2. In general the test jars have a second compilation step to create the
> module-info.java. Unfortunately, the unit
> > tests cannot be modularized because:
> > a. the unit tests open the main module (i.e. they use the same
> package names).
> > b. the test jar requires the main module.
> > c. the unit tests require the test module.
> > This creates a circularity because the unit tests depend on the test
> module and it requires the same packages
> > as the unit tests.
>
> Will this still pose a problem if we move all tests to their own module?
>
> > 3. I tried building entirely with Java 17 but there were some unit tests
> in log4j-core-test that fail. There is one class
> > that is trying to modify a final variable in the Constant class and
> it seems that the technique it is using is no
> > longer valid.
> > 4. log4j-jul must use Junit 4. It seems Junit 5 is hard-wired to use
> java.util.logging and so it isn’t possible to set the
> > system property in time to have the unit tests use Log4j’s LogManager.
>
> Do we have tickets for these two?
>
> > 5. JsonTemplateLayout is still creating a test-jar. I haven’t validated
> if anything needs it. If it does than it will need to
> > be split to have a -test module as well.
>
> JTL test-jar is used by log4j-perf.
> I have created LOG4J2-3308
> <https://issues.apache.org/jira/browse/LOG4J2-3308> to address this.
>
> > 6. I split the annotation processor into its own module -
> log4j-plugin-processor. Note that with modular compiles
> > annotation processors are no longer added automatically so this
> should be clearer.
> > 7. I have not added module-info.java files to anything beyond log4j-api,
> log4j-plugins, and log4j-core. That will be
> > coming next.
> > 8. log4j-api-test has a few tests that seem to randomly fail. I didn’t
> look into them.
>
> Again, great work Ralph!
> Thanks so much for pushing `master` further.
>
Re: Master branch
Posted by Matt Sicker <bo...@gmail.com>.
There’s already a ticket for JUnit 5 I filed a while ago.
—
Matt Sicker
> On Jan 3, 2022, at 10:28, Ralph Goers <ra...@dslextreme.com> wrote:
>
>
>
>> On Jan 3, 2022, at 5:30 AM, Volkan Yazıcı <vo...@yazi.ci> wrote:
>>
>> Fantastic work Ralph! Please see my comments below:
>>
>>> On Tue, Dec 28, 2021 at 9:39 AM Ralph Goers <ra...@dslextreme.com>
>>> wrote:
>>> Most of the components that were generating test jars have been split
>> into two modules - the main component,
>>> which only builds the jar, and a -test component that builds a -test jar
>> and then runs the unit tests.
>>
>> For completeness and consistency across all modules *and* to (hopefully)
>> mitigate the problem you have mentioned in #2, I think we should place all
>> tests to their own modules. Even though we all agree that this is ugly,
>> IIRC, our most recent Maven and JPMS research pointed us in this direction.
>
> I would recommend against this for the following reasons:
> 1. It is very clear which modules create test jars if they are the only ones that
> have a Maven test module.
> 2. Having a Maven module that has nothing in src/main means you don’t have
> an artifact. This module would have to be declared as type “pom” and skip the
> deploy step.
> 3. When the source and unit tests reside in the same Maven module it seems
> no module-info.java is required for the tests and everything can be tested on
> the module path as a JPMS module. Maven determines whether to populate
> the module path and test as a module based on the presence of a module-info.java
> in src/main. If the project has no source I don’t believe it will test it as a JPMS
> module, which is something we very much want to do.
>
>>
>> I also propose backporting every "moving tests to a module" change to
>> `release-2.x`. Otherwise, it will be really difficult to synchronize
>> changes on both branches in the future.
>
> To be honest, I haven’t been able to cherry-pick much of anything across the
> branches in quite a while. At this point with the work Matt has been doing on
> dependency injection and splitting things out into separate modules almost
> everything has problems with cherry-picking.
>
>>
>>> 1. log4j-core uses toolchains on MacOS to use Java 17. There is a bug in
>> javac on Macs that causes compilation
>>> to fail. It is fixed in Java 17 but was not back-ported to Java 11.
>>
>> For one, I am fine if we set the build (not target!) JDK requirement to
>> Java 17 for all platforms to work around this problem. This said, I am
>> confused on why we still need Maven toolchains in `master`. Shouldn't we
>> simply remove all Maven toolchains configuration and configure
>> maven-compiler-plugin and maven-enforcer plugin to require 17 for build and
>> target 11?
>
> The default for the build is Java 11. You can’t simply switch the Java version of
> the compilerin the middle of the build without using toolchains.
>
>>
>>> 2. In general the test jars have a second compilation step to create the
>> module-info.java. Unfortunately, the unit
>>> tests cannot be modularized because:
>>> a. the unit tests open the main module (i.e. they use the same
>> package names).
>>> b. the test jar requires the main module.
>>> c. the unit tests require the test module.
>>> This creates a circularity because the unit tests depend on the test
>> module and it requires the same packages
>>> as the unit tests.
>>
>> Will this still pose a problem if we move all tests to their own module?
>
> See my answer above as to why moving all tests to their own module is
> a bad idea.
>>
>>> 3. I tried building entirely with Java 17 but there were some unit tests
>> in log4j-core-test that fail. There is one class
>>> that is trying to modify a final variable in the Constant class and
>> it seems that the technique it is using is no
>>> longer valid.
>>> 4. log4j-jul must use Junit 4. It seems Junit 5 is hard-wired to use
>> java.util.logging and so it isn’t possible to set the
>>> system property in time to have the unit tests use Log4j’s LogManager.
>>
>> Do we have tickets for these two?
>
> I could create a ticket for Junit 5 but I don’t see the point. At this point it
> would be a great deal of effort for them to fix it just for us.
>
> Ralph
>
Re: Master branch
Posted by Ralph Goers <ra...@dslextreme.com>.
> On Jan 3, 2022, at 1:47 PM, Volkan Yazıcı <vo...@yazi.ci> wrote:
>>
>
> Sorry, I think I have been a victim of my own incorrect wording. I rather
> wanted to suggest "placing all tests _exposed as test JARs_ to their own
> modules". I think we both agree on this. Please, correct me if I am wrong.
There is no choice here. The vast majority of unit tests are dependent on the code in the test jar. So you end up with:
main
test jar (requires main)
unit tests (requires main and test jar classes).
Only the classes that twill be consumed by downstream modules are placed in the test jar.
Unit tests must run from classes in src/test/java but those classes may extend or use classes from the test jar.
I’m still not sure if we are saying the same thing or if you have something else in mind.
>>
>>>> 1. log4j-core uses toolchains on MacOS to use Java 17. There is a bug in
>>> javac on Macs that causes compilation
>>>> to fail. It is fixed in Java 17 but was not back-ported to Java 11.
>>>
>>> For one, I am fine if we set the build (not target!) JDK requirement to
>>> Java 17 for all platforms to work around this problem. This said, I am
>>> confused on why we still need Maven toolchains in `master`. Shouldn't we
>>> simply remove all Maven toolchains configuration and configure
>>> maven-compiler-plugin and maven-enforcer plugin to require 17 for build
>> and
>>> target 11?
>>
>> The default for the build is Java 11. You can’t simply switch the Java
>> version of
>> the compilerin the middle of the build without using toolchains.
>>
>
> I don't get it, why do we need to switch the Java version of the compiler
> in the middle of the build? I thought this is all we need:
>
> <plugin>
> <groupId>org.apache.maven.plugins</groupId>
> <artifactId>maven-compiler-plugin</artifactId>
> <configuration>
> <release>11</release>
> </configuration>
> </plugin>
>
> Doesn't this enable us to use any JDK >= 11 and still target Java 11? That
> is, one can still use Java 17 and yet produce Java 11 bytecode.
That setting doesn’t enforce that Java 17 be used. It only says that the code
must be built and tested for Java 11.
You cannot build everything with Java 17. There are at least 2 unit tests that
fail it you try as they use stuff that is no longer allowed in Java 17. I wasn’t in
the mood to try to rewrite those tests.
>
>
>>>> 2. In general the test jars have a second compilation step to create the
>>> module-info.java. Unfortunately, the unit
>>>> tests cannot be modularized because:
>>>> a. the unit tests open the main module (i.e. they use the same
>>> package names).
>>>> b. the test jar requires the main module.
>>>> c. the unit tests require the test module.
>>>> This creates a circularity because the unit tests depend on the test
>>> module and it requires the same packages
>>>> as the unit tests.
>>>
>>> Will this still pose a problem if we move all tests to their own module?
>>
>> See my answer above as to why moving all tests to their own module is
>> a bad idea.
>>
>
> I will slightly change my question: if we move only tests exposed as
> test-JARs to their own module, does this still pose a problem?
I still don’t understand. The test jars DO NOT contain any tests. They contain
classes that support testing like LoggerContextRule that need to be shared with
other modules that need to perform unit testing.
For the -test projects the src/main directories contain everything that will be included
in the test jar while the src/test directories contain all the unit tests.
One very important thing to note. The classes in src/main CANNOT overlap the
package space of the main jar. Both of these will be packaged as JPMS modules.
The convention here is that for a module named org.apache.logging.log4j.foo its
test jar, if it builds one, would be org.apache.logging.log4j.foo.test. All classes in
that test jar would in that package or subpackaages of it.
The unit tests still use the package space of the main component that they are
testing (org.apache.logging.log4j.foo).
>
>
>>>> 3. I tried building entirely with Java 17 but there were some unit tests
>>> in log4j-core-test that fail. There is one class
>>>> that is trying to modify a final variable in the Constant class and
>>> it seems that the technique it is using is no
>>>> longer valid.
>>>> 4. log4j-jul must use Junit 4. It seems Junit 5 is hard-wired to use
>>> java.util.logging and so it isn’t possible to set the
>>>> system property in time to have the unit tests use Log4j’s LogManager.
>>>
>>> Do we have tickets for these two?
>>
>> I could create a ticket for Junit 5 but I don’t see the point. At this
>> point it
>> would be a great deal of effort for them to fix it just for us.
>>
>
> I meant creating LOG4J2 tickets for #3 and #4 you have mentioned. They look
> like pretty isolated problems so people can pick them up.
I haven’t created Jira issues for these. For item 3 it was simpler to just use Java 11
and it didn’t bother me enough that I felt like I needed to do anything about it.
Item 4 isn’t our problem. There is nothing we can do about it. Once Junit 5 calls j
ava.util.logging there is no way to change the LogManager implementation. In an
ideal world java.util.logging would be modified to use ServiceLoader, in which
case we could hook in simply by configuration.
Ralph
Re: Master branch
Posted by Volkan Yazıcı <vo...@yazi.ci>.
On Mon, Jan 3, 2022 at 5:28 PM Ralph Goers <ra...@dslextreme.com>
wrote:
>
>
> > On Jan 3, 2022, at 5:30 AM, Volkan Yazıcı <vo...@yazi.ci> wrote:
> >
> > Fantastic work Ralph! Please see my comments below:
> >
> > On Tue, Dec 28, 2021 at 9:39 AM Ralph Goers <ra...@dslextreme.com>
> > wrote:
> >> Most of the components that were generating test jars have been split
> > into two modules - the main component,
> >> which only builds the jar, and a -test component that builds a -test jar
> > and then runs the unit tests.
> >
> > For completeness and consistency across all modules *and* to (hopefully)
> > mitigate the problem you have mentioned in #2, I think we should place
> all
> > tests to their own modules. Even though we all agree that this is ugly,
> > IIRC, our most recent Maven and JPMS research pointed us in this
> direction.
>
> I would recommend against this for the following reasons:
> 1. It is very clear which modules create test jars if they are the only
> ones that
> have a Maven test module.
> 2. Having a Maven module that has nothing in src/main means you don’t have
> an artifact. This module would have to be declared as type “pom” and skip
> the
> deploy step.
> 3. When the source and unit tests reside in the same Maven module it seems
> no module-info.java is required for the tests and everything can be tested
> on
> the module path as a JPMS module. Maven determines whether to populate
> the module path and test as a module based on the presence of a
> module-info.java
> in src/main. If the project has no source I don’t believe it will test it
> as a JPMS
> module, which is something we very much want to do.
>
Sorry, I think I have been a victim of my own incorrect wording. I rather
wanted to suggest "placing all tests _exposed as test JARs_ to their own
modules". I think we both agree on this. Please, correct me if I am wrong.
> > I also propose backporting every "moving tests to a module" change to
> > `release-2.x`. Otherwise, it will be really difficult to synchronize
> > changes on both branches in the future.
>
> To be honest, I haven’t been able to cherry-pick much of anything across
> the
> branches in quite a while. At this point with the work Matt has been doing
> on
> dependency injection and splitting things out into separate modules almost
> everything has problems with cherry-picking.
>
Hrm... Sadly, what you say makes sense.
>
> >> 1. log4j-core uses toolchains on MacOS to use Java 17. There is a bug in
> > javac on Macs that causes compilation
> >> to fail. It is fixed in Java 17 but was not back-ported to Java 11.
> >
> > For one, I am fine if we set the build (not target!) JDK requirement to
> > Java 17 for all platforms to work around this problem. This said, I am
> > confused on why we still need Maven toolchains in `master`. Shouldn't we
> > simply remove all Maven toolchains configuration and configure
> > maven-compiler-plugin and maven-enforcer plugin to require 17 for build
> and
> > target 11?
>
> The default for the build is Java 11. You can’t simply switch the Java
> version of
> the compilerin the middle of the build without using toolchains.
>
I don't get it, why do we need to switch the Java version of the compiler
in the middle of the build? I thought this is all we need:
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<release>11</release>
</configuration>
</plugin>
Doesn't this enable us to use any JDK >= 11 and still target Java 11? That
is, one can still use Java 17 and yet produce Java 11 bytecode.
> >> 2. In general the test jars have a second compilation step to create the
> > module-info.java. Unfortunately, the unit
> >> tests cannot be modularized because:
> >> a. the unit tests open the main module (i.e. they use the same
> > package names).
> >> b. the test jar requires the main module.
> >> c. the unit tests require the test module.
> >> This creates a circularity because the unit tests depend on the test
> > module and it requires the same packages
> >> as the unit tests.
> >
> > Will this still pose a problem if we move all tests to their own module?
>
> See my answer above as to why moving all tests to their own module is
> a bad idea.
>
I will slightly change my question: if we move only tests exposed as
test-JARs to their own module, does this still pose a problem?
> >> 3. I tried building entirely with Java 17 but there were some unit tests
> > in log4j-core-test that fail. There is one class
> >> that is trying to modify a final variable in the Constant class and
> > it seems that the technique it is using is no
> >> longer valid.
> >> 4. log4j-jul must use Junit 4. It seems Junit 5 is hard-wired to use
> > java.util.logging and so it isn’t possible to set the
> >> system property in time to have the unit tests use Log4j’s LogManager.
> >
> > Do we have tickets for these two?
>
> I could create a ticket for Junit 5 but I don’t see the point. At this
> point it
> would be a great deal of effort for them to fix it just for us.
>
I meant creating LOG4J2 tickets for #3 and #4 you have mentioned. They look
like pretty isolated problems so people can pick them up.
Re: Master branch
Posted by Ralph Goers <ra...@dslextreme.com>.
> On Jan 3, 2022, at 5:30 AM, Volkan Yazıcı <vo...@yazi.ci> wrote:
>
> Fantastic work Ralph! Please see my comments below:
>
> On Tue, Dec 28, 2021 at 9:39 AM Ralph Goers <ra...@dslextreme.com>
> wrote:
>> Most of the components that were generating test jars have been split
> into two modules - the main component,
>> which only builds the jar, and a -test component that builds a -test jar
> and then runs the unit tests.
>
> For completeness and consistency across all modules *and* to (hopefully)
> mitigate the problem you have mentioned in #2, I think we should place all
> tests to their own modules. Even though we all agree that this is ugly,
> IIRC, our most recent Maven and JPMS research pointed us in this direction.
I would recommend against this for the following reasons:
1. It is very clear which modules create test jars if they are the only ones that
have a Maven test module.
2. Having a Maven module that has nothing in src/main means you don’t have
an artifact. This module would have to be declared as type “pom” and skip the
deploy step.
3. When the source and unit tests reside in the same Maven module it seems
no module-info.java is required for the tests and everything can be tested on
the module path as a JPMS module. Maven determines whether to populate
the module path and test as a module based on the presence of a module-info.java
in src/main. If the project has no source I don’t believe it will test it as a JPMS
module, which is something we very much want to do.
>
> I also propose backporting every "moving tests to a module" change to
> `release-2.x`. Otherwise, it will be really difficult to synchronize
> changes on both branches in the future.
To be honest, I haven’t been able to cherry-pick much of anything across the
branches in quite a while. At this point with the work Matt has been doing on
dependency injection and splitting things out into separate modules almost
everything has problems with cherry-picking.
>
>> 1. log4j-core uses toolchains on MacOS to use Java 17. There is a bug in
> javac on Macs that causes compilation
>> to fail. It is fixed in Java 17 but was not back-ported to Java 11.
>
> For one, I am fine if we set the build (not target!) JDK requirement to
> Java 17 for all platforms to work around this problem. This said, I am
> confused on why we still need Maven toolchains in `master`. Shouldn't we
> simply remove all Maven toolchains configuration and configure
> maven-compiler-plugin and maven-enforcer plugin to require 17 for build and
> target 11?
The default for the build is Java 11. You can’t simply switch the Java version of
the compilerin the middle of the build without using toolchains.
>
>> 2. In general the test jars have a second compilation step to create the
> module-info.java. Unfortunately, the unit
>> tests cannot be modularized because:
>> a. the unit tests open the main module (i.e. they use the same
> package names).
>> b. the test jar requires the main module.
>> c. the unit tests require the test module.
>> This creates a circularity because the unit tests depend on the test
> module and it requires the same packages
>> as the unit tests.
>
> Will this still pose a problem if we move all tests to their own module?
See my answer above as to why moving all tests to their own module is
a bad idea.
>
>> 3. I tried building entirely with Java 17 but there were some unit tests
> in log4j-core-test that fail. There is one class
>> that is trying to modify a final variable in the Constant class and
> it seems that the technique it is using is no
>> longer valid.
>> 4. log4j-jul must use Junit 4. It seems Junit 5 is hard-wired to use
> java.util.logging and so it isn’t possible to set the
>> system property in time to have the unit tests use Log4j’s LogManager.
>
> Do we have tickets for these two?
I could create a ticket for Junit 5 but I don’t see the point. At this point it
would be a great deal of effort for them to fix it just for us.
Ralph