Skip to content

Commit

Permalink
Mark reset targets as testonly when modifying test rules (#157)
Browse files Browse the repository at this point in the history
We need to mark reset targets as `testonly` when modifying test rules.
Otherwise, `transitioning_alias` target might depend on a `testonly`
target while being a non-test target. And we get the following error:

```
ERROR: /Users/halil.sener/Projects/datadog/oss/with_cfg.bzl/examples/java_21_library_with_reset/BUILD.bazel:48:13: in java_21_test_reset rule //examples/java_21_library_with_reset:tests__deps_0: non-test target '//examples/java_21_library_with_reset:tests__deps_0' depends on testonly target '//examples/java_21_library_with_reset:testonly_legacy_lib' and doesn't have testonly attribute set
ERROR: /Users/halil.sener/Projects/datadog/oss/with_cfg.bzl/examples/java_21_library_with_reset/BUILD.bazel:48:13: Analysis of target '//examples/java_21_library_with_reset:tests__deps_0' failed
```

Diff to reproduce (note that `:testonly_legacy_lib` is testonly):

```diff
diff --git a/examples/java_21_library_with_reset/BUILD.bazel b/examples/java_21_library_with_reset/BUILD.bazel
index c5ca192..925aa31 100644
--- a/examples/java_21_library_with_reset/BUILD.bazel
+++ b/examples/java_21_library_with_reset/BUILD.bazel
@@ -1,5 +1,5 @@
 load("@with_cfg.bzl", "original_settings")
-load(":java_21_library.bzl", "java_21_library", "java_21_library_reset")
+load(":java_21_library.bzl", "java_21_library", "java_21_library_reset", "java_21_test")
 
 java_binary(
     name = "Main",
@@ -39,6 +39,19 @@ java_library(
     ],
 )
 
+java_library(
+    name = "testonly_legacy_lib",
+    srcs = ["src/main/java/com/example/LegacyIllegalArgumentException.java"],
+    testonly = True,
+)
+
+java_21_test(
+    name = "tests",
+    srcs = ["src/main/java/com/example/IntegerType.java"],
+    test_class = "com.example.IntegerTypeTest",
+    deps = [":testonly_legacy_lib"],
+)
+
 # This target exists purely to demonstrate (and verify) that implicit outputs of
 # the underlying rule, in this case source jars, are available on the rule
 # produced by with_cfg.
@@ -50,3 +63,7 @@ alias(
 original_settings(
     name = "java_21_library_original_settings",
 )
+
+original_settings(
+    name = "java_21_test_original_settings",
+)
diff --git a/examples/java_21_library_with_reset/java_21_library.bzl b/examples/java_21_library_with_reset/java_21_library.bzl
index 275f21d..52b8386 100644
--- a/examples/java_21_library_with_reset/java_21_library.bzl
+++ b/examples/java_21_library_with_reset/java_21_library.bzl
@@ -4,3 +4,9 @@ _builder = with_cfg(native.java_library)
 _builder.set("java_language_version", "21").set("java_runtime_version", "remotejdk_21")
 _builder.resettable(Label(":java_21_library_original_settings"))
 java_21_library, java_21_library_reset = _builder.build()
+
+_test_builder = with_cfg(native.java_test)
+_test_builder.set("java_language_version", "21").set("java_runtime_version", "remotejdk_21")
+_test_builder.resettable(Label(":java_21_test_original_settings"))
+_test_builder.reset_on_attrs("deps", "runtime_deps")
+java_21_test, java_21_test_reset = _test_builder.build()

```

```sh
$ bazel query --output build //examples/java_21_library_with_reset:tests_/tests  
# /Users/halil.sener/Projects/datadog/oss/with_cfg.bzl/examples/java_21_library_with_reset/BUILD.bazel:48:13
java_test(
  name = "tests_/tests",
  visibility = ["//visibility:private"],
  tags = ["manual"],
  generator_name = "tests",
  generator_function = "lambda",
  generator_location = "examples/java_21_library_with_reset/BUILD.bazel:48:13",
  testonly = True,
  test_class = "com.example.IntegerTypeTest",
  srcs = ["//examples/java_21_library_with_reset:src/main/java/com/example/IntegerType.java"],
  deps = ["//examples/java_21_library_with_reset:tests__deps_0"],
  launcher = "@bazel_tools//tools/jdk:launcher_flag_alias",
)

$ bazel query --output build //examples/java_21_library_with_reset:tests__deps_0
# /Users/halil.sener/Projects/datadog/oss/with_cfg.bzl/examples/java_21_library_with_reset/BUILD.bazel:48:13
java_21_test_reset(
  name = "tests__deps_0",
  visibility = ["//visibility:private"],
  tags = ["manual"],
  generator_name = "tests",
  generator_function = "lambda",
  generator_location = "examples/java_21_library_with_reset/BUILD.bazel:48:13",
  exports = "//examples/java_21_library_with_reset:testonly_legacy_lib",
)

$ bazel query --output build //examples/java_21_library_with_reset:testonly_legacy_lib
# /Users/halil.sener/Projects/datadog/oss/with_cfg.bzl/examples/java_21_library_with_reset/BUILD.bazel:42:13
java_library(
  name = "testonly_legacy_lib",
  generator_name = "testonly_legacy_lib",
  generator_function = "java_library",
  generator_location = "examples/java_21_library_with_reset/BUILD.bazel:42:13",
  testonly = True,
  srcs = ["//examples/java_21_library_with_reset:src/main/java/com/example/LegacyIllegalArgumentException.java"],
)
```
  • Loading branch information
hisener authored Feb 7, 2025
1 parent bf2bd54 commit 178f719
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 1 deletion.
19 changes: 18 additions & 1 deletion examples/java_21_library_with_reset/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
load("@with_cfg.bzl", "original_settings")
load(":java_21_library.bzl", "java_21_library", "java_21_library_reset")
load(":java_21_library.bzl", "java_21_library", "java_21_library_reset", "java_21_test")

java_binary(
name = "Main",
Expand Down Expand Up @@ -39,6 +39,19 @@ java_library(
],
)

java_library(
name = "testonly_legacy_lib",
srcs = ["src/main/java/com/example/LegacyIllegalArgumentException.java"],
testonly = True,
)

java_21_test(
name = "tests",
srcs = ["src/test/java/com/example/LegacyIllegalArgumentExceptionTest.java"],
test_class = "com.example.LegacyIllegalArgumentExceptionTest",
deps = [":testonly_legacy_lib"],
)

# This target exists purely to demonstrate (and verify) that implicit outputs of
# the underlying rule, in this case source jars, are available on the rule
# produced by with_cfg.
Expand All @@ -50,3 +63,7 @@ alias(
original_settings(
name = "java_21_library_original_settings",
)

original_settings(
name = "java_21_test_original_settings",
)
6 changes: 6 additions & 0 deletions examples/java_21_library_with_reset/java_21_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,9 @@ _builder = with_cfg(native.java_library)
_builder.set("java_language_version", "21").set("java_runtime_version", "remotejdk_21")
_builder.resettable(Label(":java_21_library_original_settings"))
java_21_library, java_21_library_reset = _builder.build()

_test_builder = with_cfg(native.java_test)
_test_builder.set("java_language_version", "21").set("java_runtime_version", "remotejdk_21")
_test_builder.resettable(Label(":java_21_test_original_settings"))
_test_builder.reset_on_attrs("deps", "runtime_deps")
java_21_test, java_21_test_reset = _test_builder.build()
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.example;

import static org.junit.Assert.assertTrue;

import org.junit.Test;

public final class LegacyIllegalArgumentExceptionTest {
@Test
public void legacyIllegalArgumentException() {
assertTrue(IllegalArgumentException.class.isAssignableFrom(LegacyIllegalArgumentException.class));
}
}
1 change: 1 addition & 0 deletions with_cfg/private/wrapper.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ def _wrapper(
reset_target = lambda *, name, exports: transitioning_alias(
name = name,
exports = exports,
testonly = common_attrs.get("testonly", False),
tags = ["manual"],
visibility = ["//visibility:private"],
),
Expand Down

0 comments on commit 178f719

Please sign in to comment.