From 79e44360cd95b14fa255daaac069f9b76c488451 Mon Sep 17 00:00:00 2001 From: Alexander Kriegisch Date: Tue, 23 Mar 2021 16:59:17 +0700 Subject: [PATCH] Make all tests run on Java 16 via '-add-opens' JVM option Due to JEP 260 (Encapsulate Most Internal APIs), aspect weaving on Java 16 now requires '--add-opens java.base/java.lang=ALL-UNNAMED' on the command line. Otherwise there will be illegal access exceptions for some internal API calls AspectJ needs, most prominently when trying to define classes in other packages or modules. This had to be done on several levels: - Maven Surefire: running tests in a JVM directly forked by Surefire. In order to make this backwards compatible, I added two profiles with JDK-level-dependent auto-activation, one 8-15 and one 16+. In the latter a property containing the JVM parameter is defined, in the former it is empty, i.e. the JVM is started without the parameter. In Java 8 the parameter did not even exist, in Java 9+ we could use it, but we need to test how users use AspectJ. - RunSpec: Whenever an XML test is declared to use '', we need to determine the current JVM version and again dynamically add the parameter when forking the target JVM. - AntSpec: Whenever an XML test is declared to use '', we need to determine the current JVM version dynamically add two properties usable from within Ant scripts: 'aj.addOpensKey' and 'aj.addOpensValue'. Unfortunately, Ant needs to use two '' parameters, because the two parts of the option are separated by a space character. - Ant scripts: When triggered by an AntSpec, each Ant target using LTW needs to manually set for each '' task. It was quite tedious to find all(?) of them. TODO: In the AspectJ 1.9.7 release notes we need to document that this parameter is now needed for LTW. Signed-off-by: Alexander Kriegisch --- pom.xml | 76 ++++++++-- run-all-junit-tests/pom.xml | 2 + testing-drivers/pom.xml | 46 +++--- .../java/org/aspectj/testing/AntSpec.java | 41 ++++-- .../java/org/aspectj/testing/RunSpec.java | 9 ++ tests/bugs153/pr155033/ant.xml | 6 +- tests/bugs153/pr157474/ant-server.xml | 6 +- tests/bugs153/pr158957/ant.xml | 2 + tests/java5/ataspectj/ajc-ant.xml | 50 ++++++- tests/ltw/ant-server.xml | 16 +- tests/ltw/ant.xml | 54 ++++--- tests/pom.xml | 3 +- tests/profiling/build.xml | 138 +++++++++--------- .../systemtest/ajc167/Ajc167Tests.java | 16 +- tests/tracing/ant.xml | 12 +- 15 files changed, 315 insertions(+), 162 deletions(-) diff --git a/pom.xml b/pom.xml index 16b3f2ba1..14952e6a7 100644 --- a/pom.xml +++ b/pom.xml @@ -50,7 +50,35 @@ installer + + + + jdk-8-to-15 + + [1.8,16) + + + + + + + jdk-16-to-xx + + [16,) + + + --add-opens java.base/java.lang=ALL-UNNAMED + + + + + @@ -66,16 +94,21 @@ maven-surefire-plugin 2.22.2 - + + ${jvm.arg.addOpens} **/*ModuleTests.java + + org.apache.maven.plugins + maven-help-plugin + 3.2.0 + + org.apache.maven.plugins @@ -88,7 +121,7 @@ -test-jar + test-jar test-jar @@ -104,25 +137,44 @@ . -**/ajcore.*.txt - - + **/ajcore.*.txt + + + **/important.log + **/another-important.log + +--> false aj-build - + false + + + org.apache.maven.plugins + maven-help-plugin + + diff --git a/run-all-junit-tests/pom.xml b/run-all-junit-tests/pom.xml index bcb21ca4e..3f63ea4d2 100644 --- a/run-all-junit-tests/pom.xml +++ b/run-all-junit-tests/pom.xml @@ -288,6 +288,7 @@ maven-surefire-plugin true + ${jvm.arg.addOpens} @@ -311,6 +312,7 @@ maven-surefire-plugin true + ${jvm.arg.addOpens} diff --git a/testing-drivers/pom.xml b/testing-drivers/pom.xml index 26de8eab9..f3ac5fa36 100644 --- a/testing-drivers/pom.xml +++ b/testing-drivers/pom.xml @@ -71,27 +71,27 @@ - - - - org.apache.maven.plugins - maven-compiler-plugin - - 1.8 - 1.8 - - - - org.apache.maven.plugins - maven-surefire-plugin - - false - -**/*AjcTestSpecAsTest* - - - - - - + + + org.apache.maven.plugins + maven-compiler-plugin + + 1.8 + 1.8 + + + + org.apache.maven.plugins + maven-surefire-plugin + + false + ${jvm.arg.addOpens} + + **/*AjcTestSpecAsTest* + + + + + + diff --git a/testing/src/test/java/org/aspectj/testing/AntSpec.java b/testing/src/test/java/org/aspectj/testing/AntSpec.java index 92ea21877..b87f60894 100644 --- a/testing/src/test/java/org/aspectj/testing/AntSpec.java +++ b/testing/src/test/java/org/aspectj/testing/AntSpec.java @@ -26,21 +26,33 @@ import org.apache.tools.ant.taskdefs.Java; import org.apache.tools.ant.types.Path; import org.aspectj.tools.ajc.AjcTestCase; +import static org.aspectj.util.LangUtil.is16VMOrGreater; + /** * Element that allow to run an abritrary Ant target in a sandbox. *

- * Such a spec is used in a " XML element. The "target" is - * optional. If not set, default myAnt.xml target is used. The "file" file is looked up from the attribute. If - * "verbose" is set to "true", the ant -v output is piped, else nothing is reported except errors. + * Such a spec is used in a {@code } XML element. The + * {@code target} is optional. If not set, default myAnt.xml target is used. The {@code file} is looked up from + * the {@code } attribute. If @{code verbose} is set to {@code true}, the {@code ant -v} output is + * piped, else nothing is reported except errors. *

- * The called Ant target benefits from 2 implicit variables: "${aj.sandbox}" points to the test current sandbox folder. "aj.path" is - * an Ant refid on the classpath formed with the sandbox folder + ltw + the AjcTestCase classpath (ie usually aspectjrt, junit, and - * testing infra) + * The called Ant target benefits from some implicit variables: + *

    + *
  • {@code ${aj.sandbox}} points to the test current sandbox folder.
    • + *
  • + * {@code ${aj.path}} is an Ant refid on the classpath formed with the sandbox folder + ltw + the AjcTestCase + * classpath (i.e. usually aspectjrt, junit, and testing infra). + *
    • + *
  • + * For Java 16+, {@code ${aj.addOpensKey}} and {@code ${aj.addOpensValue}} together add {@code --add-opens} and + * {@code java.base/java.lang=ALL-UNNAMED} as JVM parameters. They have to be used together and consecutively in + * this order as {@code jvmarg} parameter tags inside the {@code java} Ant task. + *
    • + *
*

- * Element "" and "" can be used. For now a full match is performed on the output of - * the runned target only (not the whole Ant invocation). This is experimental and advised to use a "" task instead or a - * "" whose main that throws some exception upon failure. - * + * Element {@code } and {@code } can be used. For now, a full match is + * performed on the output of the runned target only (not the whole Ant invocation). This is experimental and you are + * advised to use a {@code } task instead or a {@code } whose main throws some exception upon failure. * * @author Alexandre Vasseur */ @@ -92,6 +104,15 @@ public class AntSpec implements ITestStep { // setup aj.dir "modules" folder p.setUserProperty("aj.root", new File("..").getAbsolutePath()); + // On Java 16+, LTW no longer works without this parameter. Add the argument here and not in AjcTestCase::run, + // because even if 'useLTW' and 'useFullLTW' are not set, we might in the future have tests for weaver attachment + // during runtime. See also docs/dist/doc/README-187.html. + // + // Attention: Ant 1.6.3 under Linux neither likes "" (empty string) nor " " (space), on Windows it would not be + // a problem. So we use "_dummy" Java system properties, even though they pollute the command line. + p.setUserProperty("aj.addOpensKey", is16VMOrGreater() ? "--add-opens" : "-D_dummy"); + p.setUserProperty("aj.addOpensValue", is16VMOrGreater() ? "java.base/java.lang=ALL-UNNAMED" : "-D_dummy"); + // create the test implicit path aj.path that contains the sandbox + regular test infra path Path path = new Path(p, inTestCase.getSandboxDirectory().getAbsolutePath()); populatePath(path, DEFAULT_LTW_CLASSPATH_ENTRIES); diff --git a/testing/src/test/java/org/aspectj/testing/RunSpec.java b/testing/src/test/java/org/aspectj/testing/RunSpec.java index 2fc786593..f4dc98206 100644 --- a/testing/src/test/java/org/aspectj/testing/RunSpec.java +++ b/testing/src/test/java/org/aspectj/testing/RunSpec.java @@ -22,6 +22,8 @@ import java.util.StringTokenizer; import org.aspectj.tools.ajc.AjcTestCase; import org.aspectj.util.FileUtil; +import static org.aspectj.util.LangUtil.is16VMOrGreater; + /** * @author Adrian Colyer */ @@ -63,6 +65,13 @@ public class RunSpec implements ITestStep { try { setSystemProperty("test.base.dir", inTestCase.getSandboxDirectory().getAbsolutePath()); + if (vmargs == null) + vmargs = ""; + // On Java 16+, LTW no longer works without this parameter. Add the argument here and not in AjcTestCase::run, + // because even if 'useLTW' and 'useFullLTW' are not set, we might in the future have tests for weaver attachment + // during runtime. See also docs/dist/doc/README-187.html. + vmargs += is16VMOrGreater() ? " --add-opens java.base/java.lang=ALL-UNNAMED" : ""; + AjcTestCase.RunResult rr = inTestCase.run(getClassToRun(), getModuleToRun(), args, vmargs, getClasspath(), getModulepath(), useLtw, "true".equalsIgnoreCase(usefullltw)); if (stdErrSpec != null) { diff --git a/tests/bugs153/pr155033/ant.xml b/tests/bugs153/pr155033/ant.xml index 5995cb9d5..4f2dfb885 100644 --- a/tests/bugs153/pr155033/ant.xml +++ b/tests/bugs153/pr155033/ant.xml @@ -15,12 +15,14 @@ + + - +--> diff --git a/tests/bugs153/pr157474/ant-server.xml b/tests/bugs153/pr157474/ant-server.xml index af315984a..73db721c9 100644 --- a/tests/bugs153/pr157474/ant-server.xml +++ b/tests/bugs153/pr157474/ant-server.xml @@ -9,16 +9,18 @@ - + - + + + diff --git a/tests/bugs153/pr158957/ant.xml b/tests/bugs153/pr158957/ant.xml index c2de686f1..ca5dbc254 100644 --- a/tests/bugs153/pr158957/ant.xml +++ b/tests/bugs153/pr158957/ant.xml @@ -20,6 +20,8 @@ + + diff --git a/tests/java5/ataspectj/ajc-ant.xml b/tests/java5/ataspectj/ajc-ant.xml index c3ac6bac8..f69cfd4dd 100644 --- a/tests/java5/ataspectj/ajc-ant.xml +++ b/tests/java5/ataspectj/ajc-ant.xml @@ -24,6 +24,8 @@ + + @@ -34,6 +36,8 @@ + + @@ -44,6 +48,8 @@ + + @@ -54,6 +60,8 @@ + + @@ -61,6 +69,8 @@ + + @@ -70,6 +80,8 @@ + + @@ -89,6 +101,8 @@ + + @@ -100,27 +114,33 @@ + + + + + + @@ -142,30 +162,36 @@ + + + + + + @@ -187,6 +213,8 @@ + + @@ -204,6 +232,8 @@ + + + + @@ -243,6 +275,8 @@ + + @@ -256,13 +290,13 @@ - + + - \ No newline at end of file + diff --git a/tests/ltw/ant-server.xml b/tests/ltw/ant-server.xml index 31baef28f..c5f143fea 100644 --- a/tests/ltw/ant-server.xml +++ b/tests/ltw/ant-server.xml @@ -14,9 +14,11 @@ - - - + + + + + @@ -28,9 +30,11 @@ - - - + + + + + diff --git a/tests/ltw/ant.xml b/tests/ltw/ant.xml index dff2071e7..0cae97a36 100644 --- a/tests/ltw/ant.xml +++ b/tests/ltw/ant.xml @@ -11,15 +11,17 @@ - - - +--> - - + + + @@ -27,22 +29,24 @@ - - - - +--> - - + + + + + + - - + + + @@ -82,15 +90,15 @@ - + - - +--> @@ -101,6 +109,8 @@ + + @@ -114,8 +124,10 @@ - + + + diff --git a/tests/pom.xml b/tests/pom.xml index 3abcdde51..01a890f36 100644 --- a/tests/pom.xml +++ b/tests/pom.xml @@ -131,20 +131,19 @@ - org.apache.maven.plugins maven-surefire-plugin false + ${jvm.arg.addOpens} **/TestsModuleTests* - diff --git a/tests/profiling/build.xml b/tests/profiling/build.xml index 67ce475a1..ccb4d8f82 100644 --- a/tests/profiling/build.xml +++ b/tests/profiling/build.xml @@ -2,12 +2,12 @@ @@ -16,14 +16,14 @@ - + - + @@ -43,13 +43,13 @@ - + - - @@ -57,14 +57,14 @@ - + - + - + @@ -83,7 +83,7 @@ - + @@ -105,33 +105,33 @@ - + - + - - - - + + + + - + - + - - - + + + - + - - + - + - + @@ -159,8 +159,8 @@ - - - + @@ -182,10 +182,10 @@ - + - - - + - + - + - + @@ -233,7 +233,7 @@ - + - + @@ -262,26 +262,26 @@ - + - + - + - + - + - + - + + + @@ -297,30 +299,32 @@ - + - + - + - + - + + + @@ -328,7 +332,7 @@ - + @@ -341,9 +345,9 @@ - + - + @@ -353,7 +357,7 @@ - + - + - + - + @@ -388,7 +392,7 @@ - + @@ -401,7 +405,7 @@ - + @@ -414,5 +418,5 @@ - - \ No newline at end of file + + diff --git a/tests/src/test/java/org/aspectj/systemtest/ajc167/Ajc167Tests.java b/tests/src/test/java/org/aspectj/systemtest/ajc167/Ajc167Tests.java index 3c8399e0c..a97708e50 100644 --- a/tests/src/test/java/org/aspectj/systemtest/ajc167/Ajc167Tests.java +++ b/tests/src/test/java/org/aspectj/systemtest/ajc167/Ajc167Tests.java @@ -10,9 +10,8 @@ *******************************************************************************/ package org.aspectj.systemtest.ajc167; -import org.aspectj.testing.XMLBasedAjcTestCase; - import junit.framework.Test; +import org.aspectj.testing.XMLBasedAjcTestCase; public class Ajc167Tests extends org.aspectj.testing.XMLBasedAjcTestCase { @@ -27,14 +26,17 @@ public class Ajc167Tests extends org.aspectj.testing.XMLBasedAjcTestCase { runTest("optimizing string anno value binding"); } + // FIXME: This test is flaky - can depend on machine load public void testOptimizingAnnotationBinding() { runTest("optimizing annotation binding"); } - // bit flakey - can depend on machine load - // public void testOptimizingAnnotationBindingPerfTest() { - // runTest("optimizing annotation binding - 2"); - // } + // FIXME: This test is flaky - can depend on machine load +/* + public void testOptimizingAnnotationBindingPerfTest() { + runTest("optimizing annotation binding - 2"); + } +*/ /* * public void testPerThisLTW_295092() { runTest("perthis ltw"); } @@ -84,4 +86,4 @@ public class Ajc167Tests extends org.aspectj.testing.XMLBasedAjcTestCase { return getClassResource("ajc167.xml"); } -} \ No newline at end of file +} diff --git a/tests/tracing/ant.xml b/tests/tracing/ant.xml index f41252dcc..6e0ddd78e 100644 --- a/tests/tracing/ant.xml +++ b/tests/tracing/ant.xml @@ -33,6 +33,8 @@ + + @@ -46,6 +48,8 @@ + + @@ -59,6 +63,8 @@ + + @@ -69,11 +75,13 @@ - + + + - + -- 2.39.5