Junit & Specs2 support #163

ittaiz · 2017-03-09T11:40:39Z

Hi,
As discussed long ago ( #72 and bazelbuild/bazel#503) this is junit and specs2 support.
It actually brings with it a powerful test discovery mechanism which imitates the maven behavior. One can define multiple patterns and all classes whose name fits the pattern will be run. In Scala the classes (vs file-name) emphasis is substantial due to many classes in a file and mismatch between file-names and class names.

Features:
JUnit and Specs2 (using specs2 spec) Tests can:

Have jvm_flags (we use this internally to increase xmx for some suites)
Run test classes based on multiple patterns (we use IT*,*IT,*E2E for example)
Use supplied deps and runtime_deps

Notes:

I used bind even though it looks like Bazel want to deprecate it since that's what's used here (and Oscar advocates it)
Specs2 version- I extracted it to a method since I spent a few hours debugging a mismatch between the version of specs2 and specs2_junit :(
Scala build file changes- I've added filegroups since I using "@scala//:lib/scala-reflect.jar" as a label didn't work for me. If you know how I can skip that I'd love to know.

Dilemas I'd love feedback on:

Grep usage- I'm currently using grep to filter the relevant classes. I considered the implications of switching from grep to Scala code (which already does some manipulation on the output). The pro of grep is performance implications since AFAIR Ulf warned me about these traversals being expensive on the JVM when we're talking about big targets. The pro of moving to Scala is to have the logic more cohesive (currently the Scala code assumes stuff from the grep like the fact that everything ends with .class) and to have a more expected regex syntax.
Better handle where a target has no test classes- currently if there are no matches grep returns an exit code of 1 and you'll get a weird message

ERROR: /Users/ittaiz/workspace/rules_scala/test/BUILD:260:1: error executing shell command: 'unzip -l bazel-out/local-fastbuild/bin/test/JunitWithNoTests.jar | grep  -e Test\.class > bazel-out/local-fastbuild/bin/test/JunitWithNoTests_discovered_classes.txt' failed: bash failed: error executing command /bin/bash -c 'unzip -l bazel-out/local-fastbuild/bin/test/JunitWithNoTests.jar | grep  -e Test\.class > bazel-out/local-fastbuild/bin/test/JunitCustomPattern2_discovered_classes.txt': com.google.devtools.build.lib.shell.BadExitStatusException: Process exited with status 1.
Target //test:JunitWithNoTests failed to build
Use --verbose_failures to see the command lines of failed build steps.

This might actually be another point towards moving this filtering to the Scala code since we can fail with a clearer message. I don't think this is a crucial point btw and we can live with it.

WDYT about adding default macros for unit tests and "bigger" tests where the first has default suffixes of (Test* and Test) and the latter has the default suffixes of (IT, *IT, *E2E)? We will definitely build this internally since that's what we use. I think this has value in the OSS but didn't want to push too much.
WDYT about removing the "suffix" and "multiple suffixes" tests? They helped me evolve the code but I think that the multiple suffixes test covers them.
Can I merge two structs? Currently I do it manually but didn't kno if there was an easier syntax for this.

bazel-io · 2017-03-09T11:40:40Z

Can one of the admins verify this patch?

ittaiz · 2017-03-09T11:41:23Z

cc @ulfjack and @damienmg for their thoughts about performance implications of using grep vs jvm filtering (more info in the above message)

run files currently manually built DiscoveredTestSuite correctly parses the zip entries JunitTest removes the @RunWith since it's not mandatory

…shared runfiles

…ingle_file)

ittaiz · 2017-03-13T06:28:24Z

@johnynek wdyt?

johnynek · 2017-03-14T02:18:35Z

scala/scala.bzl

+      #TODO consider with Damien/Ulf/Oscar the implications of switching from grep to scala code
+      #Pro-> logic will be cohesive (currently the scala code assumes stuff from the grep)
+      #Con-> IIRC Ulf warned me about performance implications of these traversals
+      command="unzip -l {archive} | grep {combined_patterns} > {out}".format(archive=archive.path, combined_patterns=_prep_grep_pattern_from(patterns), out=discovered_classes.path))


can we use jar tf here and use the bazel_tools jar so we don't depend on unzip?

we can, I actually thought about it but decided (for some reason I don't remember) on unzip. I'll change it.

I moved to jar but when I tried to use the one from bazel_tools I got the following error:

____Loading package: test ____Loading package: @local_config_cc// ____Loading package: @local_config_xcode// ____Loading complete. Analyzing... ____Loading package: @io_bazel_rules_scala_org_specs2_specs2_matcher//jar ____Loading package: @io_bazel_rules_scala_org_openjdk_jmh_jmh_generator_asm//jar ____Loading package: @io_bazel_rules_scala_org_openjdk_jmh_jmh_generator_reflection//jar ____Found 89 targets... ____Building... ____[342 / 391] Discovering classes with patterns of ["E2E", "IT"] ERROR: /Users/ittaiz/workspace/rules_scala/test/BUILD:245:1: error executing shell command: 'external/local_jdk/bin/jar -tf bazel-out/local-fastbuild/bin/test/JunitMultipleSuffixes.jar | grep -e E2E\.class -e IT\.class > bazel-out/local-fastbuild/bin/test/JunitMultipleSuffixes_discovered_...' failed: Process exited with status 1 [sandboxed]. Error: could not find libjava.dylib Error: Could not find Java SE Runtime Environment. Use --strategy=Generating=standalone to disable sandboxing for the failing actions. ____Building complete. ____Elapsed time: 3.011s, Critical Path: 0.66s

My attempt was via adding an internal attribute (like _ijar), add it to the inputs of the action and then use it instead of the vanilla jar command but that doesn't seem like it's enough. It seems more stuff are needed as inputs. Any idea?

johnynek · 2017-03-14T02:22:27Z

scala/scala.bzl

@@ -623,6 +689,30 @@ exports_files([
  "lib/scala-xml_2.11-1.0.4.jar",
  "lib/scalap-2.11.8.jar",
 ])
+
+filegroup(


we already have these here:

https://github.com/bazelbuild/rules_scala/blob/master/src/scala/BUILD#L1

Can we consolidate on those targets? Why were these needed?

I'm concerned about supporting 2.12 and 2.11 with all these direct references in different places.

we can, they were needed since using "@scala//:lib/scala-reflect.jar" as a label didn't work for me. If you know how I can skip that I'd love to know. I know my PR notes were long but this is one of the things I noted there.

:( Out of luck.

____Loading complete. Analyzing... ERROR: /Users/ittaiz/workspace/rules_scala/test/BUILD:258:1: no such target '//src/scala:parser_xml': target 'parser_xml' not declared in package 'src/scala' defined by /Users/ittaiz/workspace/rules_scala/src/scala/BUILD and referenced by '//test:Specs2Tests'. ERROR: /Users/ittaiz/workspace/rules_scala/test/BUILD:258:1: no such target '//src/scala:parser_library': target 'parser_library' not declared in package 'src/scala' defined by /Users/ittaiz/workspace/rules_scala/src/scala/BUILD and referenced by '//test:Specs2Tests'. ERROR: /Users/ittaiz/workspace/rules_scala/test/BUILD:258:1: no such target '//src/scala:parser_reflect': target 'parser_reflect' not declared in package 'src/scala' defined by /Users/ittaiz/workspace/rules_scala/src/scala/BUILD and referenced by '//test:Specs2Tests'. ERROR: Analysis of target '//test:Specs2Tests' failed; build aborted. ____Elapsed time: 0.073s

Any idea why I'm getting this?

isn't the target //src/scala:scala_xml? It seems you have :parser_xml.

any comment about this?

Ok, moved to //src/scala.
Gave @scala//:lib/scala-library.jar another try but got the following message so am staying with the filegroup.

ERROR: /Users/ittaiz/workspace/rules_scala/test/BUILD:258:1: in deps attribute of scala_junit_test rule //test:Specs2Tests: file '@scala//:lib/scala-library.jar' is misplaced here (expected no files). Since this rule was created by the macro 'scala_specs2_junit_test', the error might have been caused by the macro implementation in /Users/ittaiz/workspace/rules_scala/scala/scala.bzl:817:11.

@johnynek //src/scala doesn't work when I try to use it in an external repository.
I think it's because it's a macro.
If I prefix it with the name of the rules_scala repo in my workspace then it works but that doesn't sound like the right solution since people might name it otherwise, right?
Any thoughts?

johnynek · 2017-03-14T02:24:40Z

specs2/specs2.bzl

@@ -0,0 +1,51 @@
+def specs2_version():
+  return "3.8.8"


do we need this special function?

answered in the other comment about it

johnynek · 2017-03-14T02:25:46Z

specs2/specs2.bzl

+  "//external:io_bazel_rules_scala/dependency/specs2/specs2_matcher",
+  "//external:io_bazel_rules_scala/dependency/scalaz/scalaz_effect",
+  "//external:io_bazel_rules_scala/dependency/scalaz/scalaz_core",
+  "@scala//:scala-xml",


does this really need xml and parser combinators? If so, that's a bummer.

it does. why is that a bummer?

xml literals are deprecated, and the library is being removed from the standard library, I think.

johnynek · 2017-03-14T02:26:31Z

specs2/specs2_junit.bzl

+  # Aditional dependencies for specs2 junit runner
+  native.maven_jar(
+      name = "io_bazel_rules_scala_org_specs2_specs2_junit_2_11",
+      artifact = "org.specs2:specs2-junit_2.11:" + specs2_version(),


I don't see how that function helps (specs2_version) since only one version will have the right sha1 below, no (unless someone generates a sha1 collision for specs2! ;)

As I mentioned in the PR notes the reason I added this was because I (mistakenly) used one version in the specs2.bzl file and a different one in the specs2_junit.bzl file. Finding that bug took me a few hours since the runtime error messages are really unclear. Once I found it out I realized my problem was code duplication so I solved it with a method.

johnynek · 2017-03-14T02:30:39Z

src/java/io/bazel/rulesscala/test_discovery/DiscoveredTestSuite.scala

+import org.junit.runners.model.RunnerBuilder
+
+@RunWith(classOf[TextFileSuite])
+class DiscoveredTestSuite


can you add a comment to the approach here? It looks like this is always registered at the test to run, and then by environmental variable all classes are passed in. Is this right?

that's right. I'll add a comment though I'd rather try and change the name to make it more explicit. If it was called TextFileSuiteEntryPoint would that be clearer that this is just a "vessel"? an entry point? and that the interesting bit is in the TextFileSuite?

I think documenting the architecture of how you are doing this is a good idea. To be honest, I'm only probably 80% aware how, and since I won't be using this code, in 6 months I'll probably be at 10% and may have to review patches at that time. I'd really appreciate a paragraph here explaining the approach rather than someone having to go read about junit architecture, then put together the approach from integrating the .bzl and scala code.

johnynek · 2017-03-14T02:30:54Z

src/java/io/bazel/rulesscala/test_discovery/DiscoveredTestSuite.scala

+  private def readDiscoveredClasses(classesRegistry: String): Array[Class[_]] =
+  	entries(classesRegistry)
+  		.map(filterWhitespace)
+      .map(filterMetadata)


strange spacing here.

true, sublime editing. Will fix.

johnynek · 2017-03-14T02:31:39Z

src/java/io/bazel/rulesscala/test_discovery/DiscoveredTestSuite.scala

+    zipEntry.split("\\s+")
+
+  private def dropFileSuffix(classEntry: String): String = 
+  	classEntry.split("\\.").head


inconsistent indentation in this file.

true, sublime editing. Will fix.

johnynek · 2017-03-14T02:33:04Z

test/src/main/scala/scala/test/junit/specs2/Specs2Tests.scala

+    "run smoothly in bazel" >> {
+      JUnitCompileTimeDep.hello
+      success
+    }


can we add a negative test in the failing tests path? We should show that something that should fail does.

I thought in the beginning I'll need to add such a test but didn't need to since the tests I added pushed very small parts of the code. I'm really confident about the regression here. In any case I'll add one per your request.

johnynek · 2017-03-14T02:34:04Z

test_run.sh

+  else
+    return 1
+  fi
+}


does this create .xml output? I had to do extra work for scalatest with that. we test it above, note. If this does generate xml output, we should make sure we are testing it.

JUnitCore creates an xml output. I actually don't think we should test this since I didn't add any code relating to it. Additionally when we'll (soon) move to java_test and remove the launcher this test will be even more redundant.
WDYT?

I just mean that bazel expects test xml on a certain path:
https://github.com/bazelbuild/rules_scala/blob/master/scala/support/JUnitXmlReporter.scala#L82

so, I wonder if we can see that we are generating it on the right path.

ittaiz · 2017-03-14T07:39:56Z

@johnynek thanks for your review! Answered the comments but I'm not sure in the right github way since I started my own review (whatever this means). Hope all is clear.

ittaiz · 2017-03-14T12:20:32Z

@johnynek I've pushed the changes I understand and I was able to do.
Would appreciate your feedback on the outstanding things.

ittaiz · 2017-03-27T20:26:59Z

No need to apologize. I should have run the build locally but GH's resolve conflicts UI was too compelling.

…

On Mon, 27 Mar 2017 at 23:25 Stephen Twigg ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In scala/scala.bzl <#163 (comment)> : > + printFlag = "-Dbazel.discover.classes.print.discovered=%s" % ctx.attr.print_discovered_classes) + +def _scala_junit_test_impl(ctx): + if (not(ctx.attr.prefixes) and not(ctx.attr.suffixes)): + fail("Setting at least one of the attributes ('prefixes','suffixes') is required") + deps = ctx.attr.deps + [ctx.attr._suite] + jars = _collect_jars(deps) + (cjars, rjars) = (jars.compiletime, jars.runtime) + junit_deps = [ctx.file._junit,ctx.file._hamcrest] + cjars += junit_deps + rjars += [ctx.outputs.jar, + ctx.file._scalalib + ] + junit_deps + rjars += _collect_jars(ctx.attr.runtime_deps).runtime + test_suite = _gen_test_suite_flags_based_on_prefixes_and_suffixes(ctx, ctx.outputs.jar) + launcherJvmFlags = ["-ea", test_suit.archiveFlag, test_suite.prefixesFlag, test_suite.suffixesFlag, test_suite.printFlag] Yeah, sorry about that. My weak defense is that I was in a coffeeshop and quickly writing it before I had to run off. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#163 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABUIF279tE222OYrDhA4T7QPUSovguFIks5rqBtTgaJpZM4MX-WK> .

johnynek · 2017-03-28T03:19:51Z

Looks good to me, but the tests are red. As soon as they are green we should merge.

johnynek · 2017-03-28T03:30:39Z

test this please

johnynek · 2017-03-28T03:30:47Z

I think it looks good when the CI is green. On Mon, Mar 27, 2017 at 5:27 PM Ittai Zeidman <notifications@github.com> wrote:

…

Oscar, ping? Anything left here? On Mon, 27 Mar 2017 at 23:42 Stephen Twigg ***@***.***> wrote: > LGTM > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > < #163 (comment) >, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/ABUIF235K402Jaj-H1N9Nlud1o1V8yVpks5rqBUTgaJpZM4MX-WK > > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#163 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEJdmaMWIWstFjN6vOJrhYgLc3yD2voks5rqH4WgaJpZM4MX-WK> .

ittaiz · 2017-03-28T03:41:51Z

this still fails locally but with a weird error:
trying to understand how these changes relate to _build_nosrc_jar for twitter_scrooge:allscrooges

____Loading complete.  Analyzing...
____Found 102 targets...
____[1 / 24] scala //test/src/main/scala/scala/test/twitter_scrooge:allscrooges
ERROR: /Users/ittaiz/workspace/rules_scala/src/scala/scripts/BUILD:19:1: error executing shell command: '
  rm -rf bazel-out/host/bin/src/scala/scripts/generator.jar_tmp
  set -e
  mkdir -p bazel-out/host/bin/src/scala/scripts/generator.jar_tmp
  # copy any resources

  external/local_jdk/bin/java -...' failed: Process exited with status 2 [sandboxed].
Error: could not find libjava.dylib
Error: Could not find Java SE Runtime Environment.
Use --strategy=Generating=standalone to disable sandboxing for the failing actions.
____Elapsed time: 0.737s, Critical Path: 0.61s

ittaiz · 2017-03-28T03:46:24Z

I wonder if I can trigger as well-
test this please

johnynek · 2017-03-28T03:47:20Z

test this please

johnynek · 2017-03-28T03:49:51Z

This is weird. I wonder if master is actually broken and somehow we didn't see? But I thought everything was green?

ittaiz · 2017-03-28T03:52:17Z

test bazel build test/... passes on travis and fails on my machine so I'm not really certain what's going on here :(

johnynek · 2017-03-28T04:05:52Z

Travis can't use sandboxing, I don't think, so I think we don't see issues there. I would look most closely at the ci.bazel.io.

I wonder if we can really remove the dependencies on the jdk as was done. cc @sdtwigg

ittaiz · 2017-03-28T04:07:02Z

@johnynek seems like mac os is broken on CI which makes sense since that's my os. getting kids ready for school. will take a look soon.

ittaiz · 2017-03-28T04:12:46Z

@johnynek @sdtwigg checkout master and indeed it's broken.

sdtwigg · 2017-03-28T04:30:51Z

I'm re-reviewing as well.... Is there a way to get the error log from the failing build?
I had tried the CL in a local hermetic build and it was fine with the JDK removed but it IS possible something strange happened. :/

sdtwigg · 2017-03-28T04:44:06Z

No, we need to add the dependency on the jdk back. The default bazel jdk/java tooling setup, located at https://github.com/bazelbuild/bazel/blob/master/src/main/tools/jdk.BUILD, does not properly associate dependencies for the java and javac targets.

'Amusingly' (and I'll try to figure out some way to avert this for future tests), I HAD run this on a hermetic BUILD system; however, the system jdk/java tooling in that system is slightly different. Notably, it was establishing a dependency for java and javac on the jdk.

ittaiz · 2017-03-28T04:54:23Z

I think the main way to avert this in the future is to mandate an automatic ci.bazel.io green light and not settle for a Travis one or a manual run. Should we merge this and then Stephen will fix on top of it? It was green before merging master in

…

On Tue, 28 Mar 2017 at 7:44 Stephen Twigg ***@***.***> wrote: No, we need to add the dependency on the jdk back. The default bazel jdk/java tooling setup, located at https://github.com/bazelbuild/bazel/blob/master/src/main/tools/jdk.BUILD, does not properly associate dependencies for the java and javac targets. 'Amusingly' (and I'll try to figure out some way to avert this for future tests), I HAD run this on a hermetic BUILD system; however, the system jdk/java tooling in that system is slightly different. Notably, it was establishing a dependency for java and javac on the jdk. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#163 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABUIF6OT58KbVMgCSO23lyf01OTUjtDZks5rqJAWgaJpZM4MX-WK> .

sdtwigg · 2017-03-28T05:06:52Z

Since you use _scala_binary_common rather than create your own binary, should be fine with merging this before #172. (I want to wait for that to show green in CI so concerned with delaying your PR further.)

johnynek · 2017-03-28T17:42:49Z

okay @ittaiz we should be good to go if you can merge master one more time. Sorry for the pain and thanks for bearing with it in this contribution.

The reason bazel ci is not automatic, I understand, is to limit security risk. A human looks first to make sure it looks sane.

ittaiz · 2017-03-28T21:05:50Z

@johnynek merged. can you please trigger CI?

damienmg · 2017-03-28T21:19:56Z

test this please

ittaiz · 2017-03-28T21:24:55Z

@johnynek @damienmg
All tests have passed but strange build failure-

FATAL: Remote call on Channel to /192.168.59.18 failed
java.io.IOException: Remote call on Channel to /192.168.59.18 failed
	at hudson.remoting.Channel.call(Channel.java:830)
	at hudson.Launcher$RemoteLauncher.kill(Launcher.java:984)
	at hudson.model.AbstractBuild$AbstractBuildExecution.run(AbstractBuild.java:540)
	at hudson.model.Run.execute(Run.java:1728)
	at hudson.matrix.MatrixRun.run(MatrixRun.java:146)
	at hudson.model.ResourceController.execute(ResourceController.java:98)
	at hudson.model.Executor.run(Executor.java:404)
Caused by: java.lang.NoClassDefFoundError: Could not initialize class hudson.util.ProcessTree$UnixReflection
	at hudson.util.ProcessTree$UnixProcess.kill(ProcessTree.java:575)
	at hudson.util.ProcessTree$UnixProcess.killRecursively(ProcessTree.java:596)
	at hudson.util.ProcessTree$UnixProcess.killRecursively(ProcessTree.java:595)
	at hudson.util.ProcessTree$Unix.killAll(ProcessTree.java:517)
	at hudson.Launcher$RemoteLauncher$KillTask.call(Launcher.java:996)
	at hudson.Launcher$RemoteLauncher$KillTask.call(Launcher.java:987)
	at hudson.remoting.UserRequest.perform(UserRequest.java:153)
	at hudson.remoting.UserRequest.perform(UserRequest.java:50)
	at hudson.remoting.Request$2.run(Request.java:336)
	at hudson.remoting.InterceptingExecutorService$1.call(InterceptingExecutorService.java:68)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1158)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:632)
	at hudson.remoting.Engine$1$1.run(Engine.java:94)
	at java.lang.Thread.run(Thread.java:804)
	at ......remote call to Channel to /192.168.59.18(Native Method)
	at hudson.remoting.Channel.attachCallSiteStackTrace(Channel.java:1537)
	at hudson.remoting.UserResponse.retrieve(UserRequest.java:253)
	at hudson.remoting.Channel.call(Channel.java:822)
	... 6 more

damienmg · 2017-03-28T21:28:56Z

A problem on our CI ? /cc @philwo that have been touching our VM today And maybe that will do it: retest this please

…

On Tue, Mar 28, 2017, 11:25 PM Ittai Zeidman ***@***.***> wrote: @johnynek <https://github.com/johnynek> @damienmg <https://github.com/damienmg> Caused by: java.lang.NoClassDefFoundError: Could not initialize class hudson.util.ProcessTree$UnixReflection :( build failed and I have no idea why this is happening. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#163 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADjHf_JKY3k3NOjA5cX7vvh7hi7zwMjjks5rqXqogaJpZM4MX-WK> .

johnynek · 2017-03-28T22:47:06Z

retest this please

johnynek · 2017-03-28T22:53:09Z

It looks like the CI is having an issue after all the tests have passed. Since 2/3 of the platforms on bazel ci are green and travis is green, I'm going to merge.

Thanks for carrying this one over the finish line @ittaiz

philwo · 2017-03-29T07:15:24Z

@johnynek Yes, Bazel CI is currently having trouble, because I replaced the old Ubuntu 15.10 slaves with Ubuntu 16.04. That version is somehow insisting on making a pre-release version of Java 9 the default on the system, which breaks the Jenkins slave service, giving the NoClassDefFoundError exception.

I'll fix this today.

johnynek · 2017-03-29T07:17:52Z

Thanks for the update.

…

On Tue, Mar 28, 2017 at 21:15 Philipp Wollermann ***@***.***> wrote: @johnynek <https://github.com/johnynek> Yes, Bazel CI is currently having trouble, because I replaced the old Ubuntu 15.10 slaves with Ubuntu 16.04. That version is somehow insisting on making a pre-release version of Java 9 the default on the system, which breaks the Jenkins slave service, giving the NoClassDefFoundError exception. I'll fix this today. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#163 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEJdgkkhSY6pUopb27x1oQ1jiKWEC7Vks5rqgUMgaJpZM4MX-WK> .

ittaiz · 2017-03-29T07:31:28Z

Sure thing, thank you for your review. Happy to see this baby growing :) On Wed, 29 Mar 2017 at 10:17 P. Oscar Boykin <notifications@github.com> wrote:

…

Thanks for the update. On Tue, Mar 28, 2017 at 21:15 Philipp Wollermann ***@***.*** > wrote: > @johnynek <https://github.com/johnynek> Yes, Bazel CI is currently having > trouble, because I replaced the old Ubuntu 15.10 slaves with Ubuntu 16.04. > That version is somehow insisting on making a pre-release version of Java 9 > the default on the system, which breaks the Jenkins slave service, giving > the NoClassDefFoundError exception. > > I'll fix this today. > > — > You are receiving this because you were mentioned. > > > Reply to this email directly, view it on GitHub > < #163 (comment) >, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/AAEJdgkkhSY6pUopb27x1oQ1jiKWEC7Vks5rqgUMgaJpZM4MX-WK > > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#163 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABUIF2renabwr_WSrSDuGGHSG82E-acoks5rqgWhgaJpZM4MX-WK> .

googlebot added the cla: yes label Mar 9, 2017

ittaiz added 18 commits March 9, 2017 13:43

adding junit scala test running

7e2e198

discovered_classes output actually redirected to file

34620c8

run files currently manually built DiscoveredTestSuite correctly parses the zip entries JunitTest removes the @RunWith since it's not mandatory

added discovered_classes to rjars and so removed cp hack and reusing …

41c0a48

…shared runfiles

explicitly adding the discovered_classes file to the runfiles

60caa69

removed stale comment

410018d

changed target name so it's clear target name is not bound to class name

287921f

explained a todo better, removed allow_files since it's included in s…

6fd76c9

…ingle_file)

split whitespace and metadata filtering

87dbf01

added comment about the usage of grep in skylark

ea10b40

added support for runtime deps and compile time deps

9effdb9

added support for jvm_flags and multiple custom suffixes

f9f084c

support patterns and not only suffixes

3158e76

added specs2 junit support

0033f21

extracted specs2 version to method

d0e7089

removed comments

5c57a83

removed another comment

5161db1

moved to depend on an isolated dependency

99a639c

apparently -a doesn't add untracked

eb7095f

ittaiz force-pushed the junit_support branch from 8c5e77b to eb7095f Compare March 9, 2017 11:45

ittaiz mentioned this pull request Mar 9, 2017

java rules should be able to depend on Skylark rules bazelbuild/bazel#970

Closed

ittaiz mentioned this pull request Mar 13, 2017

Add specs2 support #88

Closed

johnynek suggested changes Mar 14, 2017

View reviewed changes

ittaiz added 2 commits March 14, 2017 13:24

moved from unzip to jar and reformatted DiscoveredTestSuite

30fef56

added test to show junit failures are supported

64a6404

another typo from the script

39824a7

Merge remote-tracking branch 'origin/master' into junit_support

d71bc1d

johnynek approved these changes Mar 28, 2017

View reviewed changes

johnynek merged commit b51e54c into bazelbuild:master Mar 28, 2017

Junit & Specs2 support #163

Junit & Specs2 support #163

Conversation

ittaiz commented Mar 9, 2017

bazel-io commented Mar 9, 2017

ittaiz commented Mar 9, 2017

ittaiz commented Mar 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ittaiz commented Mar 14, 2017

ittaiz commented Mar 14, 2017

ittaiz commented Mar 27, 2017 via email

johnynek commented Mar 28, 2017

johnynek commented Mar 28, 2017

johnynek commented Mar 28, 2017 via email

ittaiz commented Mar 28, 2017

ittaiz commented Mar 28, 2017

johnynek commented Mar 28, 2017

johnynek commented Mar 28, 2017

ittaiz commented Mar 28, 2017

johnynek commented Mar 28, 2017

ittaiz commented Mar 28, 2017

ittaiz commented Mar 28, 2017

sdtwigg commented Mar 28, 2017

sdtwigg commented Mar 28, 2017

ittaiz commented Mar 28, 2017 via email

sdtwigg commented Mar 28, 2017

johnynek commented Mar 28, 2017

ittaiz commented Mar 28, 2017

damienmg commented Mar 28, 2017

ittaiz commented Mar 28, 2017 • edited Loading

damienmg commented Mar 28, 2017 via email

johnynek commented Mar 28, 2017

johnynek commented Mar 28, 2017

philwo commented Mar 29, 2017

johnynek commented Mar 29, 2017 via email

ittaiz commented Mar 29, 2017 via email

ittaiz commented Mar 28, 2017 •

edited

Loading