Skip to content

Commit c56b4a5

Browse files
MivrMivr
authored
feat: add action environment variables to RunEnvironmentInfo (#2303)
Fixes: #2077 ### Changes are visible to end-users: no ### Test plan - new tests added
1 parent 197401b commit c56b4a5

File tree

4 files changed

+183
-7
lines changed

4 files changed

+183
-7
lines changed

‎js/private/js_binary.bzl‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -573,12 +573,23 @@ def _js_binary_impl(ctx):
573573
runfiles = launcher.runfiles
574574

575575
providers = []
576+
577+
# Create RunEnvironmentInfo provider with both env and env_inherit (if available)
578+
run_env_info_kwargs = {}
579+
580+
if ctx.attr.env:
581+
action_context_env_expanded = {}
582+
for key, value in ctx.attr.env.items():
583+
action_context_env_expanded[key] = _expand_env_if_needed(ctx, value)
584+
run_env_info_kwargs["environment"] = action_context_env_expanded
585+
586+
# Add inherited environment variables (for js_test)
576587
if hasattr(ctx.attr, "env_inherit"):
577-
providers.append(
578-
RunEnvironmentInfo(
579-
inherited_environment = ctx.attr.env_inherit,
580-
),
581-
)
588+
run_env_info_kwargs["inherited_environment"] = ctx.attr.env_inherit
589+
590+
# Only create provider if we have something to provide
591+
if run_env_info_kwargs:
592+
providers.append(RunEnvironmentInfo(**run_env_info_kwargs))
582593

583594
if ctx.attr.testonly and ctx.configuration.coverage_enabled:
584595
# We have to propagate _lcov_merger runfiles since bazel does not treat _lcov_merger as a proper tool.
@@ -591,7 +602,7 @@ def _js_binary_impl(ctx):
591602
# TODO: Remove once bazel<8 support is dropped.
592603
if hasattr(ctx.attr, "_lcov_merger"):
593604
runfiles = runfiles.merge(ctx.attr._lcov_merger[DefaultInfo].default_runfiles)
594-
providers = [
605+
providers.append(
595606
coverage_common.instrumented_files_info(
596607
ctx,
597608
source_attributes = ["data"],
@@ -609,7 +620,7 @@ def _js_binary_impl(ctx):
609620
"tsx",
610621
],
611622
),
612-
]
623+
)
613624

614625
return providers + [
615626
DefaultInfo(

‎js/private/test/BUILD.bazel‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ load("@aspect_bazel_lib_host//:defs.bzl", "host")
33
load("@bazel_skylib//rules:write_file.bzl", "write_file")
44
load("//js:defs.bzl", "js_binary", "js_library", "js_test")
55
load(":js_library_test.bzl", "js_library_test_suite")
6+
load(":run_environment_info_test.bzl", "run_environment_info_test_suite")
67

78
####################################################################################################
89
# Write a js_binary launcher to the source tree so it is shell checked on commit
@@ -42,6 +43,8 @@ write_source_files(
4243

4344
js_library_test_suite(name = "js_library_test")
4445

46+
run_environment_info_test_suite(name = "run_environment_info_tests")
47+
4548
# js_library(data) wrapper of the data
4649
js_library(
4750
name = "data-js_library-data",

‎js/private/test/run_environment_info_test.bzl‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
"""Tests for RunEnvironmentInfo provider in js_binary and js_test rules."""
2+
3+
load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts")
4+
load("//js:defs.bzl", "js_binary", "js_test")
5+
6+
def _run_environment_info_test_impl(ctx):
7+
env = analysistest.begin(ctx)
8+
target_under_test = analysistest.target_under_test(env)
9+
10+
if ctx.attr.expect_no_provider:
11+
asserts.false(
12+
env,
13+
RunEnvironmentInfo in target_under_test,
14+
"RunEnvironmentInfo provider should NOT be present when no env vars are set",
15+
)
16+
return analysistest.end(env)
17+
18+
asserts.true(
19+
env,
20+
RunEnvironmentInfo in target_under_test,
21+
"RunEnvironmentInfo provider should be present",
22+
)
23+
24+
run_env_info = target_under_test[RunEnvironmentInfo]
25+
26+
if ctx.attr.expect_environment:
27+
asserts.true(
28+
env,
29+
hasattr(run_env_info, "environment"),
30+
"environment field should exist in RunEnvironmentInfo",
31+
)
32+
33+
for key, expected in ctx.attr.expect_environment.items():
34+
actual = run_env_info.environment.get(key)
35+
asserts.true(
36+
env,
37+
actual != None,
38+
"Key '{}' should exist in environment".format(key),
39+
)
40+
41+
if "$(location" in expected:
42+
asserts.true(
43+
env,
44+
"data.json" in actual,
45+
"Location should have been expanded for '{}'".format(key),
46+
)
47+
else:
48+
asserts.equals(env, expected, actual)
49+
50+
if ctx.attr.expect_inherited:
51+
asserts.true(
52+
env,
53+
hasattr(run_env_info, "inherited_environment"),
54+
"inherited_environment field should exist in RunEnvironmentInfo",
55+
)
56+
asserts.equals(
57+
env,
58+
sorted(ctx.attr.expect_inherited),
59+
sorted(run_env_info.inherited_environment),
60+
)
61+
62+
return analysistest.end(env)
63+
64+
run_environment_info_test = analysistest.make(
65+
_run_environment_info_test_impl,
66+
attrs = {
67+
"expect_environment": attr.string_dict(
68+
doc = "Expected environment variables and their values",
69+
),
70+
"expect_inherited": attr.string_list(
71+
doc = "Expected inherited environment variable names",
72+
),
73+
"expect_no_provider": attr.bool(
74+
default = False,
75+
doc = "If true, expect that RunEnvironmentInfo provider is NOT present",
76+
),
77+
},
78+
)
79+
80+
def run_environment_info_test_suite(name):
81+
"""Test suite for RunEnvironmentInfo provider.
82+
83+
Args:
84+
name: Name of the test suite
85+
"""
86+
87+
js_binary(
88+
name = name + "_binary_env_subject",
89+
entry_point = "test_env.js",
90+
env = {
91+
"BINARY_VAR": "binary_value",
92+
"ANOTHER_VAR": "another_value",
93+
"LOCATION_VAR": "$(location :data.json)",
94+
},
95+
data = [":data.json"],
96+
)
97+
98+
run_environment_info_test(
99+
name = name + "_binary_env_test",
100+
target_under_test = ":" + name + "_binary_env_subject",
101+
expect_environment = {
102+
"BINARY_VAR": "binary_value",
103+
"ANOTHER_VAR": "another_value",
104+
"LOCATION_VAR": "$(location :data.json)",
105+
},
106+
)
107+
108+
js_test(
109+
name = name + "_test_both_subject",
110+
entry_point = "test_env.js",
111+
env = {
112+
"TEST_VAR": "test_value",
113+
"EXPANDED_PATH": "$(location :data.json)",
114+
},
115+
env_inherit = ["PATH", "HOME"],
116+
data = [":data.json"],
117+
)
118+
119+
run_environment_info_test(
120+
name = name + "_test_both_test",
121+
target_under_test = ":" + name + "_test_both_subject",
122+
expect_environment = {
123+
"TEST_VAR": "test_value",
124+
"EXPANDED_PATH": "$(location :data.json)",
125+
},
126+
expect_inherited = ["PATH", "HOME"],
127+
)
128+
129+
js_test(
130+
name = name + "_test_inherit_only_subject",
131+
entry_point = "test_env.js",
132+
env_inherit = ["USER", "SHELL"],
133+
)
134+
135+
run_environment_info_test(
136+
name = name + "_test_inherit_only_test",
137+
target_under_test = ":" + name + "_test_inherit_only_subject",
138+
expect_inherited = ["USER", "SHELL"],
139+
)
140+
141+
js_binary(
142+
name = name + "_binary_no_env_subject",
143+
entry_point = "test_env.js",
144+
)
145+
146+
run_environment_info_test(
147+
name = name + "_binary_no_env_test",
148+
target_under_test = ":" + name + "_binary_no_env_subject",
149+
expect_no_provider = True,
150+
)
151+
152+
native.test_suite(
153+
name = name,
154+
tests = [
155+
":" + name + "_binary_env_test",
156+
":" + name + "_test_both_test",
157+
":" + name + "_test_inherit_only_test",
158+
":" + name + "_binary_no_env_test",
159+
],
160+
)

‎js/private/test/test_env.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// Test entry point for RunEnvironmentInfo tests
2+
console.log("Test entry point for RunEnvironmentInfo tests");

0 commit comments

Comments
 (0)