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

Testing toolchain and Junit deps #1102

Merged
merged 3 commits into from
Sep 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
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
45 changes: 45 additions & 0 deletions docs/testing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
## Testing toolchain configuration
Copy link
Member

Choose a reason for hiding this comment

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

The idea is that Wix can set up a testing toolchain of junit and specs2, twitter of junit and scalatest and stripe only if scalatest?
Since unneeded rules won't be evaluated and the missing deps won't be a problem? If so it will probably be worthwhile to make sure that bazel toolchains actually works like that (for example move scalatest to this and see that Wix defines only specs2 + junit in the single testing toolchain and everything works).
Finally I think it's important in clarifying this idea.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Toolchain only knows a list of labels, whether it correctly works depends on the actual rule that uses this toolchain and requests any of the dependencies. So if there is no rule that asks for JUnit, there's no need to add Junit deps to the toolchain.
Docs will be updated when there will be more rules that use it.

Copy link
Member

Choose a reason for hiding this comment

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

I think there's value in explicitly specifying this issue of actual deps.
Most of our users will not understand toolchain in the same depths you do.


Toolchain type `testing_toolchain_type` is used to set up test dependencies.

### Example to set up JUnit dependencies

`BUILD` file content in your prefered package:
```starlark
load("@io_bazel_rules_scala//scala:providers.bzl", "declare_deps_provider")
load("@io_bazel_rules_scala//testing/toolchain:toolchain.bzl", "scala_testing_toolchain")

scala_testing_toolchain(
name = "testing_toolchains_with_junit",
dep_providers = [
":junit_classpath_provider",
],
visibility = ["//visibility:public"],
)

toolchain(
name = "testing_toolchain",
toolchain = ":testing_toolchains_with_junit",
toolchain_type = "@io_bazel_rules_scala//testing/toolchain:testing_toolchain_type",
visibility = ["//visibility:public"],
)

declare_deps_provider(
name = "junit_classpath_provider",
deps_id = "junit_classpath",
visibility = ["//visibility:public"],
deps = [
"@my_hamcrest_core",
"@my_junit",
],
)
```

`junit_classpath_provider` (deps_id `junit_classpath`) is where classpath required for junit tests
is defined.

Toolchain must be registerd in your `WORKSPACE` file:
```starlark
register_toolchains('//my/package:testing_toolchain')
```

2 changes: 2 additions & 0 deletions junit/junit.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,5 @@ def junit_repositories(maven_servers = _default_maven_server_urls()):
name = "io_bazel_rules_scala/dependency/hamcrest/hamcrest_core",
actual = "@io_bazel_rules_scala_org_hamcrest_hamcrest_core//jar",
)

native.register_toolchains("@io_bazel_rules_scala//testing:testing_toolchain")
Copy link
Member

Choose a reason for hiding this comment

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

This is the default? For people who don't override?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but probably it will stay here only until Specs2 and ScalaTest will be migrated. Each test lib will have it's own configuration.

3 changes: 1 addition & 2 deletions scala/private/phases/phase_collect_jars.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ def phase_collect_jars_macro_library(ctx, p):
def phase_collect_jars_junit_test(ctx, p):
args = struct(
extra_deps = [
ctx.attr._junit,
ctx.attr._hamcrest,
ctx.attr._junit_classpath,
ctx.attr.suite_label,
ctx.attr._bazel_test_runner,
],
Expand Down
6 changes: 2 additions & 4 deletions scala/private/phases/phase_compile.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,14 @@ def phase_compile_junit_test(ctx, p):
args = struct(
buildijar = False,
implicit_junit_deps_needed_for_java_compilation = [
ctx.attr._junit,
ctx.attr._hamcrest,
ctx.attr._junit_classpath,
],
unused_dependency_checker_ignored_targets = [
target.label
for target in p.scalac_provider.default_classpath +
ctx.attr.unused_dependency_checker_ignored_targets
] + [
ctx.attr._junit.label,
ctx.attr._hamcrest.label,
ctx.attr._junit_classpath.label,
ctx.attr.suite_label.label,
ctx.attr._bazel_test_runner.label,
],
Expand Down
16 changes: 3 additions & 13 deletions scala/private/rules/scala_junit_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,8 @@ _scala_junit_test_attrs = {
mandatory = False,
),
"jvm_flags": attr.string_list(),
"_junit": attr.label(
default = Label(
"//external:io_bazel_rules_scala/dependency/junit/junit",
),
),
"_hamcrest": attr.label(
default = Label(
"//external:io_bazel_rules_scala/dependency/hamcrest/hamcrest_core",
),
"_junit_classpath": attr.label(
default = Label("@io_bazel_rules_scala//testing/toolchain:junit_classpath"),
),
"_bazel_test_runner": attr.label(
default = Label(
Expand All @@ -93,10 +86,7 @@ _junit_resolve_deps = {
Label(
"@io_bazel_rules_scala//scala/private/toolchain_deps:scala_library_classpath",
),
Label("//external:io_bazel_rules_scala/dependency/junit/junit"),
Label(
"//external:io_bazel_rules_scala/dependency/hamcrest/hamcrest_core",
),
Label("@io_bazel_rules_scala//testing/toolchain:junit_classpath"),
],
allow_files = False,
),
Expand Down
2 changes: 1 addition & 1 deletion src/java/io/bazel/rulesscala/test_discovery/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ scala_library(
"FilteredRunnerBuilder.scala",
],
visibility = ["//visibility:public"],
deps = ["//external:io_bazel_rules_scala/dependency/junit/junit"],
deps = ["@io_bazel_rules_scala//testing/toolchain:junit_classpath"],
ittaiz marked this conversation as resolved.
Show resolved Hide resolved
)
6 changes: 2 additions & 4 deletions test/aspect/aspect.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,12 @@ def _rule_impl(ctx):
"scala_junit_test": [
"//test/aspect:scala_junit_test",
"//scala/private/toolchain_deps:scala_library_classpath",
"@io_bazel_rules_scala_junit_junit//:io_bazel_rules_scala_junit_junit",
"@io_bazel_rules_scala_org_hamcrest_hamcrest_core//:io_bazel_rules_scala_org_hamcrest_hamcrest_core",
"//testing/toolchain:junit_classpath",
],
"scala_specs2_junit_test": [
"//scala/private/toolchain_deps:scala_library_classpath",
"//test/aspect:scala_specs2_junit_test",
"@io_bazel_rules_scala_junit_junit//:io_bazel_rules_scala_junit_junit",
"@io_bazel_rules_scala_org_hamcrest_hamcrest_core//:io_bazel_rules_scala_org_hamcrest_hamcrest_core",
"//testing/toolchain:junit_classpath",
# From specs2/specs2.bzl:specs2_dependencies()
"@io_bazel_rules_scala//specs2:specs2",
"@io_bazel_rules_scala_org_specs2_specs2_common//:io_bazel_rules_scala_org_specs2_specs2_common",
Expand Down
27 changes: 27 additions & 0 deletions testing/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
load("@io_bazel_rules_scala//scala:providers.bzl", "declare_deps_provider")
load("@io_bazel_rules_scala//testing/toolchain:toolchain.bzl", "scala_testing_toolchain")

scala_testing_toolchain(
name = "testing_toolchains_with_all_deps_impl",
dep_providers = [
":junit_classpath_provider",
],
visibility = ["//visibility:public"],
)

toolchain(
name = "testing_toolchain",
toolchain = ":testing_toolchains_with_all_deps_impl",
toolchain_type = "@io_bazel_rules_scala//testing/toolchain:testing_toolchain_type",
visibility = ["//visibility:public"],
)

declare_deps_provider(
name = "junit_classpath_provider",
deps_id = "junit_classpath",
visibility = ["//visibility:public"],
deps = [
"//external:io_bazel_rules_scala/dependency/hamcrest/hamcrest_core",
"//external:io_bazel_rules_scala/dependency/junit/junit",
],
)
12 changes: 12 additions & 0 deletions testing/toolchain/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
load("@io_bazel_rules_scala//testing/toolchain:toolchain_deps.bzl", "testing_toolchain_deps")

toolchain_type(
name = "testing_toolchain_type",
visibility = ["//visibility:public"],
)

testing_toolchain_deps(
name = "junit_classpath",
deps_id = "junit_classpath",
visibility = ["//visibility:public"],
)
18 changes: 18 additions & 0 deletions testing/toolchain/toolchain.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
load("//scala:providers.bzl", _DepsInfo = "DepsInfo")

def _scala_toolchain_impl(ctx):
toolchain = platform_common.ToolchainInfo(
dep_providers = ctx.attr.dep_providers,
)
return [toolchain]

scala_testing_toolchain = rule(
_scala_toolchain_impl,
attrs = {
"dep_providers": attr.label_list(
default = [
],
providers = [_DepsInfo],
),
},
)
18 changes: 18 additions & 0 deletions testing/toolchain/toolchain_deps.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
load(
"//scala/private/toolchain_deps:toolchain_deps.bzl",
"expose_toolchain_deps",
"java_info_for_deps",
)

_toolchain_type = "@io_bazel_rules_scala//testing/toolchain:testing_toolchain_type"

def _testing_toolchain_deps(ctx):
return expose_toolchain_deps(ctx, _toolchain_type)

testing_toolchain_deps = rule(
implementation = _testing_toolchain_deps,
attrs = {
"deps_id": attr.string(mandatory = True),
},
toolchains = [_toolchain_type],
)