add support for rule customization scala test color and stack trace o… #268
Conversation
|
Can one of the admins verify this patch? |
|
Looking for some input on what a good test would look like for this |
scala/scala.bzl
Outdated
| @@ -852,6 +860,8 @@ scala_test = rule( | |||
| attrs={ | |||
| "main_class": attr.string(default="io.bazel.rulesscala.scala_test.Runner"), | |||
| "suites": attr.string_list(), | |||
| "colors": attr.bool(default=False), | |||
| "full_stacktraces": attr.bool(default=False), | |||
There was a problem hiding this comment.
I'm happy making both of these default to True and let people opt out, personally.
There was a problem hiding this comment.
Me too actually. We'd opt to enable these as our internal defaults either way, but I wasn't sure how much pushback I'd get from changing existing behavior if I did. Looks like not much :)
scala/scala.bzl
Outdated
| @@ -987,11 +997,12 @@ def _sanitize_string_for_usage(s): | |||
| # This auto-generates a test suite based on the passed set of targets | |||
| # we will add a root test_suite with the name of the passed name | |||
| def scala_test_suite(name, srcs = [], deps = [], runtime_deps = [], data = [], resources = [], | |||
| scalacopts = [], jvm_flags = [], visibility = None, size = None): | |||
| scalacopts = [], jvm_flags = [], visibility = None, size = None, | |||
| colors=False, full_stacktraces=False): | |||
There was a problem hiding this comment.
again, I'm happy for these to default to true.
scala/scala.bzl
Outdated
| @@ -682,9 +682,17 @@ def _scala_test_impl(ctx): | |||
|
|
|||
| transitive_rjars += [ctx.outputs.jar] | |||
|
|
|||
| # output report test duration | |||
There was a problem hiding this comment.
Any chance you can extract this block into a "flags" method? This will decrease the likelihood of us having a collision.
There was a problem hiding this comment.
absolutely. I eta on that is probably within the next day or so
|
Couldn't help noticing travis build was failing. I don't think it's related to these changes though bazel command not found https://travis-ci.org/bazelbuild/rules_scala/jobs/268985675#L574 |
|
no, our HEAD build is broken again.
It's against a "pirate" url so it keeps being broken.
I'm good with merging if 0.5.3 is passing but Oscar might have some
reservations.
…On Mon, Aug 28, 2017 at 4:51 PM doug tangren ***@***.***> wrote:
Couldn't help noticing travis build was failing. I don't *think* it's
related to these changes though
bazel command not found
https://travis-ci.org/bazelbuild/rules_scala/jobs/268985675#L574
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#268 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIF5wjmOd7nZdW195Oqmvv9QQE04NNks5scsXogaJpZM4PD6Jk>
.
|
|
I'd like to follow up on making the more dev friendly settings the default if that's okay. I can also follow up with factoring out a python method for the flags if that makes it less likely to conflict with the other branch |
|
updated pull with factored out flag func and more useful defaults for test colors and stacktraces |
|
Bump |
|
Thanks @softprops ! merged side stepping the HEAD issue which is persistent. |
|
Thanks @johnynek |
…utput