clean up, refactor, restructure, and fix unit tests

Author: Gubaer

build.gradle

@@ -67,6 +67,7 @@ def deriveJosmCompileVersion(releases) {
         ? project.property("plugin.josmCompileVersion")
         : null
     def josmCompileVersion
+    //noinspection GroovyFallthrough
     switch (value) {
         case null:
             josmCompileVersion = "latest"
@@ -159,11 +160,13 @@ File josmHomeForTests() {
     return new File(project.projectDir, "build/josm.home")
 }
 
+/**
+ * Run unit tests that only depend on the JDK, the plugin classes,
+ * and Mozilla Rhino. Runs on the current stock Java VM and JDK.
+ */
 test {
     useJUnitPlatform()
     environment "JOSM_SCRIPTING_PLUGIN_HOME", project.projectDir
-    environment "TEST_COMMONJS_MODULE_REPO",
-            "${project.projectDir}/src/test/resources/require"
     environment "BUILTIN_COMMONJS_MODULE_REPO",
             "${project.projectDir}/src/main/javascript/v2"
     systemProperty "josm.home", josmHomeForTests().absolutePath
@@ -175,6 +178,10 @@ test {
     testLogging.events TestLogEvent.FAILED, TestLogEvent.PASSED
 }
 
+/**
+ * Runs the API V1 JavaScript unit tests with JOSM and Mozilla
+ * Rhino present.
+ */
 task testScriptApiV1 (type: Test) {
     useJUnitPlatform()
     environment "JOSM_SCRIPTING_PLUGIN_HOME", project.projectDir
@@ -192,30 +199,39 @@ def getGraalVMHome() {
     }
     final version = project.property("graalvm.version")
     if (!project.hasProperty("graalvm.jdk")) {
-        throw new GradleException("Missing mandatory project property 'jdk'")
+        throw new GradleException("Missing mandatory project property 'graalvm.jdk'")
     }
     final jdk = project.property("graalvm.jdk")
     return new File(project.projectDir, "software/graalvm-ce-$jdk-$version")
 }
 
+/**
+ * Runs the API V2 JavaScript unit tests with JOSM and GraalJS
+ * present. The tests are run with with a GraalVM 'java' binary.
+ */
 task testScriptApiV2(type: Test) {
     dependsOn "downloadGraalVM"
     useJUnitPlatform()
-    testLogging {
-        outputs.upToDateWhen {false}
-        showStandardStreams = true
-    }
     environment "JOSM_SCRIPTING_PLUGIN_HOME", project.projectDir
+    environment "TEST_COMMONJS_MODULE_REPO",
+            "${project.projectDir}/src/test/resources/require/modules"
     executable new File(getGraalVMHome(), "bin/java").absolutePath
     environment "JAVA_HOME", getGraalVMHome().absolutePath
     scanForTestClasses= false
     include "org/openstreetmap/josm/plugins/scripting/graalvm/api/*.class"
     testLogging {
+        outputs.upToDateWhen {false}
+        showStandardStreams = true
         events TestLogEvent.FAILED, TestLogEvent.PASSED
         exceptionFormat = TestExceptionFormat.FULL
     }
 }
 
+/**
+ * Runs tests for functionality that tests whether GraalJS is
+ * present or not. Runs with the current stock Java VM and JDK and makes
+ * sure that GraalJS/GraalVM isn't present on the classpath.
+ */
 task testGraalVMSupportNotPresent(type: Test) {
     // remove GraalVM from classpath under test
     classpath = classpath.filter {! it.toString().toLowerCase().contains("graalvm")}
@@ -228,54 +244,47 @@ task testGraalVMSupportNotPresent(type: Test) {
     }
 }
 
+/**
+ * Run unit tests for GraalJS functionality. Run it with the GraalVM java
+ * executable.
+ */
 import java.nio.file.Paths
 import java.nio.file.StandardCopyOption
-task testGraalVMSupportPresent(type: Test) {
+task testWithGraalVMJDK(type: Test) {
+    dependsOn "downloadGraalVM"
     doFirst {
         // prepare a suitable 'preferences.xml' for the test cases in this
         // executed by this task
         def josmHome = josmHomeForTests()
         josmHome.mkdirs()
         Files.copy(
             Paths.get("${project.projectDir}/src/test/resources/josm-test-environments/env01/preferences.xml"),
-            Paths.get("${josmHome.absolutePath}preferences.xml"),
+            Paths.get("${josmHome.absolutePath}/preferences.xml"),
             StandardCopyOption.REPLACE_EXISTING
         )
-        println("testGraalVMSupportPresent: prepared preferences.xml")
     }
     useJUnitPlatform()
-    environment "JOSM_SCRIPTING_PLUGIN_HOME",project.projectDir
+    environment "JOSM_SCRIPTING_PLUGIN_HOME", project.projectDir
+    environment "TEST_COMMONJS_MODULE_REPO",
+            "${project.projectDir}/src/test/resources/require/modules"
     systemProperty "josm.home", josmHomeForTests().absolutePath
-    scanForTestClasses= false
-    include "**/graalvm/GraalVMPresentTest.class"
-    include "**/graalvm/GraalVMEvalTest.class"
-    include "**/graalvm/GraalVMEmbeddedInJOSMTest.class"
-    include "**/graalvm/GraalVMAndRequireTest.class"
-    testLogging {
-        events TestLogEvent.FAILED, TestLogEvent.PASSED
-        exceptionFormat = TestExceptionFormat.FULL
-    }
-}
 
-task testWithGraalVMJDK(type: Test) {
-    dependsOn "downloadGraalVM"
-    useJUnitPlatform()
-    environment "JOSM_SCRIPTING_PLUGIN_HOME", project.projectDir
     scanForTestClasses= false
+    // configuration to use GraalVM
     executable new File(getGraalVMHome(), "bin/java").absolutePath
     environment "JAVA_HOME", getGraalVMHome()
-
-    include "**/graalvm/GraalVMPresentTest.class"
-    include "**/graalvm/GraalVMEvalTest.class"
-    include "**/graalvm/GraalVMEmbeddedInJOSMTest.class"
-    include "**/graalvm/GraalVMAndRequireTest.class"
-    include "**/graalvm/ScriptingAPIV2Test.class"
+    scanForTestClasses= false
+    include "org/openstreetmap/josm/plugins/scripting/graalvm/with_graalvm/*.class"
     testLogging {
         events TestLogEvent.FAILED, TestLogEvent.PASSED
         exceptionFormat = TestExceptionFormat.FULL
     }
 }
 
+/**
+ * Run tests on a stock JDK with the groovy and python scripting
+ * engines present.
+ */
 task testJSR223EnginePresent(type: Test) {
     useJUnitPlatform()
 
@@ -291,7 +300,6 @@ task testJSR223EnginePresent(type: Test) {
 tasks.check.dependsOn(tasks.testScriptApiV1)
 tasks.check.dependsOn(tasks.testScriptApiV2)
 tasks.check.dependsOn(tasks.testGraalVMSupportNotPresent)
-tasks.check.dependsOn(tasks.testGraalVMSupportPresent)
 tasks.check.dependsOn(tasks.testJSR223EnginePresent)
 tasks.check.dependsOn(tasks.testWithGraalVMJDK)
 

src/main/java/org/openstreetmap/josm/plugins/scripting/graalvm/CommonJSModuleRepositoryRegistry.java

@@ -199,9 +199,9 @@ public void clear() {
     }
 
     private void logFine(Supplier<String> messageBuilder) {
-        if (logger.isLoggable(Level.FINE)) {
+        if (logger.isLoggable(Level.INFO)) {
             final String message = messageBuilder.get();
-            logger.log(Level.FINE, message);
+            logger.log(Level.INFO, message);
         }
     }
 

src/main/java/org/openstreetmap/josm/plugins/scripting/graalvm/FileSystemJSModuleRepository.java

@@ -38,7 +38,7 @@ protected boolean isRepoFile(String repoPath) {
      * @param baseDir the base directory. Must not be null. Must not
      *                be empty.
      */
-    FileSystemJSModuleRepository(@NotNull String baseDir) {
+    public FileSystemJSModuleRepository(@NotNull String baseDir) {
         Objects.requireNonNull(baseDir);
         this.baseDir = new File(baseDir).getAbsoluteFile();
     }
@@ -49,7 +49,7 @@ protected boolean isRepoFile(String repoPath) {
      *
      * @param baseDir the base directory. Must not be null.
      */
-    FileSystemJSModuleRepository(@NotNull File baseDir) {
+    public FileSystemJSModuleRepository(@NotNull File baseDir) {
         Objects.requireNonNull(baseDir);
         this.baseDir = baseDir.getAbsoluteFile();
     }

src/main/java/org/openstreetmap/josm/plugins/scripting/graalvm/GraalVMFacade.java

@@ -27,7 +27,16 @@ public class GraalVMFacade  implements IGraalVMFacade {
 
     private Context context;
 
-    private void populateContext(final Context context) {
+    /**
+     * Initializes a GraalVM context with the standard bindings.
+     *
+     * Can be used in test cases to properly initialize a
+     * GraalVM context for testing.
+     *
+     * @param context the context
+     */
+    static public void populateContext(@NotNull final Context context) {
+        Objects.requireNonNull(context);
         final Value bindings = context.getBindings("js");
 
         // populate the context with the require function

src/test/resources/require/modules/hello-world-message.js

@@ -0,0 +1 @@
+exports.message = "hello-world"

src/test/unit/groovy/org/openstreetmap/josm/plugins/scripting/graalvm/AbstractGraalVMBasedTest.groovy

@@ -1,6 +1,7 @@
 package org.openstreetmap.josm.plugins.scripting.graalvm
 
-
+import org.junit.jupiter.api.AfterEach
+import org.junit.jupiter.api.BeforeAll
 import org.junit.jupiter.api.BeforeEach
 import org.openstreetmap.josm.plugins.PluginException
 import org.openstreetmap.josm.plugins.PluginInformation
@@ -17,18 +18,37 @@ class AbstractGraalVMBasedTest extends JOSMFixtureBasedTest {
             desc.getLanguageName().filter{it == "JavaScript"}
         }
 
-    @BeforeEach
-    void setup() throws PluginException, IOException {
-        def projectDir = getProjectHome()
+    static ICommonJSModuleRepository moduleRepo
+
+    @BeforeAll
+    static void readEnvironmentVariables() {
+        final moduleRepoPath = System.getenv("TEST_COMMONJS_MODULE_REPO")
+        if (moduleRepoPath == null) {
+            fail("environment variable TEST_COMMONJS_MODULE_REPO not set")
+        }
+        final dir = new File(moduleRepoPath)
+        if (!dir.isDirectory() || !dir.canRead()) {
+            fail("directory '$dir.absolutePath' with CommonJS modules doesn't exist or isn't readable")
+        }
+        moduleRepo = new FileSystemJSModuleRepository(moduleRepoPath)
+    }
 
+    @BeforeEach
+    void resetRepositoryRegistry() {
         // Initialize the CommonJS module repositories
         def registry = CommonJSModuleRepositoryRegistry.getInstance()
         registry.setBuiltInRepository( new JarJSModuleRepository(scriptingJarFile, "/js/v2"))
         registry.addUserDefinedRepository(
-            new FileSystemJSModuleRepository(
-                new File(projectDir, "src/test/unit/javascript/v2")
-            )
+                new FileSystemJSModuleRepository(
+                        new File(getProjectHome(), "src/test/unit/javascript/v2")
+                )
         )
+        CommonJSModuleRepositoryRegistry.instance.clear()
+        CommonJSModuleRepositoryRegistry.instance.addUserDefinedRepository(moduleRepo)
+    }
+
+    @BeforeEach
+    void setup() throws PluginException, IOException {
         //noinspection GroovyResultOfObjectAllocationIgnored
         new ScriptingPlugin(new PluginInformation(scriptingJarFile), true /* in test environment */)
 
@@ -38,4 +58,18 @@ class AbstractGraalVMBasedTest extends JOSMFixtureBasedTest {
         Logging.getLogger().setFilter(
         record -> record.getLevel().intValue() >= Level.WARNING.intValue())
     }
+
+    protected IGraalVMFacade facade
+
+    @BeforeEach
+    void initGraalVMFacade() {
+        facade = GraalVMFacadeFactory.getOrCreateGraalVMFacade()
+    }
+
+    @AfterEach
+    void resetGraalVMFacade() {
+        if (facade != null) {
+            facade.resetContext()
+        }
+    }
 }

src/test/unit/groovy/org/openstreetmap/josm/plugins/scripting/graalvm/CommonJSModuleCacheTest.groovy

@@ -1,15 +1,11 @@
 package org.openstreetmap.josm.plugins.scripting.graalvm
 
+import groovy.test.GroovyTestCase
 import org.graalvm.polyglot.Context
 import org.graalvm.polyglot.Value
 import org.junit.jupiter.api.Test
 
-import static org.junit.Assert.assertNotNull
-import static org.junit.Assert.assertTrue
-import static org.junit.Assert.assertFalse
-import static org.junit.Assert.assertEquals
-
-class CommonJSModuleCacheTest {
+class CommonJSModuleCacheTest extends GroovyTestCase {
 
     @Test
     void "singleton should be available"() {

src/test/unit/groovy/org/openstreetmap/josm/plugins/scripting/graalvm/CommonJSModuleRepositoryFactoryTest.groovy

@@ -1,11 +1,10 @@
 package org.openstreetmap.josm.plugins.scripting.graalvm
 
+import groovy.test.GroovyTestCase
 import org.junit.jupiter.api.BeforeAll
 import org.junit.jupiter.api.Test
 
-import static org.junit.Assert.assertTrue
-
-class CommonJSModuleRepositoryFactoryTest {
+class CommonJSModuleRepositoryFactoryTest extends GroovyTestCase{
 
     static def projectHome