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 support for rule customization scala test color and stack trace o… #268

Merged
merged 2 commits into from
Aug 30, 2017
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions scala/scala.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -682,9 +682,17 @@ def _scala_test_impl(ctx):

transitive_rjars += [ctx.outputs.jar]

# output report test duration
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance you can extract this block into a "flags" method? This will decrease the likelihood of us having a collision.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

absolutely. I eta on that is probably within the next day or so

flags = "-oD"
if ctx.attr.full_stacktraces:
flags += "F"
else:
flags += "S"
if not ctx.attr.colors:
flags += "W"
args = " ".join([
"-R \"{path}\"".format(path=ctx.outputs.jar.short_path),
"-oWDS",
flags,
"-C io.bazel.rules.scala.JUnitXmlReporter ",
])
# main_class almost has to be "org.scalatest.tools.Runner" due to args....
Expand Down Expand Up @@ -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),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy making both of these default to True and let people opt out, personally.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

"_scalatest": attr.label(default=Label("//external:io_bazel_rules_scala/dependency/scalatest/scalatest"), allow_files=True),
"_scalatest_runner": attr.label(executable=True, cfg="host", default=Label("//src/java/io/bazel/rulesscala/scala_test:runner.jar"), allow_files=True),
"_scalatest_reporter": attr.label(default=Label("//scala/support:test_reporter")),
Expand Down Expand Up @@ -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):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, I'm happy for these to default to true.

ts = []
for test_file in srcs:
n = "%s_test_suite_%s" % (name, _sanitize_string_for_usage(test_file))
scala_test(name = n, srcs = [test_file], deps = deps, runtime_deps = runtime_deps, resources=resources, scalacopts=scalacopts, jvm_flags=jvm_flags, visibility=visibility, size=size)
scala_test(name = n, srcs = [test_file], deps = deps, runtime_deps = runtime_deps, resources=resources, scalacopts=scalacopts, jvm_flags=jvm_flags, visibility=visibility, size=size, colors=colors, full_stacktraces=full_stacktraces)
ts.append(n)
native.test_suite(name = name, tests = ts, visibility = visibility)

Expand Down