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

Add Scala.js test suite #37

Closed
wants to merge 3 commits into from
Closed

Add Scala.js test suite #37

wants to merge 3 commits into from

Conversation

jonas
Copy link
Contributor

@jonas jonas commented Nov 24, 2013

Complete with Jasmine runner, console reporter and 72 specs.
It currently has one failing spec.

screen shot 2013-11-24 at 1 19 18

@sjrd
Copy link
Member

sjrd commented Nov 24, 2013

This looks amazing! I will review this tomorrow. Thanks a lot!

@jonas
Copy link
Contributor Author

jonas commented Nov 24, 2013

Great. I've rebased onto master and made the change to fix the scala.Enumeration test.

One thing that would be nice to fix before pulling this in would be to use the jasmine webjar instead of including the 2000+ line jasmine.js in the repository.

diff --git a/project/ScalaJSBuild.scala b/project/ScalaJSBuild.scala
index dd7965f..70affae 100644
--- a/project/ScalaJSBuild.scala
+++ b/project/ScalaJSBuild.scala
@@ -259,13 +259,16 @@ object ScalaJSBuild extends Build {

   // Testing

+  val jasmineVersion = "1.3.1"
+
   lazy val test: Project = Project(
       id = "scalajs-test",
       base = file("test"),
       settings = defaultSettings ++ scalaJSSettings ++ Seq(
           name := "Scala.js test suite",
           publishArtifact in Compile := false,
-          scalacOptions += "-Yskip:cleanup,icode,jvm"
+          scalacOptions += "-Yskip:cleanup,icode,jvm",
+          libraryDependencies += "org.webjars" % "jasmine" % jasmineVersion % "test"
       )
   ).dependsOn(compiler % "plugin")

So far I have not been able to find a way to ensure that jasmine.js is included before jasmine-html.js in the webjar. And that test/src/test/resources/bootstrap.js is included before jasmine.js ...

info] Extracting /Users/fonseca/.ivy2/local/org.scala-lang.modules.scalajs/scalajs-library_2.10/0.1-SNAPSHOT/jars/scalajs-library_2.10.jar ...
[info] Compiling 12 Scala sources to /opt/scala-js/test/target/scala-2.10/test-classes...
[info] Extracting /Users/fonseca/.ivy2/cache/org.webjars/jasmine/jars/jasmine-1.3.1.jar ...
[info] Running ...
org.mozilla.javascript.EcmaError: ReferenceError: "jasmine" is not defined. (/opt/scala-js/test/target/streams/test/externalDependencyClasspath/$global/package-js/extracted-jars/jasmine-1.3.1.jar--696c88a/META-INF/resources/webjars/jasmine/1.3.1/jasmine-html.js#1)
    at org.mozilla.javascript.ScriptRuntime.constructError(ScriptRuntime.java:3687)

@sjrd
Copy link
Member

sjrd commented Nov 25, 2013

Hi,

I built a few more commits on top of this pull request, here:
https://github.com/sjrd/scala-js/commits/feature/test-suite

  • Move tests to package scala.scalajs.test._, because I feel bad about polluting standard packages like java.lang, scala and scala.scalajs.js with test classes.
  • Fix the build script (so that incremental compilation works)
  • Fetch jasmine.js from the webjar, as you suggested. The way to fix the ordering of the files is super hacky right now, but at least it works.

WDYT? If it seems OK to you, I suggest squashing all the commits, and merge that.

@sjrd sjrd mentioned this pull request Nov 25, 2013
@jonas
Copy link
Contributor Author

jonas commented Nov 25, 2013

Looks good to me. :)
On Nov 25, 2013 10:00 AM, "Sébastien Doeraene" notifications@github.com
wrote:

Hi,

I built a few more commits on top of this pull request, here:
https://github.com/sjrd/scala-js/commits/feature/test-suite

  • Move tests to package scala.scalajs.test._, because I feel bad about
    polluting standard packages like java.lang, scala and scala.scalajs.js with
    test classes.
  • Fix the build script (so that incremental compilation works)
  • Fetch jasmine.js from the webjar, as you suggested. The way to fix
    the ordering of the files is super hacky right now, but at least it works.

WDYT? If it seems OK to you, I suggest squashing all the commits, and
merge that.


Reply to this email directly or view it on GitHubhttps://github.com//pull/37#issuecomment-29207873
.

@jonas
Copy link
Contributor Author

jonas commented Nov 25, 2013

And please feel free to also change the copyright to LAMP/EPFL, if that is
preferable.
On Nov 25, 2013 10:07 AM, "Jonas Fonseca" jonas.fonseca@gmail.com wrote:

Looks good to me. :)
On Nov 25, 2013 10:00 AM, "Sébastien Doeraene" notifications@github.com
wrote:

Hi,

I built a few more commits on top of this pull request, here:
https://github.com/sjrd/scala-js/commits/feature/test-suite

  • Move tests to package scala.scalajs.test._, because I feel bad
    about polluting standard packages like java.lang, scala and
    scala.scalajs.js with test classes.
  • Fix the build script (so that incremental compilation works)
  • Fetch jasmine.js from the webjar, as you suggested. The way to fix
    the ordering of the files is super hacky right now, but at least it works.

WDYT? If it seems OK to you, I suggest squashing all the commits, and
merge that.


Reply to this email directly or view it on GitHubhttps://github.com//pull/37#issuecomment-29207873
.

@sjrd
Copy link
Member

sjrd commented Nov 25, 2013

Yes, actually it is preferable, thank you for suggesting it.

It's done, and merged :-)
Thank you so much for this, this is so cool!

@sjrd sjrd closed this Nov 25, 2013
@jonas
Copy link
Contributor Author

jonas commented Nov 26, 2013

It's far from complete, but will hopefully make it easier to contribute to Scala.js.

@jonas jonas deleted the patch-3 branch November 29, 2013 18:37
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

Successfully merging this pull request may close these issues.

None yet

2 participants