Skip to content

Commit 7e07684

Browse files
authored
tests(pystar): add analysis tests to cover basic windows building (#1452)
The CI currently only runs on Ubuntu, so it assumes that is the target platform. This ends up missing some notable Windows code paths, though. Since its analysis-phase logic, we can force the platform to be Windows for the analysis tests, and then the rules follow the code paths that should be taken under Windows. This allows testing Windows logic under Ubuntu.
1 parent 8d7645e commit 7e07684

File tree

8 files changed

+139
-25
lines changed

8 files changed

+139
-25
lines changed

python/private/common/py_executable.bzl

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ load(
3535
"create_py_info",
3636
"csv",
3737
"filter_to_py_srcs",
38+
"target_platform_has_any_constraint",
3839
"union_attrs",
3940
)
4041
load(
@@ -48,6 +49,7 @@ load(
4849
"ALLOWED_MAIN_EXTENSIONS",
4950
"BUILD_DATA_SYMLINK_PATH",
5051
"IS_BAZEL",
52+
"PLATFORMS_LOCATION",
5153
"PY_RUNTIME_ATTR_NAME",
5254
"TOOLS_REPO",
5355
)
@@ -93,6 +95,11 @@ filename in `srcs`, `main` must be specified.
9395
# NOTE: Some tests care about the order of these values.
9496
values = ["PY2", "PY3"],
9597
),
98+
"_windows_constraints": attr.label_list(
99+
default = [
100+
PLATFORMS_LOCATION + "/os:windows",
101+
],
102+
),
96103
},
97104
create_srcs_version_attr(values = SRCS_VERSION_ALL_VALUES),
98105
create_srcs_attr(mandatory = True),
@@ -201,8 +208,7 @@ def _validate_executable(ctx):
201208
check_native_allowed(ctx)
202209

203210
def _compute_outputs(ctx, output_sources):
204-
# TODO: This should use the configuration instead of the Bazel OS.
205-
if _py_builtins.get_current_os_name() == "windows":
211+
if target_platform_has_any_constraint(ctx, ctx.attr._windows_constraints):
206212
executable = ctx.actions.declare_file(ctx.label.name + ".exe")
207213
else:
208214
executable = ctx.actions.declare_file(ctx.label.name)

python/private/common/py_executable_bazel.bzl

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ load(
2121
"create_binary_semantics_struct",
2222
"create_cc_details_struct",
2323
"create_executable_result_struct",
24+
"target_platform_has_any_constraint",
2425
"union_attrs",
2526
)
2627
load(":common_bazel.bzl", "collect_cc_info", "get_imports", "maybe_precompile")
@@ -174,9 +175,7 @@ def _create_executable(
174175
runtime_details = runtime_details,
175176
)
176177

177-
# TODO: This should use the configuration instead of the Bazel OS.
178-
# This is just legacy behavior.
179-
is_windows = _py_builtins.get_current_os_name() == "windows"
178+
is_windows = target_platform_has_any_constraint(ctx, ctx.attr._windows_constraints)
180179

181180
if is_windows:
182181
if not executable.extension == "exe":

tests/base_rules/BUILD.bazel

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,3 @@
1111
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
14-
15-
platform(
16-
name = "mac",
17-
constraint_values = [
18-
"@platforms//os:macos",
19-
],
20-
)
21-
22-
platform(
23-
name = "linux",
24-
constraint_values = [
25-
"@platforms//os:linux",
26-
],
27-
)

tests/base_rules/py_executable_base_tests.bzl

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,52 @@
1313
# limitations under the License.
1414
"""Tests common to py_binary and py_test (executable rules)."""
1515

16+
load("@rules_python_internal//:rules_python_config.bzl", rp_config = "config")
1617
load("@rules_testing//lib:analysis_test.bzl", "analysis_test")
1718
load("@rules_testing//lib:truth.bzl", "matching")
1819
load("@rules_testing//lib:util.bzl", rt_util = "util")
1920
load("//tests/base_rules:base_tests.bzl", "create_base_tests")
2021
load("//tests/base_rules:util.bzl", "WINDOWS_ATTR", pt_util = "util")
22+
load("//tests/support:test_platforms.bzl", "WINDOWS")
2123

2224
_tests = []
2325

26+
def _test_basic_windows(name, config):
27+
if rp_config.enable_pystar:
28+
target_compatible_with = []
29+
else:
30+
target_compatible_with = ["@platforms//:incompatible"]
31+
rt_util.helper_target(
32+
config.rule,
33+
name = name + "_subject",
34+
srcs = ["main.py"],
35+
main = "main.py",
36+
)
37+
analysis_test(
38+
name = name,
39+
impl = _test_basic_windows_impl,
40+
target = name + "_subject",
41+
config_settings = {
42+
"//command_line_option:cpu": "windows_x86_64",
43+
"//command_line_option:crosstool_top": Label("//tests/cc:cc_toolchain_suite"),
44+
"//command_line_option:extra_toolchains": [str(Label("//tests/cc:all"))],
45+
"//command_line_option:platforms": [WINDOWS],
46+
},
47+
attr_values = {"target_compatible_with": target_compatible_with},
48+
)
49+
50+
def _test_basic_windows_impl(env, target):
51+
target = env.expect.that_target(target)
52+
target.executable().path().contains(".exe")
53+
target.runfiles().contains_predicate(matching.str_endswith(
54+
target.meta.format_str("/{name}"),
55+
))
56+
target.runfiles().contains_predicate(matching.str_endswith(
57+
target.meta.format_str("/{name}.exe"),
58+
))
59+
60+
_tests.append(_test_basic_windows)
61+
2462
def _test_executable_in_runfiles(name, config):
2563
rt_util.helper_target(
2664
config.rule,

tests/base_rules/py_test/py_test_tests.bzl

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,12 @@ load(
2121
"create_executable_tests",
2222
)
2323
load("//tests/base_rules:util.bzl", pt_util = "util")
24+
load("//tests/support:test_platforms.bzl", "LINUX", "MAC")
2425

25-
# Explicit Label() calls are required so that it resolves in @rules_python context instead of
26-
# @rules_testing context.
26+
# Explicit Label() calls are required so that it resolves in @rules_python
27+
# context instead of @rules_testing context.
2728
_FAKE_CC_TOOLCHAIN = Label("//tests/cc:cc_toolchain_suite")
2829
_FAKE_CC_TOOLCHAINS = [str(Label("//tests/cc:all"))]
29-
_PLATFORM_MAC = Label("//tests/base_rules:mac")
30-
_PLATFORM_LINUX = Label("//tests/base_rules:linux")
3130

3231
_tests = []
3332

@@ -53,7 +52,7 @@ def _test_mac_requires_darwin_for_execution(name, config):
5352
"//command_line_option:cpu": "darwin_x86_64",
5453
"//command_line_option:crosstool_top": _FAKE_CC_TOOLCHAIN,
5554
"//command_line_option:extra_toolchains": _FAKE_CC_TOOLCHAINS,
56-
"//command_line_option:platforms": [_PLATFORM_MAC],
55+
"//command_line_option:platforms": [MAC],
5756
},
5857
)
5958

@@ -85,7 +84,7 @@ def _test_non_mac_doesnt_require_darwin_for_execution(name, config):
8584
"//command_line_option:cpu": "k8",
8685
"//command_line_option:crosstool_top": _FAKE_CC_TOOLCHAIN,
8786
"//command_line_option:extra_toolchains": _FAKE_CC_TOOLCHAINS,
88-
"//command_line_option:platforms": [_PLATFORM_LINUX],
87+
"//command_line_option:platforms": [LINUX],
8988
},
9089
)
9190

tests/cc/BUILD.bazel

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ cc_toolchain_suite(
5050
toolchains = {
5151
"darwin_x86_64": ":mac_toolchain",
5252
"k8": ":linux_toolchain",
53+
"windows_x86_64": ":windows_toolchain",
5354
},
5455
)
5556

@@ -106,3 +107,29 @@ fake_cc_toolchain_config(
106107
target_cpu = "k8",
107108
toolchain_identifier = "linux-toolchain",
108109
)
110+
111+
cc_toolchain(
112+
name = "windows_toolchain",
113+
all_files = ":empty",
114+
compiler_files = ":empty",
115+
dwp_files = ":empty",
116+
linker_files = ":empty",
117+
objcopy_files = ":empty",
118+
strip_files = ":empty",
119+
supports_param_files = 0,
120+
toolchain_config = ":windows_toolchain_config",
121+
toolchain_identifier = "windows-toolchain",
122+
)
123+
124+
toolchain(
125+
name = "windows_toolchain_definition",
126+
target_compatible_with = ["@platforms//os:windows"],
127+
toolchain = ":windows_toolchain",
128+
toolchain_type = "@bazel_tools//tools/cpp:toolchain_type",
129+
)
130+
131+
fake_cc_toolchain_config(
132+
name = "windows_toolchain_config",
133+
target_cpu = "windows_x86_64",
134+
toolchain_identifier = "windows-toolchain",
135+
)

tests/support/BUILD.bazel

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# Copyright 2023 The Bazel Authors. All rights reserved.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
# ====================
16+
# NOTE: You probably want to use the constants in test_platforms.bzl
17+
# Otherwise, you'll probably have to manually call Label() on these targets
18+
# to force them to resolve in the proper context.
19+
# ====================
20+
platform(
21+
name = "mac",
22+
constraint_values = [
23+
"@platforms//os:macos",
24+
],
25+
)
26+
27+
platform(
28+
name = "linux",
29+
constraint_values = [
30+
"@platforms//os:linux",
31+
],
32+
)
33+
34+
platform(
35+
name = "windows",
36+
constraint_values = [
37+
"@platforms//os:windows",
38+
],
39+
)

tests/support/test_platforms.bzl

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# Copyright 2023 The Bazel Authors. All rights reserved.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
"""Constants for referring to platforms."""
15+
16+
# Explicit Label() calls are required so that it resolves in @rules_python
17+
# context instead of e.g. the @rules_testing context.
18+
MAC = Label("//tests/support:mac")
19+
LINUX = Label("//tests/support:linux")
20+
WINDOWS = Label("//tests/support:windows")

0 commit comments

Comments
 (0)