From c3c23bdcdd1b1c658aaec595193fd8f5a41e8ec1 Mon Sep 17 00:00:00 2001 From: "F.Tibor" Date: Mon, 31 Aug 2026 12:07:19 +0200 Subject: [PATCH] test: integrate pylint into bazel --- .ci/micromamba/ci.yaml | 1 - .ci/micromamba/env.yaml | 1 - .github/workflows/test.yaml | 4 +- .pylintrc | 11 -- MODULE.bazel | 22 ++++ docs/contributing.md | 20 ++- src/BUILD | 10 ++ src/__init__.py | 0 test/foss/BUILD | 14 ++ test/pylint/.pylintrc | 8 ++ test/{foss/__init__.py => pylint/BUILD} | 25 +++- test/pylint/pylint_runner.py | 79 +++++++++++ test/pylint/pylint_test.bzl | 167 ++++++++++++++++++++++++ test/pylint/requirements.txt | 14 ++ test/unit/BUILD | 15 +++ test/unit/caching/BUILD | 14 ++ test/unit/plist_res/BUILD | 6 + 17 files changed, 392 insertions(+), 19 deletions(-) delete mode 100644 .pylintrc delete mode 100644 src/__init__.py create mode 100644 test/pylint/.pylintrc rename test/{foss/__init__.py => pylint/BUILD} (52%) create mode 100644 test/pylint/pylint_runner.py create mode 100644 test/pylint/pylint_test.bzl create mode 100644 test/pylint/requirements.txt diff --git a/.ci/micromamba/ci.yaml b/.ci/micromamba/ci.yaml index 5a9040d9..175fa6c8 100644 --- a/.ci/micromamba/ci.yaml +++ b/.ci/micromamba/ci.yaml @@ -8,7 +8,6 @@ channels: dependencies: - python=3.11 - pytest - - pylint - pycodestyle - buildifier - clang=21 diff --git a/.ci/micromamba/env.yaml b/.ci/micromamba/env.yaml index de8e2f88..8fa08226 100644 --- a/.ci/micromamba/env.yaml +++ b/.ci/micromamba/env.yaml @@ -8,7 +8,6 @@ channels: dependencies: - python=3.11 - pytest - - pylint - pycodestyle - buildifier - clang=21 diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index f6a9e38d..c88c321b 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -80,8 +80,8 @@ jobs: - name: Run lint tests run: | - pylint . bazel test //test/buildifier --test_output=all + bazel test //... --test_tag_filters=pylint --test_output=all shell: bash -el {0} # Splitting test runs to: bazel unit tests, bazel foss tests, python unittests @@ -155,8 +155,8 @@ jobs: - name: Run lint tests run: | - pylint . runuser -u test -- bazel test //test/buildifier --test_output=all + runuser -u test -- bazel test //... --test_tag_filters=pylint --test_output=all shell: bash -el {0} # Splitting test runs to: bazel unit tests, bazel foss tests, python unittests diff --git a/.pylintrc b/.pylintrc deleted file mode 100644 index a8a353ac..00000000 --- a/.pylintrc +++ /dev/null @@ -1,11 +0,0 @@ -[MASTER] -# To find common lib for tests -init-hook='import sys; import os; sys.path.append("test"); sys.path.append(os.getcwd())' -ignore = test_template.py - -[FORMAT] -max-line-length=80 - -[MESSAGES CONTROL] -disable= - fixme, diff --git a/MODULE.bazel b/MODULE.bazel index 7cf38295..1e56d93c 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -44,6 +44,28 @@ codechecker_extension = use_extension( ) use_repo(codechecker_extension, "default_codechecker_tools") +python = use_extension( + "@rules_python//python/extensions:python.bzl", + "python", + dev_dependency = True, +) +python.toolchain( + is_default = True, + python_version = "3.12", +) + +pip = use_extension( + "@rules_python//python/extensions:pip.bzl", + "pip", + dev_dependency = True, +) +pip.parse( + hub_name = "pylint_deps", + python_version = "3.12", + requirements_lock = "//test/pylint:requirements.txt", +) +use_repo(pip, "pylint_deps") + register_toolchains( "//:default_toolchain", ) diff --git a/docs/contributing.md b/docs/contributing.md index 9ef927b4..66712dd2 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -97,9 +97,23 @@ Or let one script do the whole round: On how to run or add a new test, see [test/README.md](https://github.com/Ericsson/rules_codechecker/blob/main/test/README.md). -Note that `test/unit/external_repository` is in `.bazelignore`, it consumes -these rules as an external module and therefore runs through `pytest` only. -It is the test that catches breakage visible to our users but not to us. +### Linting Python with Bazel + +Python targets are linted through a per-target Bazel macro. +Add a `pylint(...)` target next to a top-level +`py_binary`/`py_library`/`py_test`: + +```starlark +load("//test/pylint:pylint_test.bzl", "pylint") + +pylint( + name = "pylint", + targets = [":my_py_binary"], +) +``` + +It lints the targets and their dependencies (except pip packages) +These targets run as part of `bazel test //...` (tagged `pylint`). ## Submitting a patch diff --git a/src/BUILD b/src/BUILD index a43a5b3f..bb02a83c 100644 --- a/src/BUILD +++ b/src/BUILD @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. load("@rules_python//python:py_binary.bzl", "py_binary") +load("//test/pylint:pylint_test.bzl", "pylint") # Tool filter compile_commands.json file py_binary( @@ -32,6 +33,15 @@ py_binary( visibility = ["//visibility:public"], ) +pylint( + name = "pylint", + targets = [ + ":codechecker_script", + ":compile_commands_filter", + ":per_file_script", + ], +) + # The following are flags and default values for clang_tidy_aspect filegroup( name = "clang_tidy_config_default", diff --git a/src/__init__.py b/src/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/test/foss/BUILD b/test/foss/BUILD index 07ccc216..b6a171aa 100644 --- a/test/foss/BUILD +++ b/test/foss/BUILD @@ -25,8 +25,22 @@ # To add a new project: add a foss_test() call below. # ----------------------------------------------------------------------- +load("@rules_python//python:py_library.bzl", "py_library") +load("//test/pylint:pylint_test.bzl", "pylint") load(":foss_test.bzl", "foss_test") +# Library wrapping the foss_test runner so it can be linted. +py_library( + name = "foss_test_runner_lib", + srcs = ["foss_test_runner.py"], + tags = ["manual"], +) + +pylint( + name = "pylint", + targets = [":foss_test_runner_lib"], +) + foss_test( name = "zlib", tests = [ diff --git a/test/pylint/.pylintrc b/test/pylint/.pylintrc new file mode 100644 index 00000000..da5cfd97 --- /dev/null +++ b/test/pylint/.pylintrc @@ -0,0 +1,8 @@ +[MASTER] + +[FORMAT] +max-line-length=80 + +[MESSAGES CONTROL] +disable= + fixme, diff --git a/test/foss/__init__.py b/test/pylint/BUILD similarity index 52% rename from test/foss/__init__.py rename to test/pylint/BUILD index 78bab5f1..ae4dd6b0 100644 --- a/test/foss/__init__.py +++ b/test/pylint/BUILD @@ -1,4 +1,4 @@ -# Copyright 2023 Ericsson AB +# Copyright 2026 Ericsson AB # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -11,3 +11,26 @@ # 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. + +load("@rules_python//python:py_binary.bzl", "py_binary") +load("//test/pylint:pylint_test.bzl", "pylint") + +exports_files([ + ".pylintrc", + "pylint_runner.py", + "requirements.txt", +]) + +py_binary( + name = "pylint_runner", + srcs = ["pylint_runner.py"], + main = "pylint_runner.py", + visibility = ["//visibility:public"], + deps = ["@pylint_deps//pylint"], +) + +# Lint the pylint runner itself. +pylint( + name = "pylint", + targets = [":pylint_runner"], +) diff --git a/test/pylint/pylint_runner.py b/test/pylint/pylint_runner.py new file mode 100644 index 00000000..317e3f94 --- /dev/null +++ b/test/pylint/pylint_runner.py @@ -0,0 +1,79 @@ +# Copyright 2026 Ericsson AB +# +# 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. + +"""Runner script for the per-target pylint Bazel test. + +Reads a manifest of .py files (produced by the pylint rule) and runs +pylint on exactly those files. Imports are resolved from this runner's +own sys.path, which rules_python populates from the linted target's +runfiles; no repository-root chdir is needed. + +Pylint is imported as a Python dependency provided by Bazel, +not invoked from PATH. +""" + +import argparse +import os +import sys + +from pylint import lint + + +def read_manifest(manifest): + """Read the list of files to lint from the manifest.""" + files = [] + with open(manifest, encoding="utf-8") as handle: + for line in handle: + entry = line.strip() + if entry: + files.append(entry) + return files + + +def main(): + """Parse arguments and run pylint on the manifest sources.""" + parser = argparse.ArgumentParser( + description="Run pylint on Python source files." + ) + parser.add_argument( + "--manifest", + required=True, + help="Path to a manifest file listing the .py files to lint.", + ) + parser.add_argument( + "--rcfile", + default=None, + help="Path to a .pylintrc configuration file.", + ) + args = parser.parse_args() + + rcfile_path = os.path.realpath(args.rcfile) if args.rcfile else None + sources = [os.path.realpath(s) for s in read_manifest(args.manifest)] + + if not sources: + print("pylint_runner: no .py files found") + sys.exit(1) + + pylint_args = [] + if rcfile_path: + pylint_args.append(f"--rcfile={rcfile_path}") + pylint_args.extend(sources) + + print(f"Running: pylint {' '.join(pylint_args)}") + result = lint.Run(pylint_args, exit=False) + sys.exit(result.linter.msg_status) + + +if __name__ == "__main__": + main() diff --git a/test/pylint/pylint_test.bzl b/test/pylint/pylint_test.bzl new file mode 100644 index 00000000..b61234e1 --- /dev/null +++ b/test/pylint/pylint_test.bzl @@ -0,0 +1,167 @@ +# Copyright 2026 Ericsson AB +# +# 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. + +""" +Per-target macro for running pylint on Python targets. + +Lints the Python sources a target provides via `PyInfo.transitive_sources`. + +Usage: + load("//test/pylint:pylint_test.bzl", "pylint") + + pylint( + name = "pylint", + targets = [":my_py_binary"], + ) +""" + +def _filter_py_sources(target): + """Collect python files, written by us, provided by a target. + + Reads `PyInfo.transitive_sources` and drops files that live in an + external repository (e.g. pip deps), keeping + only files rooted in the main workspace. + + Args: + target: A target that may carry PyInfo. + + Returns: + A list of first-party python File objects (possibly empty). + """ + if PyInfo not in target: + return [] + + sources = [] + for src in target[PyInfo].transitive_sources.to_list(): + if src.extension != "py": + continue + + # Files from external repositories (e.g. pip) + # have a non-empty owner or workspace root/name. + owner = src.owner + if owner.workspace_root or owner.workspace_name: + continue + sources.append(src) + return sources + +def _collect_sources(targets): + """Union python sources across the given targets. + + Args: + targets: List of targets to collect sources from. + + Returns: + A depset of python File objects. + """ + sources = [] + for target in targets: + sources.extend(_filter_py_sources(target)) + + return depset(sources) + +def _pylint_impl(ctx): + sources = _collect_sources(ctx.attr.targets) + source_list = sources.to_list() + + # Write the manifest of files to lint. + manifest = ctx.actions.declare_file(ctx.label.name + "_manifest.txt") + ctx.actions.write( + output = manifest, + content = "\n".join([src.short_path for src in source_list]) + "\n", + ) + + executable = ctx.actions.declare_file(ctx.label.name) + runner = ctx.executable._runner + ctx.actions.write( + output = executable, + is_executable = True, + content = """#!/usr/bin/env bash +set -euo pipefail +exec "{runner}" \ + --rcfile "{rcfile}" \ + --manifest "{manifest}" "$@" +""".format( + runner = runner.short_path, + rcfile = ctx.file.pylintrc.short_path, + manifest = manifest.short_path, + ), + ) + + runfiles = ctx.runfiles( + files = source_list + [ + manifest, + ctx.file.pylintrc, + ], + ) + runfiles = runfiles.merge( + ctx.attr._runner[DefaultInfo].default_runfiles, + ) + + return [ + DefaultInfo( + executable = executable, + runfiles = runfiles, + ), + ] + +_pylint_test = rule( + implementation = _pylint_impl, + test = True, + attrs = { + "pylintrc": attr.label( + allow_single_file = True, + mandatory = True, + doc = "Label of the .pylintrc configuration file.", + ), + "targets": attr.label_list( + mandatory = True, + doc = "Python targets whose sources should be linted.", + ), + "_runner": attr.label( + default = Label("//test/pylint:pylint_runner"), + executable = True, + cfg = "target", + ), + }, +) + +def pylint( + name, + targets, + pylintrc = "//test/pylint:.pylintrc", + tags = [], + **kwargs): + """Run pylint on the first-party sources of the given Python targets. + + Sources are collected transitively from ``PyInfo``, so a top-level + target also lints the first-party libraries it imports. + + Args: + name: Test name. + targets: Python targets (py_binary/py_library/py_test) to lint. + pylintrc: Label of the .pylintrc configuration file. + tags: Additional test tags. + **kwargs: Forwarded to the underlying rule. + """ + pylint_tags = [] + tags + if "pylint" not in pylint_tags: + pylint_tags.append("pylint") + + _pylint_test( + name = name, + targets = targets, + pylintrc = pylintrc, + tags = pylint_tags, + **kwargs + ) diff --git a/test/pylint/requirements.txt b/test/pylint/requirements.txt new file mode 100644 index 00000000..937400fc --- /dev/null +++ b/test/pylint/requirements.txt @@ -0,0 +1,14 @@ +astroid==4.0.4 + # via pylint +dill==0.4.1 + # via pylint +isort==8.0.1 + # via pylint +mccabe==0.7.0 + # via pylint +platformdirs==4.11.5 + # via pylint +pylint==4.0.5 + # via -r - +tomlkit==0.15.1 + # via pylint diff --git a/test/unit/BUILD b/test/unit/BUILD index 35047442..3899bc1b 100644 --- a/test/unit/BUILD +++ b/test/unit/BUILD @@ -12,4 +12,19 @@ # See the License for the specific language governing permissions and # limitations under the License. +load("@rules_python//python:py_library.bzl", "py_library") +load("//test/pylint:pylint_test.bzl", "pylint") + exports_files(["grep_check.py"]) + +# Library wrapping the unit_test runner so it can be linted. +py_library( + name = "grep_check_lib", + srcs = ["grep_check.py"], + tags = ["manual"], +) + +pylint( + name = "pylint", + targets = [":grep_check_lib"], +) diff --git a/test/unit/caching/BUILD b/test/unit/caching/BUILD index c173b56b..7d2dfec4 100644 --- a/test/unit/caching/BUILD +++ b/test/unit/caching/BUILD @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +load("@rules_python//python:py_library.bzl", "py_library") +load("//test/pylint:pylint_test.bzl", "pylint") load( "//test/unit/caching:caching_test.bzl", "caching_test", @@ -23,6 +25,18 @@ load( exports_files(["caching_check.py"]) +# Library wrapping the caching test runner so it can be linted. +py_library( + name = "caching_check_lib", + srcs = ["caching_check.py"], + tags = ["manual"], +) + +pylint( + name = "pylint", + targets = [":caching_check_lib"], +) + # Bazel-native caching tests (bazel inside bazel) caching_test( name = "caching_monolithic_test", diff --git a/test/unit/plist_res/BUILD b/test/unit/plist_res/BUILD index 8ed9ab09..df4c3537 100644 --- a/test/unit/plist_res/BUILD +++ b/test/unit/plist_res/BUILD @@ -13,6 +13,7 @@ # limitations under the License. load("@rules_python//python:py_test.bzl", "py_test") +load("//test/pylint:pylint_test.bzl", "pylint") py_test( name = "path_resolution_test", @@ -22,3 +23,8 @@ py_test( tags = ["unit"], deps = ["//src:codechecker_script"], ) + +pylint( + name = "pylint", + targets = [":path_resolution_test"], +)