> On Jan 3, 2022, at 1:47 PM, Volkan Yazıcı <[email protected]> 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
