Skip to content

Commit 067667f

Browse files
authored
Simplify test_dependency_versions.sh (#1758)
Removes almost all of the copied `deps/test` sources and targets in favor of invoking `@rules_scala` and `@multi_frameworks_toolchain` tests directly. Moves targets depending on `@rules_python` and `@rules_shell` to new packages so test packages don't break when `@rules_scala` isn't the main module. Introduces `RULES_SCALA_TARGETS`, `MULTI_FRAMEWORKS_TOOLCHAIN_TARGETS`, and `ALL_TARGETS` arrays to easily specify compatibility test targets instead of copying them. Only keeps a single `ScalafmtTest` target within `deps/test/BUILD.bazel.test`, which is a special case (described below) and executes successfully on Windows. Though these are new packages, they're comprised of existing files and targets from their parent packages: - //test/jmh/runtime - //test/sh_tests - //test/src/main/scala/scalarules/test/twitter_scrooge/strings Also: - Marks every `bazel_dep` for `@latest_dependencies` with `dev_dependency = True`. - Replaces `$(location ...)` instances in moved targets with `$(rootpath ...)` (and in one case, with `$(execpath ...)`) per bazelbuild/bazel#25198. --- While drafting a blog post describing `test_dependency_versions.sh`, I revisited the decision to copy targets and files in #1729 and #1738. Executing test targets from `@rules_scala` directly from the test module actually works, making the test far less complex, and making for better testing advice. This required moving targets that referenced dev dependencies to their own packages, fixing `@rules_scala` test package breakages. Making the `@latest_dependencies` module a dev dependency eliminated module version warnings from `@multi_frameworks_toolchain`, since the test sets older dependency versions. Then making `@latest_dependencies` a dev dependency everywhere, instead of only in `@multi_frameworks_toolchain`, seemed more consistent. The test retains the `//:ScalafmtTest` target because `rules_scala` doesn't actually provide Scalafmt rules, but only provides `ext_scalafmt` to create custom rules, per: - docs/phase_scalafmt.md - docs/customizable_phase.md I discovered this after going down a rabbit hole regarding the implicitly defined (and ostensibly deprecated) `.format` and `.format-test` predeclared outputs for Scalafmt rules. Long story short, implicitly defined predeclared outputs have been "deprecated" since 2018-03-05, predating Bazel 0.28.0: - https://bazel.build/versions/8.3.0/extending/rules#requesting_output_files - https://bazel.build/versions/8.3.0/extending/rules#deprecated_predeclared_outputs - https://bazel.build/versions/8.3.0/rules/lib/globals/bzl#rule.outputs - 2018-03-05: Output Map Madness https://docs.google.com/document/d/1ic9lJPn-0VqgKcqSbclVWwYDW2eiV-9k6ZUK_xE6H5E/edit - 2019-04-08: bazelbuild/bazel#7977: incompatible_no_rule_outputs_param: disable outputs param of rule function bazelbuild/bazel#7977 - 2019-06-07: Rollforward "Disable outputs param of rule function" with fix (introduced --incompatible_no_rule_outputs_param in Bazel 0.28.0) bazelbuild/bazel@36c70a6 - 2019-09-04: Document the replacements for the `outputs` parameter to the `rule()` function. (Bazel 1.0.0) bazelbuild/bazel@e29ddda However, I learned that the `.format` and `.format-test` targets are Bash scripts run _after_ the original rule executes Scalafmt. The generated scripts don't depend on the `//scala/scalafmt:scalafmt` binary at all. Plus, they only work when invoked within the main module, since they reference relative paths within `BUILD_WORKSPACE_DIRECTORY`. And finally, `bazel test @rules_scala//test/scalafmt:formatted-test` fails because it tries to declare output files in a nonexistent directory. ```txt ERROR: .../external/rules_scala~/test/scalafmt/BUILD:43:20: in scalafmt_scala_test rule @@rules_scala~//test/scalafmt:formatted-test: Traceback (most recent call last): File ".../external/rules_scala~/scala/private/rules/scala_test.bzl", line 38, column 22, in _scala_test_impl return run_phases( File ".../external/rules_scala~/scala/private/phases/api.bzl", line 45, column 23, in run_phases return _run_phases(ctx, builtin_customizable_phases, target = None) File ".../external/rules_scala~/scala/private/phases/api.bzl", line 77, column 32, in _run_phases new_provider = function(ctx, current_provider) File ".../external/rules_scala~/scala/private/phases/phase_scalafmt.bzl", line 10, column 46, in phase_scalafmt manifest, files, srcs = _build_format(ctx) File ".../external/rules_scala~/scala/private/phases/phase_scalafmt.bzl", line 30, column 44, in _build_format file = ctx.actions.declare_file("{}.fmt.output".format(src.short_path)) Error in declare_file: the output artifact 'external/rules_scala~/test/rules_scala~/test/scalafmt/formatted/formatted-test.scala.fmt.output' is not under package directory 'external/rules_scala~/test/scalafmt' for target '@@rules_scala~//test/scalafmt:formatted-test' ERROR: .../external/rules_scala~/test/scalafmt/BUILD:43:20: Analysis of target '@@rules_scala~//test/scalafmt:formatted-test' failed ``` So invoking `bazel test //:ScalafmtTest` (via `/...` from `ALL_TARGETS`) is sufficient, since we're only validating that the toolchains execute properly. (`test_scalafmt.sh` tests the behavior of the `.format` and `.format-test` scripts, _including_ on Windows via `--run_under=bash`.) This also means we can invoke this target on Windows, instead of having special case logic to avoid invoking the previous `bazel run` command.
1 parent aaf28f8 commit 067667f

File tree

39 files changed

+281
-479
lines changed

39 files changed

+281
-479
lines changed

deps/test/BUILD.bazel.test

Lines changed: 4 additions & 149 deletions
Original file line numberDiff line numberDiff line change
@@ -1,156 +1,11 @@
1-
"""Test targets to ensure dependency version compatibility.
1+
"""Test targets to ensure dependency version compatibility."""
22

3-
Copied and adapted targets from the main repo as noted.
4-
"""
5-
load(
6-
":defs.bzl",
7-
"default_outputs_test",
8-
"scalafmt_scala_test",
9-
"scrooge_transitive_outputs_test",
10-
)
11-
load("@rules_java//java:defs.bzl", "java_library")
12-
load("@rules_proto//proto:defs.bzl", "proto_library")
13-
load("@rules_scala//jmh:jmh.bzl", "scala_benchmark_jmh")
14-
load("@rules_scala//scala/scalafmt:phase_scalafmt_ext.bzl", "ext_scalafmt")
15-
load("@rules_scala//scala:advanced_usage/scala.bzl", "make_scala_test")
16-
load(
17-
"@rules_scala//scala:scala.bzl",
18-
"scala_binary",
19-
"scala_doc",
20-
"scala_junit_test",
21-
"scala_library",
22-
"scala_specs2_junit_test",
23-
"scala_test",
24-
)
25-
load("@rules_scala//scala_proto:scala_proto.bzl", "scala_proto_library")
26-
load("@rules_scala//thrift:thrift.bzl", "thrift_library")
27-
load(
28-
"@rules_scala//twitter_scrooge:twitter_scrooge.bzl",
29-
"scrooge_java_library",
30-
"scrooge_scala_library",
31-
)
32-
33-
# From: `test/BUILD`
34-
scala_binary(
35-
name = "ScalaBinary",
36-
srcs = ["ScalaBinary.scala"],
37-
main_class = "scalarules.test.ScalaBinary",
38-
deps = [
39-
":HelloLib",
40-
],
41-
)
42-
43-
scala_library(
44-
name = "HelloLib",
45-
srcs = ["HelloLib.scala"],
46-
)
47-
48-
scala_doc(
49-
name = "ScalaDoc",
50-
deps = [":HelloLib"],
51-
)
52-
53-
# From: `examples/testing/multi_frameworks_toolchain/example/BUILD`
54-
scala_test(
55-
name = "scalatest_example",
56-
srcs = ["ScalaTestExampleTest.scala"],
57-
)
3+
load(":defs.bzl", "scalafmt_scala_test")
584

59-
scala_specs2_junit_test(
60-
name = "specs2_example",
61-
srcs = ["Specs2ExampleTest.scala"],
62-
suffixes = ["Test"],
63-
)
64-
65-
# Manufactured based on `docs/phase_scalafmt.md` and `test/scalafmt/BUILD`.
5+
# Based on `docs/phase_scalafmt.md`, `test/scalafmt/BUILD`, and a copy of:
6+
# examples/testing/multi_frameworks_toolchain/example/ScalaTestExampleTest.scala
667
scalafmt_scala_test(
678
name = "ScalafmtTest",
689
srcs = ["ScalaTestExampleTest.scala"],
6910
format = True,
7011
)
71-
72-
# From: `test/proto/BUILD`
73-
proto_library(
74-
name = "standalone_proto",
75-
srcs = ["standalone.proto"],
76-
)
77-
78-
scala_proto_library(
79-
name = "standalone_scala_proto",
80-
deps = [":standalone_proto"],
81-
)
82-
83-
default_outputs_test(
84-
name = "standalone_scala_proto_outs_test",
85-
expected_outs = [
86-
"standalone_proto_scalapb-src.jar",
87-
"standalone_proto_scalapb.jar",
88-
],
89-
target_under_test = ":standalone_scala_proto",
90-
)
91-
92-
# From: `test/jmh/BUILD`
93-
java_library(
94-
name = "java_type",
95-
srcs = ["JavaType.java"],
96-
visibility = ["//visibility:public"],
97-
)
98-
99-
scala_library(
100-
name = "scala_type",
101-
srcs = ["ScalaType.scala"],
102-
visibility = ["//visibility:public"],
103-
)
104-
105-
scala_library(
106-
name = "add_numbers",
107-
srcs = ["AddNumbers.scala"],
108-
visibility = ["//visibility:public"],
109-
exports = [
110-
":java_type",
111-
":scala_type",
112-
],
113-
deps = [
114-
":java_type",
115-
":scala_type",
116-
],
117-
)
118-
119-
scala_benchmark_jmh(
120-
name = "test_benchmark",
121-
srcs = ["TestBenchmark.scala"],
122-
data = ["data.txt"],
123-
deps = [":add_numbers"],
124-
)
125-
126-
# From: `test/src/main/scala/scalarules/test/twitter_scrooge/BUILD`
127-
128-
thrift_library(
129-
name = "thrift3",
130-
srcs = ["Thrift3.thrift"],
131-
visibility = ["//visibility:public"],
132-
)
133-
134-
scrooge_scala_library(
135-
name = "scrooge3",
136-
visibility = ["//visibility:public"],
137-
deps = [":thrift3"],
138-
)
139-
140-
scrooge_java_library(
141-
name = "scrooge3_java",
142-
visibility = ["//visibility:public"],
143-
deps = [":thrift3"],
144-
)
145-
146-
scrooge_transitive_outputs_test(
147-
name = "scrooge_test_scala",
148-
dep = ":scrooge3",
149-
expected_jars = ["thrift3_scrooge_scala.jar"],
150-
)
151-
152-
scrooge_transitive_outputs_test(
153-
name = "scrooge_test_java",
154-
dep = ":scrooge3_java",
155-
expected_jars = ["thrift3_scrooge_java.jar"],
156-
)

deps/test/HelloLib.scala

Lines changed: 0 additions & 22 deletions
This file was deleted.

deps/test/MODULE.bazel.template

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,19 @@
11
"""Bazel module template for test/shell/test_deps_versions.sh tests."""
22

3-
module(
4-
name = "rules_scala_deps_versions_test",
5-
bazel_compatibility = [">=${bazelversion}"],
6-
)
3+
module(name = "rules_scala_deps_versions_test")
74

85
bazel_dep(name = "rules_scala")
96
local_path_override(
107
module_name = "rules_scala",
118
path = "../..",
129
)
1310

11+
bazel_dep(name = "multi_frameworks_toolchain")
12+
local_path_override(
13+
module_name = "multi_frameworks_toolchain",
14+
path = "../../examples/testing/multi_frameworks_toolchain",
15+
)
16+
1417
bazel_dep(name = "bazel_skylib")
1518
single_version_override(
1619
module_name = "bazel_skylib",

deps/test/ScalaBinary.scala

Lines changed: 0 additions & 21 deletions
This file was deleted.

deps/test/defs.bzl

Lines changed: 2 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,7 @@
1-
load("@bazel_skylib//lib:collections.bzl", "collections")
2-
load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts", "unittest")
3-
load("@rules_java//java/common:java_info.bzl", "JavaInfo")
1+
"""Utilities required for dependency compatibility tests."""
2+
43
load("@rules_scala//scala:advanced_usage/scala.bzl", "make_scala_test")
54
load("@rules_scala//scala/scalafmt:phase_scalafmt_ext.bzl", "ext_scalafmt")
65

76
# From //test/scalafmt:phase_scalafmt_test.bzl
87
scalafmt_scala_test = make_scala_test(ext_scalafmt)
9-
10-
# From //test/proto:default_outputs_test.bzl
11-
def _default_outputs_test(ctx):
12-
env = analysistest.begin(ctx)
13-
14-
target_under_test = analysistest.target_under_test(env)
15-
actual_outs = [f.basename for f in target_under_test[DefaultInfo].files.to_list()]
16-
17-
asserts.equals(env, sorted(ctx.attr.expected_outs), sorted(actual_outs))
18-
19-
return analysistest.end(env)
20-
21-
default_outputs_test = analysistest.make(
22-
_default_outputs_test,
23-
attrs = {
24-
"expected_outs": attr.string_list(),
25-
},
26-
)
27-
28-
# From
29-
# //test/src/main/scala/scalarules/test/twitter_scrooge:twitter_scrooge_test.bzl
30-
def _scrooge_transitive_outputs(ctx):
31-
env = unittest.begin(ctx)
32-
33-
asserts.equals(
34-
env,
35-
sorted(ctx.attr.expected_jars),
36-
sorted(collections.uniq([out.class_jar.basename for out in ctx.attr.dep[JavaInfo].outputs.jars])),
37-
)
38-
39-
return unittest.end(env)
40-
41-
scrooge_transitive_outputs_test = unittest.make(
42-
_scrooge_transitive_outputs,
43-
attrs = {
44-
"dep": attr.label(),
45-
"expected_jars": attr.string_list(),
46-
},
47-
)

dt_patches/test_dt_patches/MODULE.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ local_path_override(
88
path = "../..",
99
)
1010

11-
bazel_dep(name = "latest_dependencies")
11+
bazel_dep(name = "latest_dependencies", dev_dependency = True)
1212
local_path_override(
1313
module_name = "latest_dependencies",
1414
path = "../../deps/latest",

dt_patches/test_dt_patches_user_srcjar/MODULE.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ local_path_override(
88
path = "../..",
99
)
1010

11-
bazel_dep(name = "latest_dependencies")
11+
bazel_dep(name = "latest_dependencies", dev_dependency = True)
1212
local_path_override(
1313
module_name = "latest_dependencies",
1414
path = "../../deps/latest",

examples/crossbuild/MODULE.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ local_path_override(
88
path = "../..",
99
)
1010

11-
bazel_dep(name = "latest_dependencies")
11+
bazel_dep(name = "latest_dependencies", dev_dependency = True)
1212
local_path_override(
1313
module_name = "latest_dependencies",
1414
path = "../../deps/latest",

examples/overridden_artifacts/MODULE.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ local_path_override(
88
path = "../..",
99
)
1010

11-
bazel_dep(name = "latest_dependencies")
11+
bazel_dep(name = "latest_dependencies", dev_dependency = True)
1212
local_path_override(
1313
module_name = "latest_dependencies",
1414
path = "../../deps/latest",

examples/scala3/MODULE.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ local_path_override(
88
path = "../..",
99
)
1010

11-
bazel_dep(name = "latest_dependencies")
11+
bazel_dep(name = "latest_dependencies", dev_dependency = True)
1212
local_path_override(
1313
module_name = "latest_dependencies",
1414
path = "../../deps/latest",

0 commit comments

Comments
 (0)