Merge pull request #18 from gzm0/fix-es-module-errors

Fix #17: Fail run in NodeJSEnv if ESModule fails

Author: sjrd

build.sbt

@@ -1,8 +1,10 @@
+import com.typesafe.tools.mima.core.{Problem, ProblemFilters}
+
 val previousVersion: Option[String] = Some("1.2.1")
 val newScalaBinaryVersionsInThisRelease: Set[String] = Set()
 
 inThisBuild(Def.settings(
-  version := "1.2.2-SNAPSHOT",
+  version := "1.3.0-SNAPSHOT",
   organization := "org.scala-js",
   scalaVersion := "2.12.11",
   crossScalaVersions := Seq("2.11.12", "2.12.11", "2.13.2"),
@@ -130,6 +132,13 @@ lazy val `scalajs-js-envs-test-kit` = project
         "junit" % "junit" % "4.12",
         "com.novocode" % "junit-interface" % "0.11" % "test"
     ),
+    mimaBinaryIssueFilters ++= Seq(
+        // private[test], not an issue
+        ProblemFilters.exclude[Problem]("org.scalajs.jsenv.test.RunTests.*"),
+        ProblemFilters.exclude[Problem]("org.scalajs.jsenv.test.TimeoutRunTests.*"),
+        ProblemFilters.exclude[Problem]("org.scalajs.jsenv.test.ComTests.*"),
+        ProblemFilters.exclude[Problem]("org.scalajs.jsenv.test.TimeoutComTests.*"),
+    )
   )
   .dependsOn(`scalajs-js-envs`)
 

js-envs-test-kit/src/main/scala/org/scalajs/jsenv/test/ComTests.scala

@@ -12,19 +12,15 @@
 
 package org.scalajs.jsenv.test
 
-import org.junit.{Before, Test, AssumptionViolatedException}
+import org.junit.{Test, AssumptionViolatedException}
 import org.junit.Assume._
 
 import org.scalajs.jsenv._
 import org.scalajs.jsenv.test.kit.TestKit
 
-private[test] class ComTests(config: JSEnvSuiteConfig) {
-  private val kit = new TestKit(config.jsEnv, config.awaitTimeout)
-
-  @Before
-  def before: Unit = {
-    assumeTrue("JSEnv needs com support", config.supportsCom)
-  }
+private[test] class ComTests(config: JSEnvSuiteConfig,
+    defaultInputKind: TestKit.InputKind) {
+  private val kit = new TestKit(config.jsEnv, config.awaitTimeout, defaultInputKind)
 
   @Test
   def basicTest: Unit = {

js-envs-test-kit/src/main/scala/org/scalajs/jsenv/test/JSEnvSuite.scala

@@ -13,6 +13,7 @@
 package org.scalajs.jsenv.test
 
 import org.scalajs.jsenv.JSEnv
+import org.scalajs.jsenv.test.kit.TestKit
 
 import scala.reflect.ClassTag
 
@@ -67,13 +68,45 @@ private object JSEnvSuiteRunner {
   private def getRunners(config: JSEnvSuiteConfig): java.util.List[Runner] = {
     import java.lang.Boolean.{TRUE, FALSE}
 
-    java.util.Arrays.asList(
-        r[RunTests](config, "withCom" -> FALSE),
-        r[RunTests](config, "withCom" -> TRUE),
-        r[TimeoutRunTests](config, "withCom" -> FALSE),
-        r[TimeoutRunTests](config, "withCom" -> TRUE),
-        r[ComTests](config),
-        r[TimeoutComTests](config)
-    )
+    val runners = new java.util.ArrayList[Runner]
+
+    val withComValues =
+      if (config.supportsCom) List(TRUE, FALSE)
+      else List(FALSE)
+
+    val inputKindValues = {
+      import TestKit.InputKind
+
+      val b = List.newBuilder[InputKind]
+
+      if (config.supportsScripts)
+        b += InputKind.Script
+
+      if (config.supportsCommonJSModules)
+        b += InputKind.CommonJSModule
+
+      if (config.supportsESModules)
+        b += InputKind.ESModule
+
+      b.result
+    }
+
+    for (inputKind <- inputKindValues) {
+      for (withCom <- withComValues)
+        runners.add(r[RunTests](config, "withCom" -> withCom, "inputKind" -> inputKind))
+
+      if (config.supportsTimeout) {
+        for (withCom <- withComValues)
+          runners.add(r[TimeoutRunTests](config, "withCom" -> withCom,  "inputKind" -> inputKind))
+      }
+
+      if (config.supportsCom)
+        runners.add(r[ComTests](config, "inputKind" -> inputKind))
+
+      if (config.supportsCom && config.supportsTimeout)
+        runners.add(r[TimeoutComTests](config, "inputKind" -> inputKind))
+    }
+
+    runners
   }
 }

js-envs-test-kit/src/main/scala/org/scalajs/jsenv/test/JSEnvSuiteConfig.scala

@@ -28,6 +28,9 @@ import scala.concurrent.duration._
  *  @param supportsTimeout Whether the [[JSEnv]] under test supports the
  *      JavaScript timeout methods (as defined in
  *      [[http://www.scala-js.org/api/scalajs-library/latest/#scala.scalajs.js.timers.RawTimers$ RawTimers]]).
+ *  @param supportsScripts Whether the [[JSEnv]] under test supports [[JSEnv.Input.Script]].
+ *  @param supportsCommonJSModules Whether the [[JSEnv]] under test supports [[JSEnv.Input.CommonJSModule]].
+ *  @param supportsESModules Whether the [[JSEnv]] under test supports [[JSEnv.Input.ESModule]].
  *  @param awaitTimeout Amount of time test cases wait for "things". This is
  *      deliberately not very well specified. Leave this as the default and
  *      increase it if your tests fail spuriously due to timeouts.
@@ -40,6 +43,9 @@ final class JSEnvSuiteConfig private (
     val jsEnv: JSEnv,
     val supportsCom: Boolean,
     val supportsTimeout: Boolean,
+    val supportsScripts: Boolean,
+    val supportsCommonJSModules: Boolean,
+    val supportsESModules: Boolean,
     val exitJSStatement: Option[String],
     val awaitTimeout: FiniteDuration,
     val description: String
@@ -48,6 +54,9 @@ final class JSEnvSuiteConfig private (
       jsEnv = jsEnv,
       supportsCom = true,
       supportsTimeout = true,
+      supportsScripts = true,
+      supportsCommonJSModules = true,
+      supportsESModules = true,
       exitJSStatement = None,
       awaitTimeout = 1.minute,
       description = jsEnv.name
@@ -59,6 +68,15 @@ final class JSEnvSuiteConfig private (
   def withSupportsTimeout(supportsTimeout: Boolean): JSEnvSuiteConfig =
     copy(supportsTimeout = supportsTimeout)
 
+  def withSupportsScripts(supportsScripts: Boolean): JSEnvSuiteConfig =
+    copy(supportsScripts = supportsScripts)
+
+  def withSupportsCommonJSModules(supportsCommonJSModules: Boolean): JSEnvSuiteConfig =
+    copy(supportsCommonJSModules = supportsCommonJSModules)
+
+  def withSupportsESModules(supportsESModules: Boolean): JSEnvSuiteConfig =
+    copy(supportsESModules = supportsESModules)
+
   def withExitJSStatement(code: String): JSEnvSuiteConfig =
     copy(exitJSStatement = Some(code))
 
@@ -71,10 +89,14 @@ final class JSEnvSuiteConfig private (
   private def copy(
       supportsCom: Boolean = supportsCom,
       supportsTimeout: Boolean = supportsTimeout,
+      supportsScripts: Boolean = supportsScripts,
+      supportsCommonJSModules: Boolean = supportsCommonJSModules,
+      supportsESModules: Boolean = supportsESModules,
       exitJSStatement: Option[String] = exitJSStatement,
       awaitTimeout: FiniteDuration = awaitTimeout,
       description: String = description) = {
-    new JSEnvSuiteConfig(jsEnv, supportsCom, supportsTimeout,
+    new JSEnvSuiteConfig(jsEnv, supportsCom, supportsTimeout, supportsScripts,
+        supportsCommonJSModules, supportsESModules,
         exitJSStatement, awaitTimeout, description)
   }
 }

js-envs-test-kit/src/main/scala/org/scalajs/jsenv/test/RunTests.scala

@@ -19,13 +19,14 @@ import java.nio.file.Files
 import com.google.common.jimfs.Jimfs
 
 import org.junit.Assume._
-import org.junit.{Test, Before, AssumptionViolatedException}
+import org.junit.{Test, AssumptionViolatedException}
 
 import org.scalajs.jsenv._
 import org.scalajs.jsenv.test.kit.{TestKit, Run}
 
-private[test] class RunTests(config: JSEnvSuiteConfig, withCom: Boolean) {
-  private val kit = new TestKit(config.jsEnv, config.awaitTimeout)
+private[test] class RunTests(config: JSEnvSuiteConfig, withCom: Boolean,
+    defaultInputKind: TestKit.InputKind) {
+  private val kit = new TestKit(config.jsEnv, config.awaitTimeout, defaultInputKind)
 
   private def withRun(input: Seq[Input])(body: Run => Unit) = {
     if (withCom) kit.withComRun(input)(body)
@@ -141,7 +142,7 @@ private[test] class RunTests(config: JSEnvSuiteConfig, withCom: Boolean) {
     val badFile = Jimfs.newFileSystem().getPath("nonexistent")
 
     // `start` may not throw but must fail asynchronously
-    withRun(Input.Script(badFile) :: Nil) {
+    withRun(kit.pathToInput(badFile) :: Nil) {
       _.fails()
     }
   }
@@ -155,7 +156,7 @@ private[test] class RunTests(config: JSEnvSuiteConfig, withCom: Boolean) {
       val tmpPath = tmpFile.toPath
       Files.write(tmpPath, "console.log(\"test\");".getBytes(StandardCharsets.UTF_8))
 
-      withRun(Input.Script(tmpPath) :: Nil) {
+      withRun(kit.pathToInput(tmpPath) :: Nil) {
         _.expectOut("test\n")
           .closeRun()
       }

js-envs-test-kit/src/main/scala/org/scalajs/jsenv/test/TimeoutComTests.scala

@@ -14,21 +14,16 @@ package org.scalajs.jsenv.test
 
 import scala.concurrent.duration._
 
-import org.junit.{Before, Test}
+import org.junit.Test
 import org.junit.Assert._
 import org.junit.Assume._
 
 import org.scalajs.jsenv._
 import org.scalajs.jsenv.test.kit.TestKit
 
-private[test] class TimeoutComTests(config: JSEnvSuiteConfig) {
-  private val kit = new TestKit(config.jsEnv, config.awaitTimeout)
-
-  @Before
-  def before: Unit = {
-    assumeTrue("JSEnv needs timeout support", config.supportsTimeout)
-    assumeTrue("JSEnv needs com support", config.supportsCom)
-  }
+private[test] class TimeoutComTests(config: JSEnvSuiteConfig,
+    defaultInputKind: TestKit.InputKind) {
+  private val kit = new TestKit(config.jsEnv, config.awaitTimeout, defaultInputKind)
 
   /** Slack for timeout tests (see #3457)
    *

js-envs-test-kit/src/main/scala/org/scalajs/jsenv/test/TimeoutRunTests.scala

@@ -14,26 +14,22 @@ package org.scalajs.jsenv.test
 
 import scala.concurrent.duration._
 
-import org.junit.{Before, Test}
+import org.junit.Test
 import org.junit.Assert._
 import org.junit.Assume._
 
 import org.scalajs.jsenv._
 import org.scalajs.jsenv.test.kit.{TestKit, Run}
 
-private[test] class TimeoutRunTests(config: JSEnvSuiteConfig, withCom: Boolean) {
-  private val kit = new TestKit(config.jsEnv, config.awaitTimeout)
+private[test] class TimeoutRunTests(config: JSEnvSuiteConfig, withCom: Boolean,
+    defaultInputKind: TestKit.InputKind) {
+  private val kit = new TestKit(config.jsEnv, config.awaitTimeout, defaultInputKind)
 
   private def withRun(input: String)(body: Run => Unit) = {
     if (withCom) kit.withComRun(input)(body)
     else kit.withRun(input)(body)
   }
 
-  @Before
-  def before: Unit = {
-    assumeTrue("JSEnv needs timeout support", config.supportsTimeout)
-  }
-
   /** Slack for timeout tests (see #3457)
    *
    *  Empirically we can observe that timing can be off by ~0.1ms. By cutting

js-envs-test-kit/src/main/scala/org/scalajs/jsenv/test/kit/TestKit.scala

@@ -41,15 +41,17 @@ import org.scalajs.jsenv._
  *  }}}
  *
  *  @note Methods in [[TestKit]] allow to take a string instead of an [[Input]].
- *      The string is converted into an input form supported by the [[JSEnv]] to
- *      execute the code therein.
+ *      The string is converted into an input via `defaultInputKind`.
  *
  *  @constructor Create a new [[TestKit]] for the given [[JSEnv]] and timeout.
  *  @param jsEnv The [[JSEnv]] to be tested.
  *  @param timeout Timeout for all `expect*` methods on [[Run]] / [[ComRun]].
  */
-final class TestKit(jsEnv: JSEnv, timeout: FiniteDuration) {
-  import TestKit.codeToInput
+final class TestKit(jsEnv: JSEnv, timeout: FiniteDuration,
+    defaultInputKind: TestKit.InputKind) {
+
+  def this(jsEnv: JSEnv, timeout: FiniteDuration) =
+    this(jsEnv, timeout, TestKit.InputKind.Script)
 
   /** Starts a [[Run]] for testing. */
   def start(code: String): Run =
@@ -126,6 +128,17 @@ final class TestKit(jsEnv: JSEnv, timeout: FiniteDuration) {
     finally run.close()
   }
 
+  /** Converts a Path to an Input based on this Kit's defaultInputKind */
+  def pathToInput(path: Path): Input = {
+    import TestKit.InputKind._
+
+    defaultInputKind match {
+      case Script         => Input.Script(path)
+      case CommonJSModule => Input.CommonJSModule(path)
+      case ESModule       => Input.ESModule(path)
+    }
+  }
+
   private def io[T <: JSRun](config: RunConfig)(start: RunConfig => T): (T, IOReader, IOReader) = {
     val out = new IOReader
     val err = new IOReader
@@ -147,17 +160,25 @@ final class TestKit(jsEnv: JSEnv, timeout: FiniteDuration) {
 
     (run, out, err)
   }
+
+  private def codeToInput(code: String): Seq[Input] = {
+    val p = Files.write(
+        Jimfs.newFileSystem().getPath("test.js"),
+        code.getBytes(StandardCharsets.UTF_8))
+    List(pathToInput(p))
+  }
 }
 
-private object TestKit {
+object TestKit {
   /** Execution context to run completion callbacks from runs under test. */
   private val completer =
     ExecutionContext.fromExecutor(Executors.newSingleThreadExecutor())
 
-  private def codeToInput(code: String): Seq[Input] = {
-    val p = Files.write(
-        Jimfs.newFileSystem().getPath("testScript.js"),
-        code.getBytes(StandardCharsets.UTF_8))
-    List(Input.Script(p))
+  sealed trait InputKind
+
+  object InputKind {
+    case object Script extends InputKind
+    case object CommonJSModule extends InputKind
+    case object ESModule extends InputKind
   }
 }

js-envs-test-kit/src/test/scala/org/scalajs/jsenv/test/kit/TestKitTest.scala

@@ -22,7 +22,6 @@ import org.junit.Test
 import org.scalajs.jsenv._
 
 class TestKitTest {
-  import TestKit.codeToInput
   import TestKitTest._
 
   private def noHangTest(env: TestEnv, msg: String)(body: TestKit => Unit) = {

nodejs-env/src/main/scala/org/scalajs/jsenv/nodejs/NodeJSEnv.scala

@@ -146,7 +146,7 @@ object NodeJSEnv {
          * `import()` cannot be used from the standard input).
          */
         val importChain = input.foldLeft("Promise.resolve()") { (prev, item) =>
-          s"$prev.\n  then(${execInputExpr(item)})"
+          s"$prev.\n  then(() => ${execInputExpr(item)})"
         }
         val importerFileContent = {
           s"""