Repository: bazelbuild/bazel
Issue: Executing scala tests
Labels: type: feature request
P3

Any existing workaround to use `java_test` for executing scala tests (similar to https://github.com/bazelbuild/bazel/issues/443), or is there a different approach to substitute the lack of  `scala_test` for now? Scala test classes usually use `org.junit.runner.RunWith` and a custom `JUnitRunner` to execute (like this one for [specs2](https://github.com/etorreborre/specs2/blob/master/junit/src/main/scala/org/specs2/runner/JUnitRunner.scala)).

Simple example:

``` scala
package com.example

import org.junit.runner.RunWith
import org.specs2.mutable.Specification
import org.specs2.runner.JUnitRunner

@RunWith(classOf[JUnitRunner])
class HelloWorldTest extends Specification {

  "HelloWorld" should {
    "have the correct message" in {
      HelloWorld.message must_== "Hello world!"
    }
  }
}
```


Issue comments:
you could create a scala_test that expands into java_test that imports the jar generated by scala_binary?

Added this issue as b/25141720.


Pull request review comments:
are these needed? It would be nice to default to discovering all the suites in the jar and running them. That seems like the preferred way to go. If you don't want to run a suite, it should be in its own target, and just not test that target.

Or at least, that's my view.

I think it is better to download rather than add the jar to the git repo. This can be done by adding to the WORKSPACE.

same comment about it being better to not check in the jar.

Sounds good. Added a download instead.

Added a download here as well :)

I agree that it would be nice. As discussed on the email thread, the big challenge is that, in Scala, the test suite name cannot easily be inferred from the path (because packages are independent of the file sytem). So I see two options:
- Add a custom test runner (written in Scala/Java) which wraps the usual scalatest runner and does this discovery at runtime.
- Specify the suites by hand.

I went for the second option for now because the first one seemed controversial on the thread. Also, supporting the second one seems like a good idea regardless of whether we end up adding the first one in addition.

What do you think?

I looked at the dependencies here:

http://central.maven.org/maven2/org/scalatest/scalatest_2.11/2.2.4/scalatest_2.11-2.2.4.pom

It looks like everything other than scala is optional or test. This does mean using generator/scalacheck style won't work here, but I think that's okay for a first draft.

I have this:
https://bazel-review.googlesource.com/#/c/2410/1

which pulls scala 2.11 and gets the scalac and scalalib there. It would be nice to use the jar provided there once both this and that are merged.

I didn't find the email thread. Can you link here for completeness?

Certainly, here it is: https://groups.google.com/forum/#!msg/bazel-dev/5Kbbwr11XOA/znTY-H1DBgAJ

It's also in the top-level description of the change but maybe it's more visible here inline.

Oh, nice! Specifying the scala version to use in inside the workspace rather than a symlink sounds great.

+1 regarding the plan once both are merged.

Yup, I figured if we need some of the optionals in there, we can still add them manually when the need comes up.

@johnynek @dinowernli Added a comment to the thread, which I think basically implements what @johnynek said above. Would love to hear what you think (preferably in the thread?).

Thanks Orr, responded on the main thread. Would it make sense to get this PR in as a "basic" way of specifying tests and possibly add more runners in the future?

+1 to something basic that works is better than the current state. We can definitely iterate.

these should no longer be here right?

After thinking more, leave it for now.

revert

revert, our style guide requires 2 empty lines between global def.

oops mixed up with python styleguide, you can leave it as it is.

revert

Ack :)

Don't depends on java being on the PATH, instead add dependency to //tools/defaults:jdk and to @bazel_tools//tools/jdk:java and use the latter as your java binary here.


Patch that resolved the issue:
diff --git a/WORKSPACE b/WORKSPACE
index ebe5700f2f086a..1cb41a8e00ac6c 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -52,3 +52,11 @@ new_http_archive(
     url = "http://downloads.typesafe.com/scala/2.11.7/scala-2.11.7.tgz",
     build_file = "tools/build_defs/scala/scala.BUILD",
 )
+
+# only used for the scala test rule
+http_file(
+    name = "scalatest",
+    url = "https://oss.sonatype.org/content/groups/public/org/scalatest/scalatest_2.11/2.2.6/scalatest_2.11-2.2.6.jar",
+    sha256 = "f198967436a5e7a69cfd182902adcfbcb9f2e41b349e1a5c8881a2407f615962",
+)
+
diff --git a/tools/build_defs/scala/README.md b/tools/build_defs/scala/README.md
index c2db3040358563..06d96a7db8cd41 100644
--- a/tools/build_defs/scala/README.md
+++ b/tools/build_defs/scala/README.md
@@ -5,17 +5,16 @@
   <ul>
     <li><a href="#scala_library">scala_library/scala_macro_library</a></li>
     <li><a href="#scala_binary">scala_binary</a></li>
+    <li><a href="#scala_test">scala_test</a></li>
   </ul>
 </div>
 
 ## Overview
 
 This rule is used for building [Scala][scala] projects with Bazel. There are
-currently three rules, `scala_library`, `scala_macro_library` and
-`scala_binary`. More features will be added in the future, e.g. `scala_test`.
+currently four rules, `scala_library`, `scala_macro_library`, `scala_binary` and `scala_test`.
 
-In order to use this build rule, you must add the following to your WORKSPACE
-file:
+In order to use `scala_library`, `scala_macro_library`, and `scala_binary`, you must add the following to your WORKSPACE file:
 ```python
 new_http_archive(
     name = "scala",
@@ -26,6 +25,15 @@ new_http_archive(
 )
 ```
 
+In addition, in order to use `scala_test`, you must add the following to your WORKSPACE file:
+```python
+http_file(
+    name = "scalatest",
+    url = "https://oss.sonatype.org/content/groups/public/org/scalatest/scalatest_2.11/2.2.6/scalatest_2.11-2.2.6.jar",
+    sha256 = "f198967436a5e7a69cfd182902adcfbcb9f2e41b349e1a5c8881a2407f615962",
+)
+```
+
 [scala]: http://www.scala-lang.org/
 
 <a name="scala_library"></a>
@@ -234,3 +242,15 @@ A `scala_binary` requires a `main_class` attribute.
     </tr>
   </tbody>
 </table>
+
+<a name="scala_test"></a>
+## scala_test
+
+```python
+scala_test(name, srcs, suites, deps, data, main_class, resources, scalacopts, jvm_flags)
+```
+
+`scala_test` generates a Scala executable which runs unit test suites written using the `scalatest` library. It may depend on `scala_library`, `scala_macro_library` and `java_library` rules.
+
+A `scala_test` requires a `suites` attribute, specifying the fully qualified (canonical) names of the test suites to run. In a future version, we might investigate lifting this requirement.
+
diff --git a/tools/build_defs/scala/scala.bzl b/tools/build_defs/scala/scala.bzl
index 488d526cbddf64..10ac1aec2257ea 100644
--- a/tools/build_defs/scala/scala.bzl
+++ b/tools/build_defs/scala/scala.bzl
@@ -85,9 +85,10 @@ def _write_manifest(ctx):
 def _write_launcher(ctx, jars):
   content = """#!/bin/bash
 cd $0.runfiles
-java -cp {cp} {name} "$@"
+{java} -cp {cp} {name} "$@"
 """
   content = content.format(
+      java=ctx.file._java.path,
       name=ctx.attr.main_class,
       deploy_jar=ctx.outputs.jar.path,
       cp=":".join([j.short_path for j in jars]))
@@ -95,6 +96,27 @@ java -cp {cp} {name} "$@"
       output=ctx.outputs.executable,
       content=content)
 
+def _args_for_suites(suites):
+  args = ["-o"]
+  for suite in suites:
+    args.extend(["-s", suite])
+  return args
+
+def _write_test_launcher(ctx, jars):
+  content = """#!/bin/bash
+cd $0.runfiles
+{java} -cp {cp} {name} {args} "$@"
+"""
+  content = content.format(
+      java=ctx.file._java.path,
+      name=ctx.attr.main_class,
+      args=' '.join(_args_for_suites(ctx.attr.suites)),
+      deploy_jar=ctx.outputs.jar.path,
+      cp=":".join([j.short_path for j in jars]))
+  ctx.file_action(
+      output=ctx.outputs.executable,
+      content=content)
+
 def _collect_comp_run_jars(ctx):
   compile_jars = set()
   runtime_jars = set()
@@ -140,43 +162,60 @@ def _scala_macro_library_impl(ctx):
       interface_jar_files=cjars,
       runfiles=runfiles)
 
-def _scala_binary_impl(ctx):
-  (cjars, rjars) = _collect_comp_run_jars(ctx)
+# Common code shared by all scala binary implementations.
+def _scala_binary_common(ctx, cjars, rjars):
   _write_manifest(ctx)
   _compile(ctx, cjars, False)
 
-  rjars += [ctx.outputs.jar, ctx.file._scalalib]
-  _write_launcher(ctx, rjars)
-
   runfiles = ctx.runfiles(
-      files = list(rjars) + [ctx.outputs.executable],
+      files = list(rjars) + [ctx.outputs.executable] + [ctx.file._java] + ctx.files._jdk,
       collect_data = True)
   return struct(
       files=set([ctx.outputs.executable]),
       runfiles=runfiles)
 
+def _scala_binary_impl(ctx):
+  (cjars, rjars) = _collect_comp_run_jars(ctx)
+  cjars += [ctx.file._scalareflect]
+  rjars += [ctx.outputs.jar, ctx.file._scalalib, ctx.file._scalareflect]
+  _write_launcher(ctx, rjars)
+  return _scala_binary_common(ctx, cjars, rjars)
+
+def _scala_test_impl(ctx):
+  (cjars, rjars) = _collect_comp_run_jars(ctx)
+  cjars += [ctx.file._scalareflect, ctx.file._scalatest, ctx.file._scalaxml]
+  rjars += [ctx.outputs.jar, ctx.file._scalalib, ctx.file._scalareflect, ctx.file._scalatest, ctx.file._scalaxml]
+  _write_test_launcher(ctx, rjars)
+  return _scala_binary_common(ctx, cjars, rjars)
+
 _implicit_deps = {
   "_ijar": attr.label(executable=True, default=Label("//tools/defaults:ijar"), single_file=True, allow_files=True),
   "_scalac": attr.label(executable=True, default=Label("@scala//:bin/scalac"), single_file=True, allow_files=True),
   "_scalalib": attr.label(default=Label("@scala//:lib/scala-library.jar"), single_file=True, allow_files=True),
+  "_scalaxml": attr.label(default=Label("@scala//:lib/scala-xml_2.11-1.0.4.jar"), single_file=True, allow_files=True),
   "_scalasdk": attr.label(default=Label("@scala//:sdk"), allow_files=True),
+  "_scalareflect": attr.label(default=Label("@scala//:lib/scala-reflect.jar"), single_file=True, allow_files=True),
   "_jar": attr.label(executable=True, default=Label("@bazel_tools//tools/jdk:jar"), single_file=True, allow_files=True),
   "_jdk": attr.label(default=Label("//tools/defaults:jdk"), allow_files=True),
 }
 
+# Common attributes reused across multiple rules.
+_common_attrs = {
+  "srcs": attr.label_list(
+      allow_files=_scala_filetype,
+      non_empty=True),
+  "deps": attr.label_list(),
+  "data": attr.label_list(allow_files=True, cfg=DATA_CFG),
+  "resources": attr.label_list(allow_files=True),
+  "scalacopts":attr.string_list(),
+  "jvm_flags": attr.string_list(),
+}
+
 scala_library = rule(
   implementation=_scala_library_impl,
   attrs={
       "main_class": attr.string(),
-      "srcs": attr.label_list(
-          allow_files=_scala_filetype,
-          non_empty=True),
-      "deps": attr.label_list(),
-      "data": attr.label_list(allow_files=True, cfg=DATA_CFG),
-      "resources": attr.label_list(allow_files=True),
-      "scalacopts": attr.string_list(),
-      "jvm_flags": attr.string_list(),
-      } + _implicit_deps,
+      } + _implicit_deps + _common_attrs,
   outputs={
       "jar": "%{name}_deploy.jar",
       "ijar": "%{name}_ijar.jar",
@@ -188,16 +227,8 @@ scala_macro_library = rule(
   implementation=_scala_macro_library_impl,
   attrs={
       "main_class": attr.string(),
-      "srcs": attr.label_list(
-          allow_files=_scala_filetype,
-          non_empty=True),
-      "deps": attr.label_list(),
-      "data": attr.label_list(allow_files=True, cfg=DATA_CFG),
-      "resources": attr.label_list(allow_files=True),
-      "scalacopts": attr.string_list(),
-      "jvm_flags": attr.string_list(),
       "_scala-reflect": attr.label(default=Label("@scala//:lib/scala-reflect.jar"), single_file=True, allow_files=True),
-      } + _implicit_deps,
+      } + _implicit_deps + _common_attrs,
   outputs={
       "jar": "%{name}_deploy.jar",
       "manifest": "%{name}_MANIFEST.MF",
@@ -208,18 +239,27 @@ scala_binary = rule(
   implementation=_scala_binary_impl,
   attrs={
       "main_class": attr.string(mandatory=True),
-      "srcs": attr.label_list(
-          allow_files=_scala_filetype,
-          non_empty=True),
-      "deps": attr.label_list(),
-      "data": attr.label_list(allow_files=True, cfg=DATA_CFG),
-      "resources": attr.label_list(allow_files=True),
-      "scalacopts":attr.string_list(),
-      "jvm_flags": attr.string_list(),
-      } + _implicit_deps,
+      "_java": attr.label(executable=True, default=Label("@bazel_tools//tools/jdk:java"), single_file=True, allow_files=True),
+      } + _implicit_deps + _common_attrs,
+  outputs={
+      "jar": "%{name}_deploy.jar",
+      "manifest": "%{name}_MANIFEST.MF",
+      },
+  executable=True,
+)
+
+scala_test = rule(
+  implementation=_scala_test_impl,
+  attrs={
+      "main_class": attr.string(default="org.scalatest.tools.Runner"),
+      "suites": attr.string_list(),
+      "_scalatest": attr.label(executable=True, default=Label("@scalatest//file"), single_file=True, allow_files=True),
+      "_java": attr.label(executable=True, default=Label("@bazel_tools//tools/jdk:java"), single_file=True, allow_files=True),
+      } + _implicit_deps + _common_attrs,
   outputs={
       "jar": "%{name}_deploy.jar",
       "manifest": "%{name}_MANIFEST.MF",
       },
   executable=True,
+  test=True,
 )


Test patch:
diff --git a/tools/build_defs/scala/test/BUILD b/tools/build_defs/scala/test/BUILD
index 9c2e71db8637dc..f516a10c21ea11 100644
--- a/tools/build_defs/scala/test/BUILD
+++ b/tools/build_defs/scala/test/BUILD
@@ -1,4 +1,4 @@
-load("/tools/build_defs/scala/scala", "scala_binary", "scala_library")
+load("/tools/build_defs/scala/scala", "scala_binary", "scala_library", "scala_test")
 
 # The examples below show how to combine Scala and Java rules.
 # ScalaBinary is the Scala equivalent of JavaBinary.
@@ -33,6 +33,16 @@ scala_library(
     ],
 )
 
+scala_test(
+    name = "HelloLibTest",
+    srcs = ["HelloLibTest.scala"],
+    suites = ["scala.test.ScalaSuite", "scala.test.JavaSuite"],
+    size = "medium",  # Not a macro, can pass test-specific attributes.
+    deps = [
+        ":HelloLib",
+    ],
+)
+
 scala_library(
     name = "OtherLib",
     srcs = ["OtherLib.scala"],
diff --git a/tools/build_defs/scala/test/HelloLib.scala b/tools/build_defs/scala/test/HelloLib.scala
index 88db4546acc66a..a1fef111da6b4b 100644
--- a/tools/build_defs/scala/test/HelloLib.scala
+++ b/tools/build_defs/scala/test/HelloLib.scala
@@ -2,7 +2,15 @@ package scala.test
 
 object HelloLib {
   def printMessage(arg: String) {
-    println(arg + " " + OtherLib.getMessage())
-    println(arg + " " + OtherJavaLib.getMessage())
+    println(getOtherLibMessage(arg))
+    println(getOtherJavaLibMessage(arg))
+  }
+
+  def getOtherLibMessage(arg: String) : String = {
+    arg + " " + OtherLib.getMessage()
+  }
+
+  def getOtherJavaLibMessage(arg: String) : String = {
+    arg + " " + OtherJavaLib.getMessage()
   }
 }
diff --git a/tools/build_defs/scala/test/HelloLibTest.scala b/tools/build_defs/scala/test/HelloLibTest.scala
new file mode 100644
index 00000000000000..7f5cb5f9b76df4
--- /dev/null
+++ b/tools/build_defs/scala/test/HelloLibTest.scala
@@ -0,0 +1,16 @@
+package scala.test
+
+import org.scalatest._
+
+class ScalaSuite extends FlatSpec {
+  "HelloLib" should "call scala" in {
+    assert(HelloLib.getOtherLibMessage("hello").equals("hello scala!"))
+  }
+}
+
+class JavaSuite extends FlatSpec {
+  "HelloLib" should "call java" in {
+    assert(HelloLib.getOtherJavaLibMessage("hello").equals("hello java!"))
+  }
+}
+