Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Scala.js 1.0.1 support #114

Closed
tindzk opened this issue Apr 25, 2020 · 6 comments
Closed

Scala.js 1.0.1 support #114

tindzk opened this issue Apr 25, 2020 · 6 comments

Comments

@tindzk
Copy link
Contributor

tindzk commented Apr 25, 2020

I am trying to use MUnit with Scala.js 1.0.1 and Scala 2.13, but the linker is encountering errors:

$ sbt test
[...]
[info] Fast optimizing /tmp/testtest/munit-scalajs-10/root/target/scala-2.13/root-test-fastopt.js
[error] munit.internal.JSFs$ needs to be imported from module 'fs' but module support is disabled.
[error] munit.internal.JSPath$ needs to be imported from module 'path' but module support is disabled.
[error] There were module imports without fallback to global variables, but module support is disabled.
[error] To enable module support, set `scalaJSLinkerConfig ~= (_.withModuleKind(ModuleKind.CommonJSModule))`.
[error] (root / Test / fastOptJS) There were module imports without fallback to global variables, but module support is disabled.
[error] To enable module support, set `scalaJSLinkerConfig ~= (_.withModuleKind(ModuleKind.CommonJSModule))`.

Here is a minimal sbt project exhibiting the faulty behaviour.

I have also tried it with Bloop (using scalacenter/bloop#1234) and am getting the following errors:

[E] org.junit.runner.Runner (of kind Class) is not a valid interface implemented by munit.MUnitRunner (of kind Class)
[E]   called from munit.MUnitRunner
[E]   called from private munit.internal.PlatformCompat$.$anonfun$newRunner$1(scala.scalajs.reflect.InstantiatableClass)munit.MUnitRunner
[E]   called from munit.internal.PlatformCompat$.newRunner(sbt.testing.TaskDef,java.lang.ClassLoader)scala.Option
[E]   called from munit.internal.junitinterface.JUnitTask.execute(sbt.testing.EventHandler,[sbt.testing.Logger,scala.Function1)void
[E]   called from private org.scalajs.testing.bridge.HTMLRunner$.$anonfun$scheduleTask$1(sbt.testing.Task,org.scalajs.testing.bridge.HTMLRunner$EventCounter$Handler,org.scalajs.testing.bridge.HTMLRunner$UI$TestTask,scala.concurrent.Promise)void
[E]   called from private org.scalajs.testing.bridge.HTMLRunner$.scheduleTask(sbt.testing.Task,org.scalajs.testing.bridge.HTMLRunner$UI)scala.concurrent.Future
[E]   called from private org.scalajs.testing.bridge.HTMLRunner$.$anonfun$runTests$1(org.scalajs.testing.bridge.HTMLRunner$UI,sbt.testing.Task)scala.concurrent.Future
[E]   called from private org.scalajs.testing.bridge.HTMLRunner$.runAllTasks$1(scala.collection.Seq,org.scalajs.testing.bridge.HTMLRunner$UI)scala.concurrent.Future
[E]   called from private org.scalajs.testing.bridge.HTMLRunner$.runTests(sbt.testing.Framework,scala.collection.immutable.Seq,org.scalajs.testing.bridge.HTMLRunner$UI)scala.concurrent.Future
[E]   called from private org.scalajs.testing.bridge.HTMLRunner$.$anonfun$onLoad$10(scala.Function1,org.scalajs.testing.bridge.HTMLRunner$UI,scala.Tuple2)scala.concurrent.Future
[E]   called from private org.scalajs.testing.bridge.HTMLRunner$.onLoad(org.scalajs.testing.common.IsolatedTestSet)void
[E]   called from org.scalajs.testing.bridge.HTMLRunner$.org$scalajs$testing$bridge$HTMLRunner$$$anonfun$start$1(org.scalajs.testing.common.IsolatedTestSet)void
[E]   called from org.scalajs.testing.bridge.HTMLRunner$.start(org.scalajs.testing.common.IsolatedTestSet)void
[E]   called from org.scalajs.testing.bridge.Bridge$.start()void
[E]   called from static org.scalajs.testing.bridge.Bridge.start()void
[E]   called from core module module initializers
[E] involving instantiated classes:
[E]   munit.internal.PlatformCompat$
[E]   munit.internal.junitinterface.JUnitTask
[E]   org.scalajs.testing.bridge.HTMLRunner$
[E]   org.scalajs.testing.bridge.Bridge$

The Bloop classpath looks fine though:

"classpath" : [
	"/home/tim/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-junit-test-runtime_2.13/1.0.1/scalajs-junit-test-runtime_2.13-1.0.1.jar",
	"/home/tim/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-library_2.13/1.0.1/scalajs-library_2.13-1.0.1.jar",
	"/home/tim/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-test-bridge_2.13/1.0.1/scalajs-test-bridge_2.13-1.0.1.jar",
	"/home/tim/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-js/scalajs-test-interface_2.13/1.0.1/scalajs-test-interface_2.13-1.0.1.jar",
	"/home/tim/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.2/scala-library-2.13.2.jar",
	"/home/tim/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-reflect/2.13.2/scala-reflect-2.13.2.jar",
	"/home/tim/.cache/coursier/v1/https/repo1.maven.org/maven2/org/scalameta/munit_sjs1_2.13/0.7.3/munit_sjs1_2.13-0.7.3.jar",
	"/tmp/build-test-scalajs-10/bloop/example"
]
@olafurpg
Copy link
Member

Thank you for reporting! Have you tried updating scalaJSLinkerConfig as the message recommends?

@tindzk
Copy link
Contributor Author

tindzk commented Apr 26, 2020

Thanks. I investigated both issues.

Yes, the linking errors disappear after setting the module kind to CommonJS. Is MUnit supposed to only work when emitting CommonJS modules?

As for Bloop, I could fix it by placing scalajs-test-interface_2.13-1.0.1.jar after munit_sjs1_2.13-0.7.3.jar in the classpath (mimicking sbt's classpath). Is this intentional? I could implement a workaround in Seed, but would prefer if the classpath was sorted.

@olafurpg olafurpg reopened this Apr 27, 2020
@olafurpg
Copy link
Member

I accidentally marked the issue as closed.

Yes, the linking errors disappear after setting the module kind to CommonJS. Is MUnit supposed to only work when emitting CommonJS modules?

MUnit has a few facades for java.nio.file types here

https://github.com/scalameta/munit/blob/f61429f66f8fd81ee71e6c10dc2fe212073b5b07/munit/js/src/main/scala/munit/internal/JSIO.scala

If there is an alternative way to achieve the same functionality without the CommonJS module requirement then I'm happy to merge a PR making such a change.

As for Bloop, I could fix it by placing scalajs-test-interface_2.13-1.0.1.jar after munit_sjs1_2.13-0.7.3.jar in the classpath (mimicking sbt's classpath). Is this intentional? I could implement a workaround in Seed, but would prefer if the classpath was sorted.

Does Seed sort the classpath? If so, what sorting do you use? The ordering of jars has significant meaning and the control over the ordering should ideally be left to end users.

The cause of this issue is likely that MUnit contains interfaces for JUnit here https://github.com/scalameta/munit/tree/f61429f66f8fd81ee71e6c10dc2fe212073b5b07/munit/non-jvm/src/main/scala/org/junit It looks like org.junit.runner.Runner is an empty abstract class in Scala.js while it's a trait in MUnit

https://github.com/scala-js/scala-js/blob/55a0b28977329c112873207c3483881a585f3f62/junit-runtime/src/main/scala/org/junit/runner/Runner.scala#L6

We may be able to fix the issue by changing the MUnit runner to be an abstract class instead to match the actual JUnit Runner
https://junit.org/junit4/javadoc/4.12/org/junit/runner/Runner.html

tindzk added a commit to tindzk/munit that referenced this issue May 1, 2020
When the libraries `scalajs-test-interface` and
`scalajs-junit-test-runtime` are placed before MUnit in the
classpath, the following linking error occurs:

```
org.junit.runner.Runner (of kind Class) is not a valid interface
implemented by munit.MUnitRunner (of kind Class)
```

Solve this by defining an `abstract class` instead of a `trait` to
match JUnit's [`Runner`](https://junit.org/junit4/javadoc/4.12/org/junit/runner/Runner.html).

See also scalameta#114.
@tindzk
Copy link
Contributor Author

tindzk commented May 1, 2020

Thanks for your thorough analysis!

The facades look correct. Reading the documentation, the Namespace type should be compatible with ECMAScript 2015 which is the default module kind since Scala.js 1.0.

Seed sorts the classpath by name using Scala's sorted. Since the ordering matters indeed, I will remove it. This will not solve the issue though as the same linking error occurs when scalajs-junit-test-runtime_2.13 is placed before munit (which is correct since munit depends on scalajs-junit-test-runtime). However, it did work with your suggested fix which I have submitted as a pull request (#120).

tindzk added a commit to tindzk/seed that referenced this issue May 1, 2020
tindzk added a commit to tindzk/seed that referenced this issue May 1, 2020
For newer Scala.js versions, the `scalajs-test-bridge` artefact must be
added to the classpath.

Further changes were made to improve compatibility with the MUnit
testing library:

1. Add build setting allowing to specify Scala.js module kind
2. Fix ordering of classpath in generated Bloop configuration files

See also:
- scalacenter/bloop#1234
- scalameta/munit#114
@olafurpg
Copy link
Member

olafurpg commented May 1, 2020

Sounds good! I triggered a 0.7.5 release with your fix!

@olafurpg olafurpg closed this as completed May 1, 2020
@tindzk
Copy link
Contributor Author

tindzk commented May 1, 2020

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants