Fix starlark action with shadowed_action inputs cache test
The test was flaky mainly due to using `&` instead of `&&` to execute two consecutive commands in the aspect action.
This CL contains other improvements:
- using timestamp in the test files instead of date-time to be more accurate.
- splitting the test method into two tests to clarify the purpose of each of them.
PiperOrigin-RevId: 379793635
diff --git a/src/test/shell/integration/action_aspect_test.sh b/src/test/shell/integration/action_aspect_test.sh
index 73e227b..8291db6 100755
--- a/src/test/shell/integration/action_aspect_test.sh
+++ b/src/test/shell/integration/action_aspect_test.sh
@@ -233,57 +233,12 @@
|| fail "aspect params file does not contain tree artifact args"
}
-# Test the inputs cache of Starlark actions triggered from aspects
-function test_starlark_action_inputs_cache() {
- local package="a"
- mkdir -p "${package}"
+# Test that a starlark action shadowing another action is not rerun after blaze
+# shutdown.
+function test_starlark_action_with_shadowed_action_not_rerun_after_shutdown() {
+ local package="a1"
- cat > "${package}/lib.bzl" <<EOF
-def _actions_test_impl(target, ctx):
- compile_action = None
-
- for action in target.actions:
- if action.mnemonic == "CppCompile":
- compile_action = action
-
- if not compile_action:
- fail("Couln't find compile action")
-
- aspect_out = ctx.actions.declare_file("run_timestamp")
- ctx.actions.run_shell(
- shadowed_action = compile_action,
- mnemonic = "AspectAction",
- outputs = [aspect_out],
- # record the timestamp in output file to validate action rerun
- command = "cat ${package}/x.h > %s & date >> %s" % (
- aspect_out.path,
- aspect_out.path,
- ),
- )
-
- return [OutputGroupInfo(out = [aspect_out])]
-
-actions_test_aspect = aspect(implementation = _actions_test_impl)
-EOF
-
-echo "inline int x() { return 42; }" > "${package}/x.h"
- cat > "${package}/a.cc" <<EOF
-#include "${package}/x.h"
-
-int a() { return x(); }
-EOF
- cat > "${package}/BUILD" <<EOF
-cc_library(
- name = "x",
- hdrs = ["x.h"],
-)
-
-cc_library(
- name = "a",
- srcs = ["a.cc"],
- deps = [":x"],
-)
-EOF
+ create_starlark_action_with_shadowed_action_cache_test_files "${package}"
bazel build "${package}:a" \
--aspects="//${package}:lib.bzl%actions_test_aspect" \
@@ -305,23 +260,90 @@
diff "${package}/run_1_timestamp" "${package}/run_2_timestamp" \
|| fail "Starlark action should not rerun after bazel shutdown"
+}
-# TODO(b/191195108): Disabled due to flakiness. Should be its own test as well.
-# # Test that the Starlark action would rerun if the inputs of the
-# # shadowed action changed
-# rm "${package}/x.h"
-# echo "inline int x() { return 0; }" > "${package}/x.h"
+# Test that a starlark action shadowing another action is rerun if the inputs
+# of the shadowed_action change.
+function test_starlark_action_rerun_after_shadowed_action_inputs_change() {
+ local package="a2"
-# bazel build "${package}:a" \
-# --aspects="//${package}:lib.bzl%actions_test_aspect" \
-# --output_groups=out || \
-# fail "bazel build should've succeeded"
+ create_starlark_action_with_shadowed_action_cache_test_files "${package}"
-# cp "bazel-bin/${package}/run_timestamp" "${package}/run_3_timestamp"
+ bazel build "${package}:a" \
+ --aspects="//${package}:lib.bzl%actions_test_aspect" \
+ --output_groups=out || \
+ fail "bazel build should've succeeded"
-# diff "${package}/run_1_timestamp" "${package}/run_3_timestamp" \
-# && fail "Starlark action should rerun after shadowed action inputs change" \
-# || :
+ cp "bazel-bin/${package}/run_timestamp" "${package}/run_1_timestamp"
+
+ # Test that the Starlark action would rerun if the inputs of the
+ # shadowed action changed
+ rm -f "${package}/x.h"
+ echo "inline int x() { return 0; }" > "${package}/x.h"
+
+ bazel build "${package}:a" \
+ --aspects="//${package}:lib.bzl%actions_test_aspect" \
+ --output_groups=out || \
+ fail "bazel build should've succeeded"
+
+ cp "bazel-bin/${package}/run_timestamp" "${package}/run_2_timestamp"
+
+ diff "${package}/run_1_timestamp" "${package}/run_2_timestamp" \
+ && fail "Starlark action should rerun after shadowed action inputs change" \
+ || :
+}
+
+function create_starlark_action_with_shadowed_action_cache_test_files() {
+ local package="$1"
+
+ mkdir -p "${package}"
+
+ cat > "${package}/lib.bzl" <<EOF
+def _actions_test_impl(target, ctx):
+ compile_action = None
+
+ for action in target.actions:
+ if action.mnemonic == "CppCompile":
+ compile_action = action
+
+ if not compile_action:
+ fail("Couln't find compile action")
+
+ aspect_out = ctx.actions.declare_file("run_timestamp")
+ ctx.actions.run_shell(
+ shadowed_action = compile_action,
+ mnemonic = "AspectAction",
+ outputs = [aspect_out],
+ # record the timestamp in output file to validate action rerun
+ command = "cat ${package}/x.h > %s && date '+%%s' >> %s" % (
+ aspect_out.path,
+ aspect_out.path,
+ ),
+ )
+
+ return [OutputGroupInfo(out = [aspect_out])]
+
+actions_test_aspect = aspect(implementation = _actions_test_impl)
+EOF
+
+ echo "inline int x() { return 42; }" > "${package}/x.h"
+ cat > "${package}/a.cc" <<EOF
+#include "${package}/x.h"
+
+int a() { return x(); }
+EOF
+ cat > "${package}/BUILD" <<EOF
+cc_library(
+ name = "x",
+ hdrs = ["x.h"],
+)
+
+cc_library(
+ name = "a",
+ srcs = ["a.cc"],
+ deps = [":x"],
+)
+EOF
}
run_suite "Tests Starlark API pertaining to action inspection via aspect"
