Fix #4616: Proper static dependency tracking for jl.Class #4623
Conversation
|
Still needs tests. I need to think about how to do that. |
|
For reference: To test this properly, we need a codebase in which there is no unoptimized call to |
|
Hrm, this means we cannot even write a JUnit test: JUnit itself uses |
|
Also, we must have the optimizer: Otherwise |
|
I have built a fixture so we can test this. However, it is hit by: scala-js/scala-js-js-envs#17. So it "fails to fail". |
|
Ready for review, but needs to wait for scala-js/scala-js-js-envs#18 |
| @(Rule @scala.annotation.meta.getter) | ||
| val tempFolder = new TemporaryFolder() |
There was a problem hiding this comment.
Is this something standard that I should know about, or does this deserve a comment? (I feel like it could be the former.)
There was a problem hiding this comment.
That is a standard JUnit thing. A JUnit rule essentially encapsulates test fixtures (@Before and @After).
| val output = tempFolder.newFolder().toPath | ||
|
|
||
| val env = new NodeJSEnv | ||
|
|
||
| testLink(classDefs, MainTestModuleInitializers, | ||
| linkerConfig, PathOutputDirectory(output)).flatMap { _ => | ||
| val run = env.start(Seq(Input.ESModule(output.resolve("main.mjs"))), RunConfig()) | ||
|
|
||
| run.future.finallyWith(Future(run.close())) | ||
| } |
There was a problem hiding this comment.
Looks like this section is a good candidate to be extracted in a method testLinkdAndRun or something like that, doesn't it?
There was a problem hiding this comment.
Indeed. I'll probably use the TestKit from JSEnv so that it can take care of all the I/O handling and what not (otherwise, we'll spam the test stdout).
project/Build.scala
Outdated
| @@ -979,7 +979,8 @@ object Build { | |||
| ).settings( | |||
| libraryDependencies ++= Seq( | |||
| "com.google.javascript" % "closure-compiler" % "v20211201", | |||
| "com.google.jimfs" % "jimfs" % "1.1" % "test" | |||
| "com.google.jimfs" % "jimfs" % "1.1" % "test", | |||
| "org.scala-js" %% "scalajs-env-nodejs" % "1.2.1" % "test" | |||
There was a problem hiding this comment.
| "org.scala-js" %% "scalajs-env-nodejs" % "1.2.1" % "test" | |
| "org.scala-js" %% "scalajs-env-nodejs" % "1.2.1" % "test", |
Downstream tests (notably scala-js/scala-js#4623) can profit from the ability to abstract over an input kind.
gzm0
changed the title
We need the fix to scala-js/scala-js-js-envs#17 and some API improvements in the test kit.
|
Rebased onto main. Version got bumped in #4634. |
No description provided.