Make all tests run on Java 16 via '-add-opens' JVM optionjava16-add-opens

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 '<run>', 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 '<ant>', 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 '<argLine>'
    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
      <jvmarg value="${aj.addOpensKey}"/>
      <jvmarg value="${aj.addOpensValue}"/>
    for each '<java>' 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 <Alexander@Kriegisch.name>

2 files changed, 40 insertions, 10 deletions

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.
  * <p/>
- * Such a spec is used in a "<ajc-test><ant file="myAnt.xml" [target="..."] [verbose="true"]/> XML element. The "target" is
- * optional. If not set, default myAnt.xml target is used. The "file" file is looked up from the <ajc-test dir="..."/> 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 <ajc-test><ant file="myAnt.xml" [target="..."] [verbose="true"]/>} XML element. The
+ * {@code target} is optional. If not set, default <i>myAnt.xml</i> target is used. The {@code file} is looked up from
+ * the {@code <ajc-test dir="..."/>} attribute. If @{code verbose} is set to {@code true}, the {@code ant -v} output is
+ * piped, else nothing is reported except errors.
  * <p/>
- * 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:
+ * <ul>
+ *   <li>{@code ${aj.sandbox}} points to the test current sandbox folder.</li>
+ *   <li>
+ *     {@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).
+ *   </li>
+ *   <li>
+ *     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.
+ *   </li>
+ * </ul>
  * <p/>
- * Element "<stdout><line text="..">" and "<stderr><line text="..">" 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 "<junit>" task instead or a
- * "<java>" whose main that throws some exception upon failure.
- * 
+ * Element {@code <stdout><line text="..">} and {@code <stderr><line text="..">} 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 <junit>} task instead or a {@code <java>} whose main throws some exception upon failure.
  * 
  * @author <a href="mailto:alex AT gnilux DOT com">Alexandre Vasseur</a>
  */
@@ -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) {