From 95ad08cf20f720bb1d6c5ec15f4b9f5ef26ee931 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Thu, 3 Oct 2024 09:16:33 -0700 Subject: [PATCH 1/5] feat: port pytest_test macro Comes from https://github.com/caseyduquettesc/rules_python_pytest/pull/13 It's much easier to properly gazelle-generate a BUILD file in this shape rather than the awkward way you're forced to hold our py_pytest_main. --- examples/pytest/BUILD.bazel | 14 ++------ py/defs.bzl | 20 ++++++----- pytest/BUILD.bazel | 4 +++ pytest/defs.bzl | 65 +++++++++++++++++++++++++++++++++++ pytest/pytest_shim.py | 68 +++++++++++++++++++++++++++++++++++++ 5 files changed, 152 insertions(+), 19 deletions(-) create mode 100644 pytest/BUILD.bazel create mode 100644 pytest/defs.bzl create mode 100644 pytest/pytest_shim.py diff --git a/examples/pytest/BUILD.bazel b/examples/pytest/BUILD.bazel index 24c0d607..103e9ca5 100644 --- a/examples/pytest/BUILD.bazel +++ b/examples/pytest/BUILD.bazel @@ -1,23 +1,15 @@ -load("@aspect_rules_py//py:defs.bzl", "py_pytest_main", "py_test") +load("@aspect_rules_py//pytest:defs.bzl", "pytest_test") -py_pytest_main( - name = "__test__", - deps = ["@pypi_pytest//:pkg"], -) - -py_test( +pytest_test( name = "pytest_test", srcs = [ "foo_test.py", - ":__test__", ], imports = ["../.."], - main = ":__test__.py", package_collisions = "warning", + pip_repo = "pypi", deps = [ - ":__test__", "@pypi_ftfy//:pkg", "@pypi_neptune//:pkg", - "@pypi_pytest//:pkg", ], ) diff --git a/py/defs.bzl b/py/defs.bzl index 6fabf75a..fc077266 100644 --- a/py/defs.bzl +++ b/py/defs.bzl @@ -22,22 +22,26 @@ py_unpacked_wheel = _py_unpacked_wheel resolutions = _resolutions def _py_binary_or_test(name, rule, srcs, main, deps = [], resolutions = {}, **kwargs): + if type(main) not in ["string", "Label"]: + fail("main must be a Label or a string") + # Compatibility with rules_python, see docs in py_executable.bzl main_target = "_{}.find_main".format(name) - determine_main( - name = main_target, - target_name = name, - main = main, - srcs = srcs, - **propagate_common_rule_attributes(kwargs) - ) + if type(main) == "string": + determine_main( + name = main_target, + target_name = name, + main = main, + srcs = srcs, + **propagate_common_rule_attributes(kwargs) + ) package_collisions = kwargs.pop("package_collisions", None) rule( name = name, srcs = srcs, - main = main_target, + main = main if type(main) == "Label" else main_target, deps = deps, resolutions = resolutions, package_collisions = package_collisions, diff --git a/pytest/BUILD.bazel b/pytest/BUILD.bazel new file mode 100644 index 00000000..d54da6ca --- /dev/null +++ b/pytest/BUILD.bazel @@ -0,0 +1,4 @@ +exports_files( + ["pytest_shim.py"], + visibility = ["//visibility:public"], +) diff --git a/pytest/defs.bzl b/pytest/defs.bzl new file mode 100644 index 00000000..81b60944 --- /dev/null +++ b/pytest/defs.bzl @@ -0,0 +1,65 @@ +"""Use pytest to run tests, using a wrapper script to interface with Bazel. + +Example: + +```starlark +load("@aspect_rules_py//pytest:defs.bzl", "pytest_test") + +pytest_test( + name = "test_w_pytest", + size = "small", + srcs = ["test.py"], +) +``` + +By default, `@pip//pytest` is added to `deps`. +If sharding is used (when `shard_count > 1`) then `@pip//pytest_shard` is also added. +To instead provide explicit deps for the pytest library, set `pytest_deps`: + +```starlark +pytest_test( + name = "test_w_my_pytest", + shard_count = 2, + srcs = ["test.py"], + pytest_deps = [requirement("pytest"), requirement("pytest-shard"), ...], +) +``` +""" + +load("//py:defs.bzl", "py_test") + +def pytest_test(name, srcs, deps = [], args = [], pytest_deps = None, pip_repo = "pip", **kwargs): + """ + Wrapper macro for `py_test` which supports pytest. + + Args: + name: A unique name for this target. + srcs: Python source files. + deps: Dependencies, typically `py_library`. + args: Additional command-line arguments to pytest. + See https://docs.pytest.org/en/latest/how-to/usage.html + pytest_deps: Labels of the pytest tool and other packages it may import. + pip_repo: Name of the external repository where Python packages are installed. + It's typically created by `pip.parse`. + This attribute is used only when `pytest_deps` is unset. + **kwargs: Additional named parameters to py_test. + """ + shim_label = Label("//pytest:pytest_shim.py") + + if pytest_deps == None: + pytest_deps = ["@{}//pytest".format(pip_repo)] + if kwargs.get("shard_count", 1) > 1: + pytest_deps.append("@{}//pytest_shard".format(pip_repo)) + + py_test( + name = name, + srcs = [ + shim_label, + ] + srcs, + main = shim_label, + args = [ + "--capture=no", + ] + args + ["$(location :%s)" % x for x in srcs], + deps = deps + pytest_deps, + **kwargs + ) diff --git a/pytest/pytest_shim.py b/pytest/pytest_shim.py new file mode 100644 index 00000000..3fa95e3e --- /dev/null +++ b/pytest/pytest_shim.py @@ -0,0 +1,68 @@ +"""A shim for executing pytest that supports test filtering, sharding, and more.""" + +import sys +import os + +import pytest + + +if __name__ == "__main__": + pytest_args = ["--ignore=external"] + + args = sys.argv[1:] + # pytest < 8.0 runs tests twice if __init__.py is passed explicitly as an argument. + # Remove any __init__.py file to avoid that. + # pytest.version_tuple is available since pytest 7.0 + # https://github.com/pytest-dev/pytest/issues/9313 + if not hasattr(pytest, "version_tuple") or pytest.version_tuple < (8, 0): + args = [arg for arg in args if arg.startswith("-") or os.path.basename(arg) != "__init__.py"] + + if os.environ.get("XML_OUTPUT_FILE"): + pytest_args.append("--junitxml={xml_output_file}".format(xml_output_file=os.environ.get("XML_OUTPUT_FILE"))) + + # Handle test sharding - requires pytest-shard plugin. + if os.environ.get("TEST_SHARD_INDEX") and os.environ.get("TEST_TOTAL_SHARDS"): + pytest_args.append("--shard-id={shard_id}".format(shard_id=os.environ.get("TEST_SHARD_INDEX"))) + pytest_args.append("--num-shards={num_shards}".format(num_shards=os.environ.get("TEST_TOTAL_SHARDS"))) + if os.environ.get("TEST_SHARD_STATUS_FILE"): + open(os.environ["TEST_SHARD_STATUS_FILE"], "a").close() + + # Handle plugins that generate reports - if they are provided with relative paths (via args), + # re-write it under bazel's test undeclared outputs dir. + if os.environ.get("TEST_UNDECLARED_OUTPUTS_DIR"): + undeclared_output_dir = os.environ.get("TEST_UNDECLARED_OUTPUTS_DIR") + + # Flags that take file paths as value. + path_flags = [ + "--report-log", # pytest-reportlog + "--json-report-file", # pytest-json-report + "--html", # pytest-html + ] + for i, arg in enumerate(args): + for flag in path_flags: + if arg.startswith(f"{flag}="): + arg_split = arg.split("=", 1) + if len(arg_split) == 2 and not os.path.isabs(arg_split[1]): + args[i] = f"{flag}={undeclared_output_dir}/{arg_split[1]}" + + if os.environ.get("TESTBRIDGE_TEST_ONLY"): + test_filter = os.environ["TESTBRIDGE_TEST_ONLY"] + + # If the test filter does not start with a class-like name, then use test filtering instead + if not test_filter[0].isupper(): + # --test_filter=test_module.test_fn or --test_filter=test_module/test_file.py + pytest_args.extend(args) + pytest_args.append("-k={filter}".format(filter=test_filter)) + else: + # --test_filter=TestClass.test_fn + for arg in args: + if not arg.startswith("--"): + # arg is a src file. Add test class/method selection to it. + # test.py::TestClass::test_fn + arg = "{arg}::{module_fn}".format(arg=arg, module_fn=test_filter.replace(".", "::")) + pytest_args.append(arg) + else: + pytest_args.extend(args) + + print(pytest_args, file=sys.stderr) + raise SystemExit(pytest.main(pytest_args)) From c44466f506a546a1ed8e5ec49d46618a5de712d3 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Thu, 3 Oct 2024 09:17:58 -0700 Subject: [PATCH 2/5] chore: improve error msg --- py/defs.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/py/defs.bzl b/py/defs.bzl index fc077266..89508537 100644 --- a/py/defs.bzl +++ b/py/defs.bzl @@ -23,7 +23,7 @@ resolutions = _resolutions def _py_binary_or_test(name, rule, srcs, main, deps = [], resolutions = {}, **kwargs): if type(main) not in ["string", "Label"]: - fail("main must be a Label or a string") + fail("main must be a Label or a string, not {}".format(type(main))) # Compatibility with rules_python, see docs in py_executable.bzl main_target = "_{}.find_main".format(name) From b6c9cb8d41f80d6d8e2286afb2ff49ec66541a78 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Thu, 3 Oct 2024 09:20:14 -0700 Subject: [PATCH 3/5] chore: give credit --- pytest/pytest_shim.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pytest/pytest_shim.py b/pytest/pytest_shim.py index 3fa95e3e..28d3feb2 100644 --- a/pytest/pytest_shim.py +++ b/pytest/pytest_shim.py @@ -1,4 +1,7 @@ -"""A shim for executing pytest that supports test filtering, sharding, and more.""" +"""A shim for executing pytest that supports test filtering, sharding, and more. + +Copied from https://github.com/caseyduquettesc/rules_python_pytest/blob/331e0e511130cf4859b7589a479db6c553974abf/python_pytest/pytest_shim.py +""" import sys import os From 5f9a6b475aff670f54aef44c304d282cfdea4689 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Thu, 3 Oct 2024 09:26:16 -0700 Subject: [PATCH 4/5] fix: allow main is None --- py/defs.bzl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/py/defs.bzl b/py/defs.bzl index 89508537..083329d5 100644 --- a/py/defs.bzl +++ b/py/defs.bzl @@ -22,12 +22,12 @@ py_unpacked_wheel = _py_unpacked_wheel resolutions = _resolutions def _py_binary_or_test(name, rule, srcs, main, deps = [], resolutions = {}, **kwargs): - if type(main) not in ["string", "Label"]: + if main and type(main) not in ["string", "Label"]: fail("main must be a Label or a string, not {}".format(type(main))) # Compatibility with rules_python, see docs in py_executable.bzl main_target = "_{}.find_main".format(name) - if type(main) == "string": + if type(main) != "Label": determine_main( name = main_target, target_name = name, From 7577655ea06fb1d9464448e25ceafa62395955e5 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Thu, 3 Oct 2024 10:03:29 -0700 Subject: [PATCH 5/5] refactor: rename to py_ prefix matching casey's --- examples/pytest/BUILD.bazel | 4 ++-- pytest/defs.bzl | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/examples/pytest/BUILD.bazel b/examples/pytest/BUILD.bazel index 103e9ca5..781b378f 100644 --- a/examples/pytest/BUILD.bazel +++ b/examples/pytest/BUILD.bazel @@ -1,6 +1,6 @@ -load("@aspect_rules_py//pytest:defs.bzl", "pytest_test") +load("@aspect_rules_py//pytest:defs.bzl", "py_pytest_test") -pytest_test( +py_pytest_test( name = "pytest_test", srcs = [ "foo_test.py", diff --git a/pytest/defs.bzl b/pytest/defs.bzl index 81b60944..bf6a2e4f 100644 --- a/pytest/defs.bzl +++ b/pytest/defs.bzl @@ -3,9 +3,9 @@ Example: ```starlark -load("@aspect_rules_py//pytest:defs.bzl", "pytest_test") +load("@aspect_rules_py//pytest:defs.bzl", "py_pytest_test") -pytest_test( +py_pytest_test( name = "test_w_pytest", size = "small", srcs = ["test.py"], @@ -17,7 +17,7 @@ If sharding is used (when `shard_count > 1`) then `@pip//pytest_shard` is also a To instead provide explicit deps for the pytest library, set `pytest_deps`: ```starlark -pytest_test( +py_pytest_test( name = "test_w_my_pytest", shard_count = 2, srcs = ["test.py"], @@ -28,7 +28,7 @@ pytest_test( load("//py:defs.bzl", "py_test") -def pytest_test(name, srcs, deps = [], args = [], pytest_deps = None, pip_repo = "pip", **kwargs): +def py_pytest_test(name, srcs, deps = [], args = [], pytest_deps = None, pip_repo = "pip", **kwargs): """ Wrapper macro for `py_test` which supports pytest.