Skip to content

Commit

Permalink
tests(pystar): add analysis tests to cover basic windows building (ba…
Browse files Browse the repository at this point in the history
…zelbuild#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.
  • Loading branch information
rickeylev authored Oct 4, 2023
1 parent 8d7645e commit 7e07684
Show file tree
Hide file tree
Showing 8 changed files with 139 additions and 25 deletions.
10 changes: 8 additions & 2 deletions python/private/common/py_executable.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ load(
"create_py_info",
"csv",
"filter_to_py_srcs",
"target_platform_has_any_constraint",
"union_attrs",
)
load(
Expand All @@ -48,6 +49,7 @@ load(
"ALLOWED_MAIN_EXTENSIONS",
"BUILD_DATA_SYMLINK_PATH",
"IS_BAZEL",
"PLATFORMS_LOCATION",
"PY_RUNTIME_ATTR_NAME",
"TOOLS_REPO",
)
Expand Down Expand Up @@ -93,6 +95,11 @@ filename in `srcs`, `main` must be specified.
# NOTE: Some tests care about the order of these values.
values = ["PY2", "PY3"],
),
"_windows_constraints": attr.label_list(
default = [
PLATFORMS_LOCATION + "/os:windows",
],
),
},
create_srcs_version_attr(values = SRCS_VERSION_ALL_VALUES),
create_srcs_attr(mandatory = True),
Expand Down Expand Up @@ -201,8 +208,7 @@ def _validate_executable(ctx):
check_native_allowed(ctx)

def _compute_outputs(ctx, output_sources):
# TODO: This should use the configuration instead of the Bazel OS.
if _py_builtins.get_current_os_name() == "windows":
if target_platform_has_any_constraint(ctx, ctx.attr._windows_constraints):
executable = ctx.actions.declare_file(ctx.label.name + ".exe")
else:
executable = ctx.actions.declare_file(ctx.label.name)
Expand Down
5 changes: 2 additions & 3 deletions python/private/common/py_executable_bazel.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ load(
"create_binary_semantics_struct",
"create_cc_details_struct",
"create_executable_result_struct",
"target_platform_has_any_constraint",
"union_attrs",
)
load(":common_bazel.bzl", "collect_cc_info", "get_imports", "maybe_precompile")
Expand Down Expand Up @@ -174,9 +175,7 @@ def _create_executable(
runtime_details = runtime_details,
)

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

if is_windows:
if not executable.extension == "exe":
Expand Down
14 changes: 0 additions & 14 deletions tests/base_rules/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,3 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

platform(
name = "mac",
constraint_values = [
"@platforms//os:macos",
],
)

platform(
name = "linux",
constraint_values = [
"@platforms//os:linux",
],
)
38 changes: 38 additions & 0 deletions tests/base_rules/py_executable_base_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,52 @@
# limitations under the License.
"""Tests common to py_binary and py_test (executable rules)."""

load("@rules_python_internal//:rules_python_config.bzl", rp_config = "config")
load("@rules_testing//lib:analysis_test.bzl", "analysis_test")
load("@rules_testing//lib:truth.bzl", "matching")
load("@rules_testing//lib:util.bzl", rt_util = "util")
load("//tests/base_rules:base_tests.bzl", "create_base_tests")
load("//tests/base_rules:util.bzl", "WINDOWS_ATTR", pt_util = "util")
load("//tests/support:test_platforms.bzl", "WINDOWS")

_tests = []

def _test_basic_windows(name, config):
if rp_config.enable_pystar:
target_compatible_with = []
else:
target_compatible_with = ["@platforms//:incompatible"]
rt_util.helper_target(
config.rule,
name = name + "_subject",
srcs = ["main.py"],
main = "main.py",
)
analysis_test(
name = name,
impl = _test_basic_windows_impl,
target = name + "_subject",
config_settings = {
"//command_line_option:cpu": "windows_x86_64",
"//command_line_option:crosstool_top": Label("//tests/cc:cc_toolchain_suite"),
"//command_line_option:extra_toolchains": [str(Label("//tests/cc:all"))],
"//command_line_option:platforms": [WINDOWS],
},
attr_values = {"target_compatible_with": target_compatible_with},
)

def _test_basic_windows_impl(env, target):
target = env.expect.that_target(target)
target.executable().path().contains(".exe")
target.runfiles().contains_predicate(matching.str_endswith(
target.meta.format_str("/{name}"),
))
target.runfiles().contains_predicate(matching.str_endswith(
target.meta.format_str("/{name}.exe"),
))

_tests.append(_test_basic_windows)

def _test_executable_in_runfiles(name, config):
rt_util.helper_target(
config.rule,
Expand Down
11 changes: 5 additions & 6 deletions tests/base_rules/py_test/py_test_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,12 @@ load(
"create_executable_tests",
)
load("//tests/base_rules:util.bzl", pt_util = "util")
load("//tests/support:test_platforms.bzl", "LINUX", "MAC")

# Explicit Label() calls are required so that it resolves in @rules_python context instead of
# @rules_testing context.
# Explicit Label() calls are required so that it resolves in @rules_python
# context instead of @rules_testing context.
_FAKE_CC_TOOLCHAIN = Label("//tests/cc:cc_toolchain_suite")
_FAKE_CC_TOOLCHAINS = [str(Label("//tests/cc:all"))]
_PLATFORM_MAC = Label("//tests/base_rules:mac")
_PLATFORM_LINUX = Label("//tests/base_rules:linux")

_tests = []

Expand All @@ -53,7 +52,7 @@ def _test_mac_requires_darwin_for_execution(name, config):
"//command_line_option:cpu": "darwin_x86_64",
"//command_line_option:crosstool_top": _FAKE_CC_TOOLCHAIN,
"//command_line_option:extra_toolchains": _FAKE_CC_TOOLCHAINS,
"//command_line_option:platforms": [_PLATFORM_MAC],
"//command_line_option:platforms": [MAC],
},
)

Expand Down Expand Up @@ -85,7 +84,7 @@ def _test_non_mac_doesnt_require_darwin_for_execution(name, config):
"//command_line_option:cpu": "k8",
"//command_line_option:crosstool_top": _FAKE_CC_TOOLCHAIN,
"//command_line_option:extra_toolchains": _FAKE_CC_TOOLCHAINS,
"//command_line_option:platforms": [_PLATFORM_LINUX],
"//command_line_option:platforms": [LINUX],
},
)

Expand Down
27 changes: 27 additions & 0 deletions tests/cc/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ cc_toolchain_suite(
toolchains = {
"darwin_x86_64": ":mac_toolchain",
"k8": ":linux_toolchain",
"windows_x86_64": ":windows_toolchain",
},
)

Expand Down Expand Up @@ -106,3 +107,29 @@ fake_cc_toolchain_config(
target_cpu = "k8",
toolchain_identifier = "linux-toolchain",
)

cc_toolchain(
name = "windows_toolchain",
all_files = ":empty",
compiler_files = ":empty",
dwp_files = ":empty",
linker_files = ":empty",
objcopy_files = ":empty",
strip_files = ":empty",
supports_param_files = 0,
toolchain_config = ":windows_toolchain_config",
toolchain_identifier = "windows-toolchain",
)

toolchain(
name = "windows_toolchain_definition",
target_compatible_with = ["@platforms//os:windows"],
toolchain = ":windows_toolchain",
toolchain_type = "@bazel_tools//tools/cpp:toolchain_type",
)

fake_cc_toolchain_config(
name = "windows_toolchain_config",
target_cpu = "windows_x86_64",
toolchain_identifier = "windows-toolchain",
)
39 changes: 39 additions & 0 deletions tests/support/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Copyright 2023 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

# ====================
# NOTE: You probably want to use the constants in test_platforms.bzl
# Otherwise, you'll probably have to manually call Label() on these targets
# to force them to resolve in the proper context.
# ====================
platform(
name = "mac",
constraint_values = [
"@platforms//os:macos",
],
)

platform(
name = "linux",
constraint_values = [
"@platforms//os:linux",
],
)

platform(
name = "windows",
constraint_values = [
"@platforms//os:windows",
],
)
20 changes: 20 additions & 0 deletions tests/support/test_platforms.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Copyright 2023 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""Constants for referring to platforms."""

# Explicit Label() calls are required so that it resolves in @rules_python
# context instead of e.g. the @rules_testing context.
MAC = Label("//tests/support:mac")
LINUX = Label("//tests/support:linux")
WINDOWS = Label("//tests/support:windows")

0 comments on commit 7e07684

Please sign in to comment.