Make testHtml independent of jsEnv

[mushtaq]

I am trying to use ESModule support in Scala.js to avoid having to use webpack/scalajs-bunlder. Here is minimal example that works using snowpack with Scala.js.

npm start and npm run build work as expected and loads the fastOptJS as ESModule which in turn imports rxjs. But running sbt testHtml gives an error:

(node:1996) UnhandledPromiseRejectionWarning: /Users/mushtaqahmed/projects/scala/scalajs-snowpack-example/example/target/scala-2.13/example-test-fastopt.js:685
import * as $i_rxjs from "rxjs";S 2s
^^^^^^

SyntaxError: Cannot use import statement outside a module
    at wrapSafe (internal/modules/cjs/loader.js:1117:16)
    at Module._compile (internal/modules/cjs/loader.js:1165:27)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1221:10)
    at Module.load (internal/modules/cjs/loader.js:1050:32)
    at Function.Module._load (internal/modules/cjs/loader.js:938:14)
    at ModuleWrap.<anonymous> (internal/modules/esm/translators.js:155:15)
    at ModuleJob.run (internal/modules/esm/module_job.js:140:23)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async Loader.import (internal/modules/esm/loader.js:162:24)
(Use `node --trace-warnings ...` to show where the warning was created)
(node:1996) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:1996) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

The alternate way to test could have been via scala-js-env-selenium, but that does not support ESModules, yet: scala-js/scala-js-env-selenium#115

[gzm0]

testHTML still requires that your code be executable with your jsEnv. This seems to be not the case here.

The error you are seeing is basically Node.js interpreting your file as a script (probably because it does not have the extension .mjs). You'll have to change the extension of the generated file to .mjs.

artifactPath in (<project>, Compile, fastOptJS) := (crossTarget in project).value / "<output-name>.mjs"'

Further, you'll have to configure Node.js correctly to find rxjs.

I realize this is very manual. However, managing JS bundling, libraries and such is explicitly out of scope for the core repo. So unless you want to manage your JS libraries manually, avoiding scalajs-bundler/webpack might not be the best idea.

[mushtaq]

testHTML still requires that your code be executable with your jsEnv. This seems to be not the case here.

Thanks for clarifying this. I was under the impression that testHtml will generate an HTML file that can be opened in any browser independent of the jsEnv.

I think, for my use case where I want to use Chrome exclusively, it is best that I give a shot at tacking scala-js/scala-js-env-selenium#115. Hope it is not too difficult.

[sjrd]

@gzm0 So, is this as-designed/won't fix?

[gzm0]

Well... this might be a good time to re-discuss options with regards to this.

The reason we are still triggering the jsEnv on testHtml is that we use sbt for test detection. So sbt needs to invoke a jsEnv to retrieve the fingerprints of the test framework. Then, there is black magic to inject the set of tests into the HTML.

This is not very nice, but I'm not sure how we can improve this without giving more reflection power to Scala.js. We'd need to support (in addition to what is currently supported):

• Query all subclasses of an instantiatable class.
• Query by annotations (e.g. the JUnit @Test annotation)

[sjrd]

Querying all subclasses on an instantiatable class (or all instantiatable classes for that matter) is something we could expose easily as a library-level new method in Reflect.

Querying by annotations is an entirely different story. That information is currently entirely dropped by the compiler back-end. I don't see a path towards including that information in the sjsir files, let alone at run-time.

I'm not sure it's possible to move test detection to the JS side.

[gzm0]

Querying by annotations is an entirely different story.

Yeah... That was my feeling as well :( Unfortunately, that's the mechanism JUnit uses, so any partial support would be hard to test for us as well :-/

If it's OK with you, I'd like to keep this open for a bit and think a bit more.

[sjrd]

If it's OK with you, I'd like to keep this open for a bit and think a bit more.

Sure.

[gzm0]

So, it turns out that I didn't really give the entire story: Even JUnit, at the end of the day, only cares about all subclasses of a given type (org.scalajs.junit.Bootstrapper). The way it deals with annotations is with its own compiler plugin. It seems that any reasonable implementation of a test framework must follow this approach.

What is unclear to me at this point, is if we can change the interface in a backwards compatible manner to deal with this.

[sjrd]

Technically, the bootstrapper contains everything we would want, including the ability to iterate over all the tests, for discovery.

The problem is that a generic processor would have to work with an sbt.testing.Fingerprint, and generically perform some lookups. How would that generic processor know to search for "Xyz$Bootstrapper" classes? There are two Fingerprint classes:

• SubclassFingerprint: for that, we can:
  1. depending on isModule(), iterate through reflectively loadable modules or instantiatable classes
  2. find the ones that extend superclassName() (¹)
  3. if classes, and if requireNoArgConstructor(), check that they have a 0-arg constructor (we can already do that with the Reflect API)
• AnnotatedFingerprint: for that,
  1. we can use isModule() the same way as for SubclassFingerprint
  2. but we're stuck at the problem of identifying the bootstrappers anyway, because nothing links the annotationName() to any kind of bootstrapper magic

(¹) There's one catch here, though: if we had a j.l.Class of the superclass, we could do superclassClass.isAssignableFrom(reflectivelyInstantiatableClass.runtimeClass), but the fingerprint only gives its getName(). We don't have a way to get the Class from its name since that superclass is typically abstract (or a trait) and hence not reflectively instantiable itself. So we'll also have to find something here, but that seems easier than the annotation problem.

One possible way would be to invent a new standard interface for all bootstrappers. It could have the following form:

@EnableReflectiveInstantiation
trait AnnotatedFingerprintTestBootstrapper {
  def handlesAnnotationName(annotationName: String): Boolean
  def associatedTestClassName(): String
}

The test runner could reflectively iterate through all of those, and call handlesAnnotationName(fingerprint.annotationName()) to test whether it matches. If it matches, use associatedTestClassName() to get the name of the actual test class (which will be given to the testing framework).

We could retroactively make junit.Bootstrapper extend that trait as:

trait Bootstrapper extends AnnotatedFingerprintTestBoostrapper {
  def handlesAnnotationName(annotationName: String): Boolean =
    annotationName == "org.junit.Test"

  def associatedTestClassName(): String =
    getClass().getName().stripSuffix("$scalajs$junit$bootstrapper")
}

(the second method relies on Semantics.runtimeClassName not to alter the names of those boostrappers, so for the future we'll want the JUnit plugin to override associatedTestClassName() with a constant)

In theory, the retroactive extension only works on 2.12+ because it requires the new methods to be default methods. But test classes are almost never used from binaries only; they are used from compiled sources.

[gzm0]

Hmmm... So that would require all test frameworks to provide this. I'm not sure that is feasible; especially for test frameworks that currently don't need a compiler plugin. (Although their test class itself could extend AnnotatedFingerprintTestBootstrapper but that in turn would make its methods leak).

[sjrd]

No, only testing frameworks that rely on an AnnotatedFingerprint would require this. And today, those testing frameworks require a compiler plugin anyway. Moreover, in practice, we only know of JUnit as a real example.

All other testing frameworks use a SubclassFingerprint, for which we don't need much more than what we have today (we just need something new in the Reflect API for the (¹) I mentioned above).

[gzm0]

Fair enough. I think we should probably indeed do this.

[gzm0]

The more I think about this, I'm not sure we can implement (¹) without either of:

• Adding compiler support.
• Reaching getSuperclass (this might not be a problem, but then we need to change the way how we test codebases where it isn't reached).

[sjrd]

Yes, I think you're right.

Although getSuperclass wouldn't be enough. We would also need getInterfaces, which we never implemented. And even then, it wouldn't work with altered runtimeClassName.

So I believe we need compiler support.

[gzm0]

:( that in turn changes the binary backwards compat story quite a bit.

It's quite bothering that Scala.js itself with the JSEnv test kit actually has an example that would break if we introduce a re-compilation requirement (although on the JVM, but the concept really would translate 1-1).

[sjrd]

Yes, the big downside is that it would require recompilation of the superclass/supertrait (so in practice, of the testing framework) for this feature to work. The question is: is that deal breaker? It would mean that testHtml in a new version of Scala.js would not work with a testing framework compiled for an earlier version. That's ... probably on the side of being a deal breaker, unfortunately. Unless we find a way to make it still work with older things using the technique used today, and only use the new technique for newer testing frameworks ...

It's quite bothering that Scala.js itself with the JSEnv test kit actually has an example that would break if we introduce a re-compilation requirement (although on the JVM, but the concept really would translate 1-1).

Hum, I'm not following. :s What example?

[gzm0]

The JSEnv test kit is an example of tests that are published in compiled form and intended to be used that way. It is only JVM artifacts, so not affected by this, but a good example of the impact the recompilation requirement would have on the ecosystem:

1. The scalajs-js-envs artifact needs to publish a new version.
2. All JSEnv implementations need to update the version if they wish to update Scala.js.

If step 1 does not happen for an artifact, everybody is blocked from step 2.

[sjrd]

No, that would not be the equivalent. We don't need to recompile test classes. We only need to recompile the superclass/supertrait of all tests, which is defined in the testing framework. So the equivalent here would be to recompile the JUnit testing framework, and then use the new JUnit framework with the existing binaries of the tests in the test kit, and things would work. The blocking step is for the testing framework itself to republish a version.

[gzm0]

Yes, right. It's an example of how we need backwards compatibility for the bootstrapper in JUnit specifically.

Unless we find a way to make it still work with older things using the technique used today, and only use the new technique for newer testing frameworks ...

I'll think about it, but I don't think so (short of making hardcoded lists). Mainly because we cannot access any information about the testing framework without launching some JS code.

[gzm0]

Note to self: Is it time to build sbt.testing._ version 2? List of stuff that could be fixed:

• This issue
• Performance regression in running tests, v0.6.20 #3123 (thread context switching due to async interface), or more general: abstract execution details away.
• Rename master/slave to controller/worker in the testing infrastructure. #4294
• JSEnvs are only closed after all tests are run in aggregate project #4317 (comment) (management for resources needed across multiple Frameworks)

[sjrd]

We can't really do that, I'm afraid.

First, in terms of binary compatibility, that would break every testing framework that has been published. We would have a hard time to tell them that they need to upgrade to the new version and republish if they want newer users of Scala.js to be able to use their framework; while at the same time that it would break users of older Scala.js. (testing framework authors can be very conservative about what they support)

In terms of source compatibility, one design constraint of sbt.testing._ is that it remains source compatible with the JVM one. At least in a way that frameworks can be written for both platforms using the same sources. While we could fix #4294 under that constraint, I'm not sure what we could for this issue and #3123.

[gzm0]

Just to be clear: This is not about Scala.js' sbt.testing._ it is about the Scala ecosystem's sbt.testing._. IMO backwards compatibility is not that much of an issue, because we could very likely bridge the new version to the old version (from what I see, we'd like to give less control to the framework caller, so we can probably implement something with less control on top of something with more control). This would be a change overarching the entire Scala / sbt ecosystem, so the next step would be to write a SIP / SPP / SCP.

[sjrd]

Oh, you're aiming far! Wow. That will require to get even more people on board, notably the maintainers of all the Scala build tools in circulation. I guess the right way to start a discussion like that would be to open a thread on https://contributors.scala-lang.org/

[gzm0]

An alternative to all this, could be to introduce a resource file provided by testing frameworks (with a name derived from the name of the Framework class name) that contains the fingerprints.

The TestAdapter could look for these files and if they exist, bypass execution of the jsEnv. If the file does not exist, it would fall back to executing the jsEnv in order to retain backwards compatibility.

What remains unclear, is how we'd get these files into the testing framework artifacts (as in: ensure most frameworks have them).

[sjrd]

Perhaps we should have a larger discussion with likely stakeholders, such as build tool maintainers and testing framework maintainers. It seems we're not going to achieve anything on our own, or at least without their buy-in.