-
Notifications
You must be signed in to change notification settings - Fork 366
Pre commit hook #6231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Pre commit hook #6231
Conversation
|
You are changing one of the avocado utils which has already been migrated to AAutils project https://github.com/avocado-framework/aautils and this utility will be removed after the LTS release. Please make sure that all your proposed changes are already in AAutils and this PR is only backport. For more information about AAutlis migration see https://avocado-framework.readthedocs.io/en/latest/blueprints/BP005.html For a list of migrated utilities see https://avocado-framework.github.io/aautils.html |
WalkthroughAdds pre-commit and commitlint configs and updates the static-checks submodule; removes legacy checks/targets (codespell target in Makefile.gh and CI, selftests signedoff/spell scripts) and adds a pre-commit selftest shim. Wide-spread formatting/whitespace/quote normalization and typo fixes across docs, examples, and JS/CSS. Simplifies generators via Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 YAMLlint (1.38.0).pre-commit-config.yaml[Errno 2] No such file or directory: '.pre-commit-config.yaml' Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
avocado/utils/asset.py (1)
621-625: Critical: Error message constructed as tuple, not string; typo in message.Same issue as above—
msgis a tuple instead of a concatenated string. Additionally, "Currented" should be "Currently".Apply this diff to concatenate the strings and fix the typo:
except KeyError as exc: msg = ( - "Operator not supported. Currented valid values are: ", - ", ".join(SUPPORTED_OPERATORS), - ) + "Operator not supported. Currently valid values are: " + + ", ".join(SUPPORTED_OPERATORS) + )selftests/functional/nrunner.py (2)
108-120: Assertion will fail whenstdout_statusisNone.When
len(lines) == 1,stdout_statusis set toNoneon line 110, but line 118 unconditionally asserts on it. TheassertInmethod will raiseTypeError: argument of type 'NoneType' is not iterablewhen checking if a string is inNone.Apply this diff to fix the logic:
lines = res.stdout_text.splitlines() if len(lines) == 1: first_status = final_status = lines[0] - stdout_status = None else: first_status = lines[0] stdout_status = lines[-3] final_status = lines[-1] self.assertIn("'status': 'started'", first_status) self.assertIn("'time': ", first_status) + self.assertIn("'log': b'Hello world!\\n'", stdout_status) self.assertIn("'status': 'finished'", final_status) - self.assertIn("'log': b'Hello world!\\n'", stdout_status) self.assertIn("'time': ", final_status) self.assertEqual(res.exit_status, 0)
231-243: Assertion will fail whenstdout_statusisNone.When
len(lines) == 1,stdout_statusis set toNoneon line 233, but line 242 unconditionally asserts on it. TheassertInmethod will raiseTypeError: argument of type 'NoneType' is not iterablewhen checking if a string is inNone.Apply this diff to fix the logic:
lines = res.stdout_text.splitlines() if len(lines) == 1: first_status = final_status = lines[0] - stdout_status = None else: first_status = lines[0] stdout_status = lines[-3] final_status = lines[-1] self.assertIn("'status': 'started'", first_status) self.assertIn("'id': 2", first_status) + self.assertIn("'log': b'avocado'", stdout_status) self.assertIn("'id': 2", first_status) self.assertIn("'status': 'finished'", final_status) - self.assertIn("'log': b'avocado'", stdout_status) self.assertEqual(res.exit_status, 0)
🧹 Nitpick comments (7)
contrib/scripts/avocado-safeloader-find-avocado-instrumented (1)
33-36: Consider using_for unused exception variable.The error handling correctly prevents crashes when files can't be read. However, since the
errorvariable is captured but never used, consider replacing it with_to indicate it's intentionally unused.Apply this diff:
- except IOError as error: + except IOError: continueAlternatively, if you'd like to provide user feedback for debugging purposes, you could log skipped files to stderr:
except IOError as error: print(f"Skipping {test_module_path}: {error}", file=sys.stderr) continueavocado/utils/output.py (1)
106-107: LGTM! Clean refactor to idiomatic Python.Replacing conditional branches with
max()/min()for value clamping is more concise and idiomatic. The logic correctly ensures the amount stays within[minimum, maximum]bounds.scripts/avocado-run-testplan (1)
190-195: Pre-existing security concern: shell=True with string formatting.While the formatting changes look good, this code uses
shell=Truewith string formatting, which is a security antipattern. Althoughpathis derived fromos.listdir(), it could still contain special shell characters that might cause unexpected behavior.Consider refactoring to avoid
shell=Trueor useshlex.quote()to safely escape the path:+import shlex + proc = subprocess.Popen( - "cd '%s' && git rev-parse HEAD" % path, + f"cd {shlex.quote(path)} && git rev-parse HEAD", stdout=subprocess.PIPE, stderr=subprocess.STDOUT, shell=True, )Or better, avoid
shell=Trueentirely by usingcwdparameter:proc = subprocess.Popen( - "cd '%s' && git rev-parse HEAD" % path, + ["git", "rev-parse", "HEAD"], + cwd=path, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, - shell=True, )requirements-dev.txt (1)
6-6: Consider pinning the pre-commit version.While the addition of pre-commit as a development dependency is appropriate, most other dependencies in this file are version-pinned (e.g.,
pyenchant==3.2.2,autopep8==1.6.0). Pinning the pre-commit version would maintain consistency and ensure reproducible development environments.-pre-commit +pre-commit==3.8.0Note: Adjust the version number to match your preferred pre-commit version.
.pylintrc_utils (1)
69-69: Consider the tradeoff of disablingtoo-many-positional-arguments.Disabling this check allows functions with many positional parameters, which can reduce API clarity. While this reduces linter noise, consider refactoring functions with excessive positional arguments to use keyword-only parameters or configuration objects where appropriate.
avocado/core/varianter.py (1)
352-352: Good refactoring withyield from, but theiter()call is redundant.The
yield frompattern is cleaner than explicit iteration. However, sinceiter_variantsis already a generator expression (defined at lines 347-351), the outeriter()call is unnecessary.Apply this diff to simplify:
- yield from iter(iter_variants) + yield from iter_variantsavocado/plugins/runners/avocado_instrumented.py (1)
166-166: LGTM: Clean refactor toyield from.The replacement of explicit iteration with
yield fromis idiomatic Python and properly delegates message yielding to the_monitorgenerator.Also applies to: 170-170
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
optional_plugins/html/avocado_result_html/templates/bootstrap.min.jsis excluded by!**/*.min.jsoptional_plugins/html/avocado_result_html/templates/datatables.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (88)
.github/workflows/project.yml(1 hunks).pre-commit-config.yaml(1 hunks).pylintrc(1 hunks).pylintrc_utils(1 hunks)avocado-static-checks.conf(0 hunks)avocado/core/data_dir.py(1 hunks)avocado/core/nrunner/runner.py(1 hunks)avocado/core/status/repo.py(1 hunks)avocado/core/varianter.py(1 hunks)avocado/plugins/config.py(1 hunks)avocado/plugins/json_variants.py(1 hunks)avocado/plugins/jsonresult.py(1 hunks)avocado/plugins/runners/avocado_instrumented.py(1 hunks)avocado/plugins/runners/package.py(1 hunks)avocado/plugins/runners/pip.py(2 hunks)avocado/plugins/runners/python_unittest.py(1 hunks)avocado/plugins/runners/sysinfo.py(1 hunks)avocado/plugins/runners/vmimage.py(1 hunks)avocado/utils/asset.py(1 hunks)avocado/utils/dmesg.py(1 hunks)avocado/utils/memory.py(1 hunks)avocado/utils/network/interfaces.py(1 hunks)avocado/utils/nvme.py(1 hunks)avocado/utils/output.py(1 hunks)avocado/utils/pmem.py(2 hunks)avocado/utils/ssh.py(1 hunks)avocado/utils/sysinfo.py(1 hunks)commitlint.config.ts(1 hunks)contrib/docker/Dockerfile.debian(0 hunks)contrib/docker/Dockerfile.fedora(0 hunks)contrib/job-scripts/coredump-handling/post.d/002-link-core-to-tests(2 hunks)contrib/packages/debian/changelog(0 hunks)contrib/scripts/avocado-safeloader-find-avocado-instrumented(2 hunks)contrib/scripts/avocado-safeloader-find-python-unittest(2 hunks)contrib/scripts/find-python-unittest(1 hunks)docs/source/blueprints/BP003.rst(1 hunks)docs/source/blueprints/BP004.rst(0 hunks)docs/source/blueprints/images/BP003/Resolution-to-task.drawio(1 hunks)docs/source/blueprints/images/BP003/Resolution.drawio(1 hunks)docs/source/blueprints/images/BP003/Task-scheduler-flow.drawio(1 hunks)docs/source/guides/contributor/chapters/ai_policy.rst(1 hunks)docs/source/guides/reference/chapters/runners.rst(2 hunks)docs/source/guides/user/chapters/concepts.rst(0 hunks)docs/source/guides/user/chapters/tags.rst(1 hunks)docs/source/guides/writer/chapters/writing.rst(0 hunks)docs/source/plugins/optional/results/index.rst(1 hunks)docs/source/releases/0_25_0.rst(0 hunks)docs/source/releases/108_0.rst(3 hunks)docs/source/releases/109_0.rst(1 hunks)docs/source/releases/112_0.rst(0 hunks)examples/tests/custom_env_variable.sh(0 hunks)examples/tests/failtest.sh(0 hunks)examples/tests/false(1 hunks)examples/tests/passtest.sh(0 hunks)examples/tests/raise.py.data/raise.c(1 hunks)examples/tests/sleeptest.py.data/sleeptest.json(1 hunks)examples/tests/true(1 hunks)examples/varianter_cit/test_params.cit(0 hunks)examples/yaml_to_mux/hw/hw.yaml(0 hunks)examples/yaml_to_mux/os/windows.yaml(0 hunks)optional_plugins/html/avocado_result_html/templates/avocado_html.js(2 hunks)optional_plugins/html/avocado_result_html/templates/bootstrap.css(1 hunks)optional_plugins/html/avocado_result_html/templates/datatables.css(0 hunks)optional_plugins/html/avocado_result_html/templates/jquery.js(5 hunks)optional_plugins/html/bootstrap_library_config.json(1 hunks)optional_plugins/mail/avocado_result_mail/result_mail.py(1 hunks)requirements-dev.txt(1 hunks)scripts/avocado-run-testplan(5 hunks)selftests/.data/get_data.py.data/GetData.test_a/long-cbbd/variant_data(1 hunks)selftests/.data/get_data.py.data/GetData.test_a/medium-1a7a/variant_data(1 hunks)selftests/.data/get_data.py.data/GetData.test_a/short-475e/variant_data(1 hunks)selftests/.data/get_data.py.data/GetData.test_a/test_data(1 hunks)selftests/.data/get_data.py.data/GetData.test_b/test_data(1 hunks)selftests/.data/get_data.py.data/file_data(1 hunks)selftests/.data/get_data.py.data/get_data.json(1 hunks)selftests/.data/guaca.a(1 hunks)selftests/.data/jenkins-junit.xsd(1 hunks)selftests/check.py(1 hunks)selftests/check_tmp_dirs(1 hunks)selftests/functional/nrunner.py(2 hunks)selftests/jobs/pass(1 hunks)selftests/jobs/status_server_auto(1 hunks)selftests/unit/datadir.py(1 hunks)selftests/unit/utils/astring.py(1 hunks)selftests/unit/utils/cpu.py.data/s390x(0 hunks)selftests/unit/utils/cpu.py.data/s390x_4(1 hunks)spell.ignore(1 hunks)static-checks(1 hunks)
💤 Files with no reviewable changes (17)
- docs/source/guides/user/chapters/concepts.rst
- examples/tests/passtest.sh
- examples/tests/failtest.sh
- avocado-static-checks.conf
- docs/source/releases/0_25_0.rst
- docs/source/guides/writer/chapters/writing.rst
- docs/source/releases/112_0.rst
- docs/source/blueprints/BP004.rst
- selftests/unit/utils/cpu.py.data/s390x
- contrib/packages/debian/changelog
- examples/yaml_to_mux/hw/hw.yaml
- contrib/docker/Dockerfile.debian
- examples/varianter_cit/test_params.cit
- contrib/docker/Dockerfile.fedora
- optional_plugins/html/avocado_result_html/templates/datatables.css
- examples/tests/custom_env_variable.sh
- examples/yaml_to_mux/os/windows.yaml
🧰 Additional context used
🧬 Code graph analysis (7)
avocado/utils/memory.py (1)
optional_plugins/varianter_yaml_to_mux/avocado_varianter_yaml_to_mux/mux.py (1)
items(281-284)
avocado/plugins/runners/pip.py (1)
avocado/core/utils/messages.py (3)
StdoutMessage(129-132)StderrMessage(135-138)FinishedMessage(51-89)
avocado/plugins/runners/avocado_instrumented.py (3)
avocado/plugins/runners/vmimage.py (1)
_monitor(109-127)avocado/core/exceptions.py (1)
TestInterrupt(155-162)avocado/utils/process.py (1)
terminate(774-781)
optional_plugins/mail/avocado_result_mail/result_mail.py (1)
avocado/core/job.py (1)
unique_id(511-516)
avocado/plugins/json_variants.py (2)
avocado/core/suite.py (1)
variants(264-275)avocado/core/varianter.py (1)
itertests(332-358)
avocado/plugins/runners/vmimage.py (3)
avocado/plugins/runners/avocado_instrumented.py (1)
_monitor(141-151)avocado/core/exceptions.py (1)
TestInterrupt(155-162)avocado/utils/process.py (1)
terminate(774-781)
avocado/core/varianter.py (1)
optional_plugins/varianter_yaml_to_mux/avocado_varianter_yaml_to_mux/mux.py (1)
iter_variants(79-97)
🪛 Ruff (0.14.3)
avocado/utils/nvme.py
276-276: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (38)
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: Linux with Python 3.11
- GitHub Check: Linux with Python 3.13
- GitHub Check: Linux with Python 3.12
- GitHub Check: Linux with Python 3.10
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: Podman spawner with 3rd party runner plugin
- GitHub Check: Podman Egg task
- GitHub Check: Egg task ubuntu:24.04
- GitHub Check: Egg task ubi:8.8
- GitHub Check: Egg task ubi:9.2
- GitHub Check: Egg task debian:11.0
- GitHub Check: Fedora selftests
- GitHub Check: Code Coverage (3.11)
- GitHub Check: Egg task debian:12.4
- GitHub Check: Egg task ubuntu:22.04
- GitHub Check: Version task ubuntu:22.04
- GitHub Check: Version task ubi:8.8
- GitHub Check: Windows with Python 3.12
- GitHub Check: Version task debian:12.4
- GitHub Check: macOS with Python 3.11
- GitHub Check: Code Coverage (3.11)
🔇 Additional comments (73)
docs/source/guides/user/chapters/tags.rst (1)
123-123: Good catch on the spelling correction.The typo fix from "endianess" to "endianness" is correct and improves documentation clarity. This is an isolated, low-impact documentation correction.
selftests/unit/utils/cpu.py.data/s390x_4 (1)
5-5: Trivial whitespace cleanup approved.Removing trailing whitespace from the features line is a minor formatting fix that has no functional impact on test data parsing.
avocado/utils/nvme.py (1)
276-276: Good catch on the typo fix.The spelling correction from "dettached" to "detached" is correct and improves clarity. The exception message is appropriately concise for this context, so the Ruff style hint is not a concern here.
docs/source/blueprints/BP003.rst (1)
559-559: Spelling correction looks good.Changing "activelly" to "actively" fixes a typographical error in the documentation. This is a necessary correction that improves clarity.
docs/source/guides/reference/chapters/runners.rst (2)
35-35: Spelling correction: "wihin" → "within".Typo fixed in the Motivation section. The correction is accurate and improves documentation clarity.
390-390: Spelling correction: "agreggate" → "aggregate".Typo fixed in the Task section description. The correction is accurate and improves documentation clarity.
selftests/jobs/pass (1)
16-20: Formatting changes align with pre-commit standardization.The changes to the
resolver.referencesconfiguration are purely stylistic (trailing commas and quotation normalization) with no functional impact on the test execution. This aligns with the PR's introduction of pre-commit hooks for static code quality.examples/tests/sleeptest.py.data/sleeptest.json (1)
1-1: Good catch on the trailing newline.Adding a trailing newline to text files (including JSON data files) aligns with POSIX best practices and the repository's standardization effort mentioned across the changeset. This formatting improvement has no functional impact on JSON parsing or test execution.
selftests/.data/get_data.py.data/GetData.test_b/test_data (1)
1-1: Trailing newline formatting aligned with pre-commit hook enforcement.This formatting change is consistent with the pre-commit hooks being introduced in this PR.
selftests/.data/get_data.py.data/file_data (1)
1-1: Trailing newline formatting aligned with pre-commit hook enforcement.This formatting change is consistent with the pre-commit hooks being introduced in this PR.
selftests/.data/get_data.py.data/get_data.json (1)
1-1: Trailing newline formatting aligned with pre-commit hook enforcement.This formatting change is consistent with the pre-commit hooks being introduced in this PR.
selftests/.data/get_data.py.data/GetData.test_a/medium-1a7a/variant_data (1)
1-1: Trailing newline formatting aligned with pre-commit hook enforcement.This formatting change is consistent with the pre-commit hooks being introduced in this PR.
selftests/.data/get_data.py.data/GetData.test_a/test_data (1)
1-1: Trailing newline formatting aligned with pre-commit hook enforcement.This formatting change is consistent with the pre-commit hooks being introduced in this PR.
selftests/.data/get_data.py.data/GetData.test_a/short-475e/variant_data (1)
1-1: Trailing newline formatting aligned with pre-commit hook enforcement.This formatting change is consistent with the pre-commit hooks being introduced in this PR.
selftests/.data/get_data.py.data/GetData.test_a/long-cbbd/variant_data (1)
1-1: Trailing newline formatting aligned with pre-commit hook enforcement.This formatting change is consistent with the pre-commit hooks being introduced in this PR.
optional_plugins/mail/avocado_result_mail/result_mail.py (1)
146-148: ✓ Formatting change looks good.The Subject header assignment is properly reformatted with clear parentheses grouping, making it readable while preserving the exact same behavior. No functional changes or issues here.
docs/source/blueprints/images/BP003/Resolution.drawio (1)
1-1: Formatting-only change; no review concerns.Trailing newline normalization in diagram file has no functional impact.
docs/source/blueprints/images/BP003/Task-scheduler-flow.drawio (1)
1-1: Formatting-only change; no review concerns.Line-ending normalization in diagram file has no functional impact.
docs/source/blueprints/images/BP003/Resolution-to-task.drawio (1)
1-1: Formatting-only change; no review concerns.Line-ending normalization in diagram file has no functional impact.
examples/tests/raise.py.data/raise.c (1)
7-7: LGTM! Typo fix improves documentation clarity.The spelling correction from "sinal number" to "signal number" is appropriate and enhances the readability of the comment.
selftests/check_tmp_dirs (1)
13-35: Formatting standardization looks good.The multiline reformatting of the list comprehension, quote normalization, and print statement wrapping are all correctly applied. The logic and functionality remain unchanged, and the formatting is consistent with the pre-commit hook standardization goals mentioned in the PR.
selftests/unit/datadir.py (1)
81-81: LGTM! Typo correction in test data path.The typo fix from "foor" to "foo" improves code quality without affecting test behavior.
contrib/scripts/avocado-safeloader-find-python-unittest (1)
31-31: LGTM! Clean formatting standardization.The changes standardize string quotes to use double quotes consistently, aligning with the pre-commit hook integration. The multi-line reformatting of the list comprehension on lines 44-47 also improves readability. No functional changes introduced.
Also applies to: 44-47, 49-49
contrib/scripts/find-python-unittest (1)
32-32: LGTM! Consistent quote standardization.The changes standardize string literals to use double quotes, matching the formatting improvements across the PR. No functional impact.
Also applies to: 40-40
contrib/scripts/avocado-safeloader-find-avocado-instrumented (2)
29-29: LGTM: Quote style standardization.The change to double quotes is consistent with the pre-commit hook standardization introduced in this PR.
39-42: LGTM: Improved readability.The multi-line list comprehension format improves readability without changing functionality.
examples/tests/true (1)
6-7: Consistent__main__guard formatting
Double quotes plus standard indentation keep style uniform without touching behavior.examples/tests/false (1)
6-7: Formatting now aligned with style
Indentation and quoting match the surrounding examples, keeping behavior identical.selftests/unit/utils/astring.py (1)
68-69: LGTM! Test expectation correctly updated.The ANSI escape sequences now consistently use hexadecimal notation (
\x1b) in the expected output. Both\x1b(hex) and\033(octal) represent the same ESC character (ASCII 27), so the test logic remains valid.optional_plugins/html/avocado_result_html/templates/bootstrap.css (1)
6834-6834: Trailing newline addition—approved.Straightforward formatting improvement: adding a trailing newline at end-of-file. No CSS behavior changes. Aligns with POSIX conventions and common linting standards.
optional_plugins/html/avocado_result_html/templates/jquery.js (5)
4054-4056: Comment-only tweak in .data sectionGrammar fix; no functional impact. LGTM.
6072-6074: “Re-enable scripts” comment typo fixSpelling/wording only. Safe change.
7121-7126: Background comment around CSS settersWording improvement only; behavior unchanged. Good to keep.
7708-7716: Clarification comment in propFilterDocumentation text only; no code changes. Looks good.
10524-10529: JSONP cleanup comment refinementComment edit only; logic untouched. All good.
scripts/avocado-run-testplan (8)
29-54: LGTM: Formatting improvements for consistency.The multi-line formatting, double-quote standardization, and trailing commas align with modern Python style guides and are appropriate for the pre-commit hooks being introduced.
57-64: LGTM: Dictionary formatting improvement.Multi-line formatting with trailing comma makes future additions cleaner.
115-121: LGTM: Improved readability.Multi-line dictionary formatting is more readable.
131-146: LGTM: Consistent parameter and return formatting.The double-quote standardization and multi-line return statement improve consistency.
149-154: LGTM: Consistent dictionary formatting.
161-163: LGTM: Quote style consistency.
175-184: LGTM: Print statement formatting.Multi-line formatting improves readability for long strings.
202-204: LGTM: Quote style consistency in main guard.avocado/utils/dmesg.py (1)
148-148: LGTM! Defensive initialization prevents static analysis warnings.Initializing
errto an empty string follows good defensive programming practices and satisfies static analyzers that might flag the variable as potentially unbound. This aligns well with the PR's objective of introducing pre-commit hooks for static checks.optional_plugins/html/avocado_result_html/templates/avocado_html.js (3)
27-27: Formatting changes look good.The whitespace adjustments improve readability by adding blank lines between logical sections (control container creation, event binding chain). The line break at line 112 maintains proper chaining syntax.
Also applies to: 30-30, 37-37, 40-40, 43-43, 46-46, 112-112, 117-117
51-57: Verify status column data extraction logic.Line 54 assumes
$(element).text()is safe even thoughelementcomes fromstatusColumn.data().unique().sort(). However, since this iterates over actual DOM elements in the DataTable column, this should be safe. The logic appears correct but consider adding a defensive check if column data could contain non-DOM objects in future refactors.
191-211: Column width constraint logic is sound.The minimum column width constraint (line 192-198) correctly recalculates the mouse offset and table width when the minimum is hit. The two-branch logic (lines 200-210) properly handles both table shrinking and expansion cases while maintaining column resizing capability.
commitlint.config.ts (1)
1-1: No file content visible for review.The annotated code section shows only the file path without any actual content. Unable to review the commitlint configuration without seeing the TypeScript code.
Could you verify that the file content was properly included in the review request?
optional_plugins/html/bootstrap_library_config.json (1)
402-402: LGTM! Trailing newline added.Adding a trailing newline at the end of the file aligns with common formatting standards enforced by pre-commit hooks.
avocado/plugins/json_variants.py (1)
102-102: LGTM! Usingyield fromfor cleaner delegation.The refactor to use
yield from self.variants.itertests()is more idiomatic Python and efficiently delegates iteration to the nested generator without changing behavior.avocado/utils/ssh.py (1)
1-1: LGTM! Docstring formatting corrected.The removal of the leading space after the opening triple quotes makes the docstring format more standard and consistent with PEP 257 conventions.
docs/source/releases/109_0.rst (1)
17-17: LGTM! Typo corrected.Fixed spelling error: "documentaion" → "documentation" in the hyperlink reference.
selftests/jobs/status_server_auto (1)
11-11: LGTM! Quote style normalized.Changed from single quotes to double quotes for string literal consistency, likely enforced by formatting tools in the new pre-commit setup.
docs/source/guides/contributor/chapters/ai_policy.rst (1)
114-114: LGTM! Improved formatting in Disclaimer section.Removing the bullet marker makes this line a proper continuation of the Disclaimer paragraph rather than a separate list item, improving the document structure.
avocado/core/data_dir.py (1)
376-376: Good defensive fix: prevents potential UnboundLocalError.Initializing
match_file = Noneensures the variable is defined even if the loop at lines 377-384 finds no matches, preventing an UnboundLocalError at line 386.avocado/utils/memory.py (1)
523-524: LGTM: cleaner delegation withyield from.The refactor to
yield from self.__dict__.items()is more concise and Pythonic than the previous manual loop. Behavior is identical.avocado/plugins/runners/package.py (1)
105-108: Good defensive error handling for invalid commands.Adding the else branch ensures that invalid commands are handled gracefully in
_run_software_manager, even though validation already occurs inrun()at lines 120-126. This defense-in-depth pattern prevents silent failures if the method is called directly.static-checks (1)
1-1: Verify the submodule commit is intentional and review upstream PR for scope of changes.The submodule commit
60820c9f3bd9061eb11fd11c7b481eb3b36d4501is valid and currently the HEAD of the avocado-static-checks repository. However, verify that this update aligns with the PR objectives by reviewing the upstream PR (avocado-framework/avocado-static-checks#14) to understand what specific changes are being introduced and confirm there are no unintended breaking changes.selftests/.data/guaca.a (1)
4-4: Archive payload now matches declared size.Dropping the leading hyphen keeps the 8-byte body in sync with the ar header, avoiding extraction complaints.
spell.ignore (1)
1-10: Confirm the trimmed ignore list covers all expected tokens.This drops a large number of previously ignored words. Please re-run the new pre-commit codespell hook (or equivalent pipeline) across the tree and confirm we’re not reintroducing a flood of false positives, or point to the new shared location that supersedes the removed entries.
.pylintrc (1)
99-99: LGTM - Disabling too-many-positional-arguments check.Adding R0917 to the disable list is a reasonable configuration choice to maintain consistency across the codebase during the static checks update.
avocado/plugins/config.py (1)
91-91: LGTM - Clean refactoring usingmax().Replacing the conditional comparison with
max()is more idiomatic and improves readability while maintaining identical behavior.avocado/plugins/runners/vmimage.py (1)
141-145: LGTM - Clean refactoring toyield frompattern.The change from explicit iteration to
yield from self._monitor(queue)is more Pythonic and aligns with similar refactorings across other runner modules in this PR. The pattern is correctly applied in both the normal execution path and theTestInterruptexception handler.avocado/plugins/jsonresult.py (1)
49-50: LGTM - Defensive fix prevents potential NameError.Adding the
elseclause ensures thatnameis always defined, even whentest_idis not aTestIDinstance. Without this, line 54 could raise aNameErrorwhen referencing an undefinednamevariable.docs/source/releases/108_0.rst (1)
18-18: LGTM - Documentation typo corrections.The typo fixes improve the quality of the release documentation.
Also applies to: 38-38, 51-51
selftests/check.py (1)
598-598: Pre-commit dependency is properly declared.Pre-commit is already listed in
requirements-dev.txt, so the integration at line 598 is valid. No action needed..pre-commit-config.yaml (1)
1-1: Disregard this review comment — the file being referenced does not exist in the repository.The
.pre-commit-config.yamlfile is not present in the codebase, nor are there any staged changes to create it. A search across the repository found no.pre-commit-config.yamlfiles. Whilepre-commitis listed as a development dependency inrequirements-dev.txtand referenced in test configurations, there is no configuration file to review at the location specified.Likely an incorrect or invalid review comment.
avocado/plugins/runners/pip.py (1)
48-64: LGTM: Proper initialization prevents UnboundLocalError.The initialization of
result = Noneand the conditional checks prevent accessing an unbound variable when no package name is provided, properly emitting an error message instead.avocado/plugins/runners/python_unittest.py (1)
170-170: LGTM: Initialization prevents UnboundLocalError.Initializing
next_execution_state_mark = Noneprevents an UnboundLocalError at line 182, where it's used in a comparison before being assigned (on the first loop iteration whenmost_current_execution_state_timeisNone).avocado/core/nrunner/runner.py (1)
76-76: LGTM: Initialization prevents UnboundLocalError.Initializing
next_execution_state_mark = 0prevents an UnboundLocalError at line 85, where it's used in a comparison before being assigned (on the first loop iteration whenmost_current_execution_state_timeisNone).avocado/plugins/runners/sysinfo.py (1)
176-176: LGTM: Initialization prevents UnboundLocalError.Initializing
next_execution_state_mark = Noneprevents an UnboundLocalError at line 188, where it's used in a comparison before being assigned (on the first loop iteration whenmost_current_execution_state_timeisNone).avocado/utils/pmem.py (2)
469-469: Good defensive initialization.The initialization of
argsbefore the conditional block is excellent defensive programming practice and satisfies static analysis requirements, even though the guard at lines 455-456 ensures at least one condition will assignargs.
515-515: Good defensive initialization.Consistent with line 469, this initialization ensures the variable is always defined before use, satisfying static analysis tools while improving code maintainability.
| filename = f"ifcfg-{self.name}" | ||
| if current_distro.name in ["rhel", "fedora"]: | ||
| path = "/etc/sysconfig/network-scripts" | ||
| path = "/etc/sysconfig/network-scripts" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Unconditional path assignment is appropriate for this function's scope.
The hardcoded path is correct since save_distro_rhel8_or_older() is only invoked for RHEL < 9 (line 665). However, note that Fedora systems are not explicitly handled in the save() method's branching logic (lines 661-673), even though other properties like config_filename and config_file_path treat RHEL and Fedora uniformly. Fedora would currently fall through to the else clause and raise an exception.
Please verify whether Fedora is intentionally unsupported by the save() method or if explicit handling should be added:
🏁 Script executed:
#!/bin/bash
# Description: Check for Fedora-specific handling in save-related methods and existing tests
# Search for Fedora references in the NetworkInterface class
rg -n -C3 -P '\bfedora\b' avocado/utils/network/interfaces.py
# Check if there are any tests that exercise save() on Fedora
rg -n --type=py -C3 'fedora.*save|save.*fedora' --iglob '*test*'Length of output: 689
🏁 Script executed:
#!/bin/bash
# Extract the save() method structure around the distro branching logic
# Show lines around 661-673 to see the branching structure
sed -n '660,675p' avocado/utils/network/interfaces.py
# Search for all occurrences of distro name checks within the save method
rg -n "current_distro\.name" avocado/utils/network/interfaces.py | grep -E "66[0-9]|67[0-3]"Length of output: 1056
🏁 Script executed:
#!/bin/bash
# Also search for the complete save() method structure to understand the full context
rg -n -A15 'def save\(self' avocado/utils/network/interfaces.py | head -100Length of output: 777
Critical issue: save() method documents Fedora support but lacks implementation.
The docstring at line 543 states "Currently, only RHEL, Fedora and SuSE are supported," but the branching logic (lines 661-673) only handles RHEL and SuSE explicitly. Fedora systems fall through to the else clause and raise a "Distro not supported" exception.
Other methods like config_filename and config_file_path (lines 102, 131) explicitly group rhel and fedora with unified logic, suggesting Fedora should be supported. Either:
- Extend the save() method's branching to handle Fedora (likely reusing RHEL logic for matching versions), or
- Remove Fedora from the docstring if it's intentionally unsupported
The unconditional path assignment at line 596 is correct; the issue is the missing Fedora handling at the save() method level.
🤖 Prompt for AI Agents
In avocado/utils/network/interfaces.py around lines 543, 596 and 661-673, the
save() docstring claims Fedora is supported but the branching ignores Fedora and
raises "Distro not supported"; update the save() method to handle Fedora the
same as RHEL by adding 'fedora' to the conditional(s) that currently match
'rhel' (so Fedora follows the same version-based path and file-writing logic),
and ensure any distro-specific path/filename resolution used elsewhere (e.g.,
config_filename/config_file_path) is reused; alternatively, if Fedora truly
should not be supported, remove it from the docstring at line 543—prefer the
first option and mirror the RHEL handling for 'fedora'.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #6231 +/- ##
==========================================
+ Coverage 73.72% 75.02% +1.30%
==========================================
Files 206 206
Lines 22507 22510 +3
==========================================
+ Hits 16593 16888 +295
+ Misses 5914 5622 -292 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
a63ec6c to
dee5ec0
Compare
b17f2fd to
068ef1f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (6)
selftests/.data/get_data.py (1)
79-83: Consistent newline handling; optional context-manager refactorThe updated expectations for
"get_data.py\n"and"b\n"are consistent with the fixture changes and withtest_a, so this looks good.If you want a tiny style tweak, you could use a context manager for the reads, e.g.
with open(file_data, encoding="utf-8") as f: self.assertEqual(f.read(), "get_data.py\n"), but this is optional in a quick test like this..pylintrc (1)
99-99: Consider keeping R0917 enabled or verify this change was intentional.R0917 (
too-many-positional-arguments) helps flag functions with excessive positional parameters, which can make code harder to read, call correctly, and maintain. Disabling this check means future violations won't be caught by pylint.If the project has many existing violations and you're addressing them incrementally, consider using inline
# pylint: disablecomments for specific cases rather than globally disabling the check.avocado/utils/network/interfaces.py (1)
594-610: Unconditional RHELnetwork-scriptspath is fine; consider deduping path logicGiven
save_distro_rhel8_or_older()is only called whencurrent_distro.name == "rhel"andversion < 9insave(), hardcoding/etc/sysconfig/network-scriptshere is functionally correct and simplifies the helper. As a minor cleanup, you could derive this fromself.config_file_path(or pass the directory in fromsave()) to avoid duplicating distro-specific paths in multiple places and keep all path logic centralized.avocado/utils/nvme.py (1)
270-277: Detach error message fix is fine; TRY003 is optional styleThe corrected error message (“namespace detached but still listing”) is clear and consistent with other NvmeException messages in this module. Ruff’s TRY003 recommendation (avoid per-call custom messages) is purely stylistic here; given the existing pattern of inline messages throughout this file, it’s reasonable to either keep this as-is or address TRY003 in a broader, module-wide pass later.
avocado/plugins/runners/vmimage.py (1)
129-146:yield from self._monitor(queue)is correct; consider joining the processDelegating to
_monitor()viayield fromin both the normal andTestInterruptpaths is behaviorally equivalent to the previous explicit loop and keeps this runner consistent with others.As a future improvement, you might consider calling
process.join()after_monitor()finishes (in both the normal and interrupted flows) to ensure the child process is explicitly reaped, especially if many vmimage tasks are run in a long-lived process. This is optional and not a functional regression from the prior version.avocado/plugins/runners/python_unittest.py (1)
169-186: PeriodicRunningMessagescheduling while the queue is empty is implemented correctlyThe introduction of
next_execution_state_marktogether withmost_current_execution_state_timeyields a firstRunningMessageimmediately and then atRUNNER_RUN_STATUS_INTERVALwhile the queue remains empty. The short-circuit condition avoids any uninitialized/None access, and the behavior is consistent with the status pacing used in other runners.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
optional_plugins/html/avocado_result_html/templates/bootstrap.min.jsis excluded by!**/*.min.jsoptional_plugins/html/avocado_result_html/templates/datatables.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (94)
.github/workflows/ci.yml(0 hunks).github/workflows/project.yml(1 hunks).pre-commit-config.yaml(1 hunks).pylintrc(1 hunks).pylintrc_utils(1 hunks)Makefile.gh(0 hunks)avocado-static-checks.conf(0 hunks)avocado/core/data_dir.py(1 hunks)avocado/core/nrunner/runner.py(1 hunks)avocado/core/status/repo.py(1 hunks)avocado/core/varianter.py(1 hunks)avocado/plugins/config.py(1 hunks)avocado/plugins/json_variants.py(1 hunks)avocado/plugins/jsonresult.py(1 hunks)avocado/plugins/runners/avocado_instrumented.py(1 hunks)avocado/plugins/runners/package.py(1 hunks)avocado/plugins/runners/pip.py(2 hunks)avocado/plugins/runners/python_unittest.py(1 hunks)avocado/plugins/runners/sysinfo.py(1 hunks)avocado/plugins/runners/vmimage.py(1 hunks)avocado/utils/asset.py(1 hunks)avocado/utils/dmesg.py(1 hunks)avocado/utils/memory.py(1 hunks)avocado/utils/network/interfaces.py(1 hunks)avocado/utils/nvme.py(1 hunks)avocado/utils/output.py(1 hunks)avocado/utils/pmem.py(2 hunks)avocado/utils/ssh.py(1 hunks)avocado/utils/sysinfo.py(1 hunks)commitlint.config.ts(1 hunks)contrib/docker/Dockerfile.debian(0 hunks)contrib/docker/Dockerfile.fedora(0 hunks)contrib/job-scripts/coredump-handling/post.d/002-link-core-to-tests(2 hunks)contrib/packages/debian/changelog(0 hunks)contrib/scripts/avocado-safeloader-find-avocado-instrumented(2 hunks)contrib/scripts/avocado-safeloader-find-python-unittest(2 hunks)contrib/scripts/find-python-unittest(1 hunks)docs/source/blueprints/BP003.rst(1 hunks)docs/source/blueprints/BP004.rst(0 hunks)docs/source/blueprints/images/BP003/Resolution-to-task.drawio(1 hunks)docs/source/blueprints/images/BP003/Resolution.drawio(1 hunks)docs/source/blueprints/images/BP003/Task-scheduler-flow.drawio(1 hunks)docs/source/guides/contributor/chapters/ai_policy.rst(1 hunks)docs/source/guides/contributor/chapters/how.rst(2 hunks)docs/source/guides/reference/chapters/runners.rst(2 hunks)docs/source/guides/user/chapters/concepts.rst(0 hunks)docs/source/guides/user/chapters/tags.rst(1 hunks)docs/source/guides/writer/chapters/writing.rst(0 hunks)docs/source/plugins/optional/results/index.rst(1 hunks)docs/source/releases/0_25_0.rst(0 hunks)docs/source/releases/108_0.rst(3 hunks)docs/source/releases/109_0.rst(1 hunks)docs/source/releases/112_0.rst(0 hunks)examples/tests/custom_env_variable.sh(0 hunks)examples/tests/failtest.sh(0 hunks)examples/tests/false(1 hunks)examples/tests/passtest.sh(0 hunks)examples/tests/raise.py.data/raise.c(1 hunks)examples/tests/sleeptest.py.data/sleeptest.json(1 hunks)examples/tests/true(1 hunks)examples/varianter_cit/test_params.cit(0 hunks)examples/yaml_to_mux/hw/hw.yaml(0 hunks)examples/yaml_to_mux/os/windows.yaml(0 hunks)optional_plugins/html/avocado_result_html/templates/avocado_html.js(2 hunks)optional_plugins/html/avocado_result_html/templates/bootstrap.css(1 hunks)optional_plugins/html/avocado_result_html/templates/datatables.css(0 hunks)optional_plugins/html/avocado_result_html/templates/jquery.js(5 hunks)optional_plugins/html/bootstrap_library_config.json(1 hunks)optional_plugins/mail/avocado_result_mail/result_mail.py(1 hunks)scripts/avocado-run-testplan(5 hunks)selftests/.data/get_data.py(3 hunks)selftests/.data/get_data.py.data/GetData.test_a/long-cbbd/variant_data(1 hunks)selftests/.data/get_data.py.data/GetData.test_a/medium-1a7a/variant_data(1 hunks)selftests/.data/get_data.py.data/GetData.test_a/short-475e/variant_data(1 hunks)selftests/.data/get_data.py.data/GetData.test_a/test_data(1 hunks)selftests/.data/get_data.py.data/GetData.test_b/test_data(1 hunks)selftests/.data/get_data.py.data/file_data(1 hunks)selftests/.data/get_data.py.data/get_data.json(1 hunks)selftests/.data/guaca.a(1 hunks)selftests/.data/jenkins-junit.xsd(1 hunks)selftests/check.py(1 hunks)selftests/check_tmp_dirs(1 hunks)selftests/functional/nrunner.py(2 hunks)selftests/jobs/pass(1 hunks)selftests/jobs/status_server_auto(1 hunks)selftests/pre-commit.sh(1 hunks)selftests/signedoff-check.sh(0 hunks)selftests/spell.sh(0 hunks)selftests/unit/datadir.py(1 hunks)selftests/unit/utils/astring.py(1 hunks)selftests/unit/utils/cpu.py.data/s390x(0 hunks)selftests/unit/utils/cpu.py.data/s390x_4(1 hunks)spell.ignore(1 hunks)static-checks(1 hunks)
💤 Files with no reviewable changes (21)
- docs/source/guides/writer/chapters/writing.rst
- Makefile.gh
- selftests/signedoff-check.sh
- avocado-static-checks.conf
- docs/source/guides/user/chapters/concepts.rst
- docs/source/releases/112_0.rst
- .github/workflows/ci.yml
- docs/source/blueprints/BP004.rst
- examples/yaml_to_mux/os/windows.yaml
- contrib/packages/debian/changelog
- contrib/docker/Dockerfile.debian
- examples/yaml_to_mux/hw/hw.yaml
- examples/tests/failtest.sh
- contrib/docker/Dockerfile.fedora
- examples/varianter_cit/test_params.cit
- selftests/spell.sh
- docs/source/releases/0_25_0.rst
- optional_plugins/html/avocado_result_html/templates/datatables.css
- examples/tests/custom_env_variable.sh
- examples/tests/passtest.sh
- selftests/unit/utils/cpu.py.data/s390x
✅ Files skipped from review due to trivial changes (9)
- selftests/.data/get_data.py.data/GetData.test_b/test_data
- selftests/.data/get_data.py.data/GetData.test_a/short-475e/variant_data
- selftests/.data/get_data.py.data/GetData.test_a/test_data
- optional_plugins/mail/avocado_result_mail/result_mail.py
- optional_plugins/html/avocado_result_html/templates/avocado_html.js
- selftests/unit/utils/cpu.py.data/s390x_4
- selftests/jobs/pass
- selftests/check_tmp_dirs
- selftests/.data/get_data.py.data/GetData.test_a/long-cbbd/variant_data
🚧 Files skipped from review as they are similar to previous changes (42)
- selftests/jobs/status_server_auto
- selftests/functional/nrunner.py
- docs/source/guides/user/chapters/tags.rst
- spell.ignore
- avocado/utils/memory.py
- .pylintrc_utils
- avocado/utils/sysinfo.py
- selftests/.data/get_data.py.data/get_data.json
- static-checks
- docs/source/blueprints/images/BP003/Task-scheduler-flow.drawio
- optional_plugins/html/bootstrap_library_config.json
- docs/source/blueprints/images/BP003/Resolution.drawio
- selftests/check.py
- optional_plugins/html/avocado_result_html/templates/bootstrap.css
- avocado/utils/asset.py
- avocado/plugins/runners/pip.py
- docs/source/releases/109_0.rst
- avocado/utils/output.py
- selftests/.data/get_data.py.data/GetData.test_a/medium-1a7a/variant_data
- .github/workflows/project.yml
- contrib/job-scripts/coredump-handling/post.d/002-link-core-to-tests
- docs/source/releases/108_0.rst
- avocado/plugins/runners/package.py
- contrib/scripts/find-python-unittest
- examples/tests/raise.py.data/raise.c
- contrib/scripts/avocado-safeloader-find-python-unittest
- selftests/unit/datadir.py
- selftests/.data/get_data.py.data/file_data
- avocado/core/varianter.py
- avocado/core/data_dir.py
- docs/source/blueprints/images/BP003/Resolution-to-task.drawio
- .pre-commit-config.yaml
- contrib/scripts/avocado-safeloader-find-avocado-instrumented
- avocado/utils/dmesg.py
- selftests/unit/utils/astring.py
- commitlint.config.ts
- avocado/plugins/runners/sysinfo.py
- docs/source/plugins/optional/results/index.rst
- optional_plugins/html/avocado_result_html/templates/jquery.js
- examples/tests/true
- avocado/utils/pmem.py
- selftests/.data/jenkins-junit.xsd
🧰 Additional context used
🧬 Code graph analysis (2)
avocado/plugins/runners/avocado_instrumented.py (3)
avocado/plugins/runners/vmimage.py (1)
_monitor(109-127)avocado/core/exceptions.py (1)
TestInterrupt(155-162)avocado/utils/process.py (1)
terminate(774-781)
avocado/plugins/runners/vmimage.py (3)
avocado/plugins/runners/avocado_instrumented.py (1)
_monitor(141-151)avocado/core/exceptions.py (1)
TestInterrupt(155-162)avocado/utils/process.py (1)
terminate(774-781)
🪛 Ruff (0.14.7)
avocado/utils/nvme.py
276-276: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (350)
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-41-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:fedora-41-aarch64
🔇 Additional comments (22)
examples/tests/sleeptest.py.data/sleeptest.json (1)
1-1: LGTM!This trailing newline addition is a minor housekeeping improvement and aligns with the PR's formatting cleanup objectives. The JSON content and test variant configurations remain functionally unchanged.
examples/tests/false (1)
6-7: Formatting-only change; behavior remains correctThe main-guard and
sys.exit(1)call are idiomatic and unchanged in behavior; the quote and indentation tweaks are fine.docs/source/guides/contributor/chapters/ai_policy.rst (1)
114-114: Good formatting cleanup.The removal of the bullet dash consolidates the Disclaimer section into a single flowing paragraph, which reads more naturally than a single-item bullet point. The addition of the trailing newline follows best practices for text files.
docs/source/guides/reference/chapters/runners.rst (2)
35-35: Typo correction: "wihin" → "within".The documentation correction is properly applied and enhances readability.
390-390: Typo correction: "agreggate" → "aggregate".The documentation correction is properly applied and improves accuracy of the technical description.
avocado/plugins/config.py (1)
91-91: LGTM! Cleaner, more Pythonic approach.The refactoring from an if-statement to using the built-in
max()function is semantically equivalent and improves readability.avocado/plugins/jsonresult.py (1)
46-55: Fix correctly initializesnamefor non-TestIDvaluesAdding the
else: name = str(test_id)ensuresnameis always defined per iteration and avoids leaking a previous test’snameor causingUnboundLocalError. The string coercion is appropriate here and keeps JSON output consistent.selftests/.data/guaca.a (1)
4-4: Test data normalization looks consistentThe updated content
cut, mixmatches the declared size8in the preceding header and keeps the archive structure valid. No issues from a test-data or format perspective.selftests/.data/get_data.py (1)
23-32: Expectation updates correctly reflect newline-terminated fixturesAligning the assertions with
"get_data.py\n"and"a\n"matches the updated test data and keeps these tests validating the exact on-disk contents. No issues from a correctness perspective.avocado/utils/ssh.py (1)
1-1: LGTM! Docstring formatting improvement.The removal of the leading space after the opening triple quotes aligns with PEP 257 docstring conventions.
docs/source/guides/contributor/chapters/how.rst (1)
187-235: Well-structured updates to contributor documentation.The additions to the Code Review section and the new "Using pre-commit" section effectively document the transition to pre-commit hooks. The installation steps, configuration details, check listings, and usage examples are clear and actionable. The note about auto-fix checks requiring re-staging is particularly helpful.
scripts/avocado-run-testplan (6)
29-54: LGTM! Formatting improvements enhance readability.The multi-line argument definitions and consistent double-quote usage improve code clarity while preserving all semantic details.
57-64: LGTM! Consistent quote style applied.The double-quote formatting aligns with the project's pre-commit standards.
115-121: LGTM! Multi-line dictionary improves readability.The expanded format makes the result structure clearer without changing behavior.
131-154: LGTM! Consistent formatting applied throughout.The quote style normalization and multi-line formatting improve consistency and readability.
161-184: LGTM! Print statement formatting improved.The multi-line format makes the output formatting clearer while maintaining functionality.
202-202: LGTM! Quote style consistency maintained.The double-quote usage aligns with the formatting standards applied throughout the file.
avocado/plugins/json_variants.py (1)
94-103: Delegating iteration withyield fromin__iter__looks goodThe new
yield from self.variants.itertests()is behaviorally equivalent to the previous explicit loop, keeps the existing guards for_NO_VARIANTSand uninitialized state, and is more idiomatic/concise.docs/source/blueprints/BP003.rst (1)
558-566: Active polling description is clear and consistentThe updated wording for active task status collection (execution-time polling vs. end-of-execution polling) reads clearly and matches the intended behavior; no further changes needed.
avocado/core/status/repo.py (1)
42-49: Multi-linefail_reasonassignment is a safe formatting improvementWrapping the long f-string in parentheses is a pure formatting change; it does not affect the constructed message or control flow. No further changes needed.
avocado/plugins/runners/avocado_instrumented.py (1)
153-171: Usingyield from self._monitor(queue)simplifies delegation cleanlyReplacing the explicit loop with
yield from self._monitor(queue)in both the normal andTestInterruptpaths preserves the existing behavior while making the intent clearer and reducing boilerplate.avocado/core/nrunner/runner.py (1)
69-88:running_loopinterval logic withnext_execution_state_marklooks correctInitializing
next_execution_state_markand recomputing it whenevermost_current_execution_state_timeis set ensures an immediate first “running” status, followed by periodic updates atRUNNER_RUN_STATUS_INTERVAL, without any risk of referencing an uninitialized local. The behavior is consistent with the interval-based status reporting used in other runners.
068ef1f to
04aa916
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (3)
scripts/avocado-run-testplan (1)
190-195: Security concern already flagged.The
shell=Truewith path concatenation was already identified in a previous review comment. The recommended fix using["git", "-C", path, "rev-parse", "HEAD"]withshell=Falseshould still be applied.avocado/utils/network/interfaces.py (1)
596-596: Previous critical issue about Fedora support remains unresolved.As flagged in the prior review, the
save()method's docstring (line 543) documents Fedora support, but the branching logic at lines 661-673 still does not handle Fedora systems—they fall through to raise "Distro not supported." Other properties likeconfig_filename(line 102) andconfig_file_path(line 131) explicitly group"rhel"and"fedora"together, suggesting Fedora should be supported.The unconditional path assignment at this line is acceptable given that
save_distro_rhel8_or_older()is only invoked for RHEL < 9, but the broader inconsistency persists.selftests/pre-commit.sh (1)
1-1: Remove trailing dash from shebang.The trailing dash in
#!/bin/bash -has no effect in shebangs and is non-standard. Use#!/bin/bashinstead.Apply this diff:
-#!/bin/bash - +#!/bin/bash
🧹 Nitpick comments (1)
avocado/core/varianter.py (1)
352-352: LGTM! Minor optimization opportunity.The
yield frompattern correctly delegates iteration. Sinceiter_variantsis already a generator expression (and thus an iterator), theiter()call is redundant—though harmless, asiter()on an iterator returns itself.Optionally simplify to:
- yield from iter(iter_variants) + yield from iter_variants
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
optional_plugins/html/avocado_result_html/templates/bootstrap.min.jsis excluded by!**/*.min.jsoptional_plugins/html/avocado_result_html/templates/datatables.min.jsis excluded by!**/*.min.js
📒 Files selected for processing (94)
.github/workflows/ci.yml(0 hunks).github/workflows/project.yml(1 hunks).pre-commit-config.yaml(1 hunks).pylintrc(1 hunks).pylintrc_utils(1 hunks)Makefile.gh(0 hunks)avocado-static-checks.conf(0 hunks)avocado/core/data_dir.py(1 hunks)avocado/core/nrunner/runner.py(1 hunks)avocado/core/status/repo.py(1 hunks)avocado/core/varianter.py(1 hunks)avocado/plugins/config.py(1 hunks)avocado/plugins/json_variants.py(1 hunks)avocado/plugins/jsonresult.py(1 hunks)avocado/plugins/runners/avocado_instrumented.py(1 hunks)avocado/plugins/runners/package.py(1 hunks)avocado/plugins/runners/pip.py(2 hunks)avocado/plugins/runners/python_unittest.py(1 hunks)avocado/plugins/runners/sysinfo.py(1 hunks)avocado/plugins/runners/vmimage.py(1 hunks)avocado/utils/asset.py(1 hunks)avocado/utils/dmesg.py(1 hunks)avocado/utils/memory.py(1 hunks)avocado/utils/network/interfaces.py(1 hunks)avocado/utils/nvme.py(1 hunks)avocado/utils/output.py(1 hunks)avocado/utils/pmem.py(2 hunks)avocado/utils/ssh.py(1 hunks)avocado/utils/sysinfo.py(1 hunks)commitlint.config.ts(1 hunks)contrib/docker/Dockerfile.debian(0 hunks)contrib/docker/Dockerfile.fedora(0 hunks)contrib/job-scripts/coredump-handling/post.d/002-link-core-to-tests(2 hunks)contrib/packages/debian/changelog(0 hunks)contrib/scripts/avocado-safeloader-find-avocado-instrumented(2 hunks)contrib/scripts/avocado-safeloader-find-python-unittest(2 hunks)contrib/scripts/find-python-unittest(1 hunks)docs/source/blueprints/BP003.rst(1 hunks)docs/source/blueprints/BP004.rst(0 hunks)docs/source/blueprints/images/BP003/Resolution-to-task.drawio(1 hunks)docs/source/blueprints/images/BP003/Resolution.drawio(1 hunks)docs/source/blueprints/images/BP003/Task-scheduler-flow.drawio(1 hunks)docs/source/guides/contributor/chapters/ai_policy.rst(1 hunks)docs/source/guides/contributor/chapters/how.rst(2 hunks)docs/source/guides/reference/chapters/runners.rst(2 hunks)docs/source/guides/user/chapters/concepts.rst(0 hunks)docs/source/guides/user/chapters/tags.rst(1 hunks)docs/source/guides/writer/chapters/writing.rst(0 hunks)docs/source/plugins/optional/results/index.rst(1 hunks)docs/source/releases/0_25_0.rst(0 hunks)docs/source/releases/108_0.rst(3 hunks)docs/source/releases/109_0.rst(1 hunks)docs/source/releases/112_0.rst(0 hunks)examples/tests/custom_env_variable.sh(0 hunks)examples/tests/failtest.sh(0 hunks)examples/tests/false(1 hunks)examples/tests/passtest.sh(0 hunks)examples/tests/raise.py.data/raise.c(1 hunks)examples/tests/sleeptest.py.data/sleeptest.json(1 hunks)examples/tests/true(1 hunks)examples/varianter_cit/test_params.cit(0 hunks)examples/yaml_to_mux/hw/hw.yaml(0 hunks)examples/yaml_to_mux/os/windows.yaml(0 hunks)optional_plugins/html/avocado_result_html/templates/avocado_html.js(2 hunks)optional_plugins/html/avocado_result_html/templates/bootstrap.css(1 hunks)optional_plugins/html/avocado_result_html/templates/datatables.css(0 hunks)optional_plugins/html/avocado_result_html/templates/jquery.js(5 hunks)optional_plugins/html/bootstrap_library_config.json(1 hunks)optional_plugins/mail/avocado_result_mail/result_mail.py(1 hunks)scripts/avocado-run-testplan(5 hunks)selftests/.data/get_data.py(3 hunks)selftests/.data/get_data.py.data/GetData.test_a/long-cbbd/variant_data(1 hunks)selftests/.data/get_data.py.data/GetData.test_a/medium-1a7a/variant_data(1 hunks)selftests/.data/get_data.py.data/GetData.test_a/short-475e/variant_data(1 hunks)selftests/.data/get_data.py.data/GetData.test_a/test_data(1 hunks)selftests/.data/get_data.py.data/GetData.test_b/test_data(1 hunks)selftests/.data/get_data.py.data/file_data(1 hunks)selftests/.data/get_data.py.data/get_data.json(1 hunks)selftests/.data/guaca.a(1 hunks)selftests/.data/jenkins-junit.xsd(1 hunks)selftests/check.py(1 hunks)selftests/check_tmp_dirs(1 hunks)selftests/functional/nrunner.py(2 hunks)selftests/jobs/pass(1 hunks)selftests/jobs/status_server_auto(1 hunks)selftests/pre-commit.sh(1 hunks)selftests/signedoff-check.sh(0 hunks)selftests/spell.sh(0 hunks)selftests/unit/datadir.py(1 hunks)selftests/unit/utils/astring.py(1 hunks)selftests/unit/utils/cpu.py.data/s390x(0 hunks)selftests/unit/utils/cpu.py.data/s390x_4(1 hunks)spell.ignore(1 hunks)static-checks(1 hunks)
💤 Files with no reviewable changes (21)
- examples/yaml_to_mux/os/windows.yaml
- examples/tests/passtest.sh
- .github/workflows/ci.yml
- avocado-static-checks.conf
- docs/source/guides/user/chapters/concepts.rst
- optional_plugins/html/avocado_result_html/templates/datatables.css
- Makefile.gh
- examples/varianter_cit/test_params.cit
- examples/yaml_to_mux/hw/hw.yaml
- contrib/docker/Dockerfile.debian
- docs/source/releases/0_25_0.rst
- examples/tests/failtest.sh
- contrib/packages/debian/changelog
- docs/source/guides/writer/chapters/writing.rst
- docs/source/blueprints/BP004.rst
- docs/source/releases/112_0.rst
- examples/tests/custom_env_variable.sh
- selftests/signedoff-check.sh
- selftests/spell.sh
- selftests/unit/utils/cpu.py.data/s390x
- contrib/docker/Dockerfile.fedora
✅ Files skipped from review due to trivial changes (12)
- examples/tests/false
- selftests/.data/get_data.py.data/GetData.test_a/medium-1a7a/variant_data
- selftests/.data/jenkins-junit.xsd
- selftests/.data/get_data.py.data/file_data
- optional_plugins/html/bootstrap_library_config.json
- selftests/.data/get_data.py.data/GetData.test_b/test_data
- selftests/.data/get_data.py.data/GetData.test_a/short-475e/variant_data
- examples/tests/raise.py.data/raise.c
- selftests/jobs/status_server_auto
- docs/source/blueprints/images/BP003/Resolution.drawio
- selftests/.data/get_data.py.data/GetData.test_a/test_data
- docs/source/guides/contributor/chapters/ai_policy.rst
🚧 Files skipped from review as they are similar to previous changes (45)
- selftests/unit/utils/cpu.py.data/s390x_4
- docs/source/releases/109_0.rst
- docs/source/plugins/optional/results/index.rst
- static-checks
- .github/workflows/project.yml
- avocado/core/status/repo.py
- avocado/plugins/runners/pip.py
- spell.ignore
- selftests/.data/get_data.py.data/GetData.test_a/long-cbbd/variant_data
- docs/source/blueprints/images/BP003/Task-scheduler-flow.drawio
- avocado/utils/asset.py
- selftests/functional/nrunner.py
- .pylintrc
- .pre-commit-config.yaml
- selftests/check_tmp_dirs
- contrib/scripts/find-python-unittest
- optional_plugins/html/avocado_result_html/templates/avocado_html.js
- avocado/plugins/runners/package.py
- selftests/check.py
- examples/tests/true
- docs/source/guides/reference/chapters/runners.rst
- contrib/job-scripts/coredump-handling/post.d/002-link-core-to-tests
- selftests/.data/get_data.py.data/get_data.json
- avocado/plugins/runners/vmimage.py
- contrib/scripts/avocado-safeloader-find-python-unittest
- docs/source/releases/108_0.rst
- docs/source/blueprints/images/BP003/Resolution-to-task.drawio
- avocado/utils/memory.py
- avocado/plugins/runners/avocado_instrumented.py
- optional_plugins/html/avocado_result_html/templates/jquery.js
- avocado/utils/pmem.py
- avocado/plugins/jsonresult.py
- avocado/utils/ssh.py
- avocado/plugins/config.py
- avocado/core/nrunner/runner.py
- avocado/utils/output.py
- docs/source/blueprints/BP003.rst
- selftests/jobs/pass
- contrib/scripts/avocado-safeloader-find-avocado-instrumented
- avocado/plugins/runners/sysinfo.py
- avocado/utils/dmesg.py
- avocado/plugins/runners/python_unittest.py
- optional_plugins/html/avocado_result_html/templates/bootstrap.css
- selftests/.data/guaca.a
- selftests/.data/get_data.py
🧰 Additional context used
🧬 Code graph analysis (4)
avocado/core/varianter.py (1)
optional_plugins/varianter_yaml_to_mux/avocado_varianter_yaml_to_mux/mux.py (1)
iter_variants(79-97)
avocado/plugins/json_variants.py (2)
avocado/core/suite.py (1)
variants(264-275)avocado/core/varianter.py (1)
itertests(332-358)
optional_plugins/mail/avocado_result_mail/result_mail.py (1)
avocado/core/job.py (1)
unique_id(511-516)
selftests/unit/datadir.py (1)
avocado/core/test.py (1)
logdir(390-394)
🪛 Ruff (0.14.8)
avocado/utils/nvme.py
276-276: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (45)
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-43-x86_64
- GitHub Check: rpm-build:centos-stream-9-x86_64
- GitHub Check: rpm-build:epel-9-x86_64
- GitHub Check: rpm-build:fedora-41-s390x
- GitHub Check: rpm-build:fedora-42-x86_64
- GitHub Check: rpm-build:fedora-41-ppc64le
- GitHub Check: rpm-build:fedora-rawhide-x86_64
- GitHub Check: rpm-build:fedora-41-aarch64
- GitHub Check: Code Coverage (3.11)
- GitHub Check: Analyze (python)
- GitHub Check: Code Coverage (3.11)
- GitHub Check: Windows with Python 3.13
- GitHub Check: macOS with Python 3.11
- GitHub Check: Version task debian:12.4
- GitHub Check: Windows with Python 3.12
- GitHub Check: Static checks
- GitHub Check: Fedora selftests
- GitHub Check: Build Package (wheel/tarball) for Python 3.13
- GitHub Check: Podman spawner with 3rd party runner plugin
- GitHub Check: Version task ubuntu:22.04
- GitHub Check: Version task ubi:8.8
- GitHub Check: Version task fedora:40
🔇 Additional comments (19)
docs/source/guides/user/chapters/tags.rst (1)
123-123: Good catch on the spelling correction.The typo fix from "endianess" to "endianness" aligns this line with the correct spelling used consistently throughout the rest of the documentation (lines 134, 140, 147, 155).
examples/tests/sleeptest.py.data/sleeptest.json (1)
1-1: LGTM! Standard formatting improvement.Adding a trailing newline is a best practice and aligns with POSIX standards. This formatting-only change has no functional impact on the test data.
selftests/unit/datadir.py (1)
81-81: LGTM! Typo correction improves test quality.The correction from "foor" to "foo" fixes a spelling error in the test path construction, aligning with the conventional placeholder names (foo, bar, baz).
optional_plugins/mail/avocado_result_mail/result_mail.py (1)
146-148: LGTM! Formatting change aligns with codebase style.This reformatting of the subject header assignment is purely cosmetic and matches the parentheses-wrapped pattern used elsewhere in the codebase (e.g., in
avocado/core/job.pyfor theunique_idproperty). The functional behavior remains identical.avocado/utils/nvme.py (1)
276-276: LGTM! Typo corrected.The spelling correction from "dettached" to "detached" improves code quality.
selftests/unit/utils/astring.py (1)
68-69: LGTM! Good normalization of ANSI escape sequences.The lowercase hex representation (
\x1b) is functionally identical to the uppercase version (\x1B) and improves consistency across the test expectations. This aligns well with the pre-commit hook standardization introduced in this PR.avocado/core/data_dir.py (1)
376-376: Good defensive initialization for static analysis compliance.Initializing
match_filebefore the loop ensures the variable is always defined, which satisfies static analysis tools and improves code clarity. This aligns well with the PR's objective of introducing pre-commit hooks and static checks.avocado/plugins/json_variants.py (1)
102-102: LGTM! Idiomatic delegation to sub-generator.The use of
yield fromto delegate iteration toself.variants.itertests()is the idiomatic Python approach and improves both readability and performance compared to an explicit loop..pylintrc_utils (1)
69-69: Approved: Disabling too-many-positional-arguments check.This configuration change relaxes the pylint check for functions with many positional arguments, which aligns with the broader pylint configuration cleanup mentioned in the PR objectives.
avocado/utils/sysinfo.py (1)
267-267: LGTM! Clean refactoring to useyield from.This change improves readability by replacing the explicit loop with the more Pythonic
yield fromstatement, which directly delegates to the iterator. The behavior remains unchanged.scripts/avocado-run-testplan (6)
28-54: LGTM on argparse formatting.The multi-line reformatting and consistent double-quote usage improve readability while preserving original behavior.
57-64: LGTM on RESULT_MAP formatting.Multi-line format with trailing comma is consistent with modern Python style.
115-121: LGTM on results formatting.Multi-line dictionary literal improves readability.
131-154: LGTM on method formatting.Consistent double-quote usage and multi-line formatting for return statements.
161-184: LGTM on save/report formatting.Consistent quote style and multi-line print statements improve readability.
202-204: LGTM on main guard formatting.Consistent double-quote usage.
docs/source/guides/contributor/chapters/how.rst (3)
187-189: LGTM: Clear distinction between pre-commit checks and selftests.The updated text appropriately clarifies that pre-commit checks handle code quality, while selftests verify functional correctness.
223-229: LGTM: Clear usage examples for manual pre-commit execution.The documentation provides clear examples for running pre-commit checks manually on all files or specific files.
231-234: LGTM: Helpful note about auto-fixing behavior.The note appropriately warns users about hooks that automatically fix issues and the need to stage changes after auto-fixes.
| @@ -0,0 +1 @@ | |||
| ./static-checks/commitlint.config.ts No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
fd -t f "commitlint.config" --exec cat {}Repository: avocado-framework/avocado
Length of output: 51
🏁 Script executed:
ls -la static-checks/ 2>/dev/null || echo "Directory does not exist"
fd -t f . static-checks/ 2>/dev/null || echo "No files found"
git ls-files | grep -i commitlint || echo "No commitlint files in git tracking"Repository: avocado-framework/avocado
Length of output: 200
🏁 Script executed:
cat static-checks/commitlint.config.tsRepository: avocado-framework/avocado
Length of output: 135
🏁 Script executed:
file static-checks/commitlint.config.ts
od -c static-checks/commitlint.config.ts 2>&1 || hexdump -C static-checks/commitlint.config.ts 2>&1
ls -lah static-checks/ | grep commitlintRepository: avocado-framework/avocado
Length of output: 215
🏁 Script executed:
git show HEAD:static-checks/commitlint.config.ts 2>&1 || git ls-files -s | grep commitlint
python3 -c "
import os
path = 'static-checks/commitlint.config.ts'
try:
with open(path, 'r') as f:
content = f.read()
print('Content:', repr(content))
except Exception as e:
print('Error:', e)
" 2>&1Repository: avocado-framework/avocado
Length of output: 294
🏁 Script executed:
readlink commitlint.config.ts 2>&1 || echo "readlink failed"
git cat-file blob 2c21d1f109cdccefb6f88f8667aa7bb911b915bf
git ls-files --stage | grep commitlintRepository: avocado-framework/avocado
Length of output: 212
Fix broken commitlint configuration symlink.
The file commitlint.config.ts is a symlink pointing to ./static-checks/commitlint.config.ts, but this target file does not exist in the repository. The symlink must either be removed and replaced with actual commitlint configuration, or the target file must be created at the referenced location with valid TypeScript configuration code.
A proper commitlint.config.ts should export a configuration object like:
export default {
extends: ['@commitlint/config-conventional'],
rules: {
// custom rules here
}
};🤖 Prompt for AI Agents
In commitlint.config.ts around line 1, the file is currently a broken symlink
pointing to ./static-checks/commitlint.config.ts which does not exist; replace
the symlink with a real TypeScript config file or create the missing target. Fix
by either deleting the symlink and adding a concrete commitlint.config.ts that
exports a default config object (extending @commitlint/config-conventional and
any project-specific rules), or create ./static-checks/commitlint.config.ts with
the same exported default config; ensure the file is valid TypeScript and
committed (no symlink).
| @@ -0,0 +1,2 @@ | |||
| #!/bin/bash - | |||
| python3 -m pre_commit run --all-files | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use pre-commit run --all-files directly instead of python3 -m pre_commit.
The standard invocation is pre-commit run --all-files, not the Python module form. While python3 -m pre_commit is technically valid, it's unconventional. Use the direct command instead for consistency with the ecosystem.
🤖 Prompt for AI Agents
In selftests/pre-commit.sh around line 2, replace the unconventional invocation
"python3 -m pre_commit run --all-files" with the standard direct command
"pre-commit run --all-files"; update the script to call the pre-commit
executable directly so it uses the usual CLI entrypoint and matches
project/contributor expectations.
04aa916 to
4135347
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/source/guides/contributor/chapters/how.rst`:
- Around line 207-210: The docs show running "pip install -r
static-checks/requirements.txt" and then "pre-commit install" but omit
installing the commit-msg hook required by commitlint; update the instructions
to include installing the commit-msg hook (for example by adding guidance to run
the commit-msg hook installation such as "pre-commit install --hook-type
commit-msg" or to run both hook types) and mention that this is needed for
commitlint to validate commit messages.
♻️ Duplicate comments (1)
scripts/avocado-run-testplan (1)
175-195: Replaceshell=Truewith a safe argument list.
Directory names can inject shell metacharacters; usegit -Cwithshell=False.🔒 Proposed fix
- proc = subprocess.Popen( - "cd '%s' && git rev-parse HEAD" % path, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - shell=True, - ) - out = proc.communicate()[0].strip().decode() - if not proc.poll(): - print("%s: %s" % (name, out)) + proc = subprocess.run( + ["git", "-C", path, "rev-parse", "HEAD"], + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + check=False, + ) + out = proc.stdout.strip() + if proc.returncode == 0: + print("%s: %s" % (name, out))#!/bin/bash # Verify no other shell=True usage in this script. rg -n 'shell=True' scripts/avocado-run-testplan
🧹 Nitpick comments (1)
.pylintrc (1)
65-100: Ensure pylint version constraint supportsR0917(introduced in Pylint 3.3.0).
R0917(too-many-positional-arguments) was only implemented in Pylint 3.3.0+. If the project uses an earlier version without an explicit minimum version constraint, this message ID will be silently ignored. Consider pinningpylint>=3.3.0in project dependencies to ensure consistent linting behavior across environments.
| To set up pre-commit for the first time:: | ||
|
|
||
| $ AVOCADO_CHECK_LEVEL=99 | ||
| $ ./contrib/scripts/avocado-check-pr.sh -i -v | ||
| $ pip install -r static-checks/requirements.txt | ||
| $ pre-commit install |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commitlint setup instructions may be incomplete.
The documentation mentions commitlint for commit message validation (line 221), but commitlint typically requires installing both the pre-commit hook AND the commit-msg hook. The current instructions at line 210 only show pre-commit install, which installs the pre-commit hook but not the commit-msg hook needed for commitlint.
📝 Suggested addition to installation steps
$ pip install -r static-checks/requirements.txt
$ pre-commit install
+ $ pre-commit install --hook-type commit-msgBased on standard pre-commit and commitlint integration patterns.
🤖 Prompt for AI Agents
In `@docs/source/guides/contributor/chapters/how.rst` around lines 207 - 210, The
docs show running "pip install -r static-checks/requirements.txt" and then
"pre-commit install" but omit installing the commit-msg hook required by
commitlint; update the instructions to include installing the commit-msg hook
(for example by adding guidance to run the commit-msg hook installation such as
"pre-commit install --hook-type commit-msg" or to run both hook types) and
mention that this is needed for commitlint to validate commit messages.
harvey0100
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally - auto-fix works (isort/black fix files and require re-staging before commit passes). Note: there's a merge conflict in spell.ignore that needs rebasing. Otherwise LGTM 👍
This commit introduces the latest changes in avocado-static-checks repo, which uses pre-commit hooks instead of current scripts solution. Reference: avocado-framework/avocado-static-checks#14 Signed-off-by: Jan Richter <[email protected]>
After introduction of pre-commit tool the black has been updated to version 25.9.0. This is updates after this change. Signed-off-by: Jan Richter <[email protected]>
This is a set of fixed caught by pylint after update to version 4.0.0 Signed-off-by: Jan Richter <[email protected]>
This is a set of typo fixes which occurred during migration from pylint spell to codespell tool. Signed-off-by: Jan Richter <[email protected]>
4135347 to
1b84dc7
Compare
|
Hi @harvey0100, thanks for the review. I have resolved the conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
contrib/scripts/avocado-safeloader-find-avocado-instrumented (1)
33-36: Don’t silently skip IO errors.Skipping unreadable modules without any notice can lead to missing tests with no signal. At least log to stderr so users know something was omitted.
💡 Suggested adjustment
try: test_class_methods, _ = find_avocado_tests(test_module_path) except IOError as error: + print(f"Skipping {test_module_path}: {error}", file=sys.stderr) continue
🤖 Fix all issues with AI agents
In @.pre-commit-config.yaml:
- Line 1: The .pre-commit-config.yaml symlink at the repo root points to a
missing target (./static-checks/.pre-commit-config.yaml); fix by either creating
the target file static-checks/.pre-commit-config.yaml with the intended
pre-commit configuration or removing/updating the symlink
.pre-commit-config.yaml to point to the correct existing config. Ensure the
created file contains the expected pre-commit hook definitions (or adjust the
symlink target) so tools that read .pre-commit-config.yaml at the repo root can
find a valid config.
🧹 Nitpick comments (1)
selftests/check.py (1)
595-598: Consider narrowing the glob pattern to explicitly include static-check scripts.The glob
selftests/*.shcurrently picks up three check-related scripts (modules-boundaries.sh, pre-commit.sh, safeloader.sh), which is correct. However, the pattern is fragile—any new shell script added toselftests/will be automatically included without review. SinceTEST_SIZE["static-checks"] = 3suggests an intentional count, consider using explicit file names or a dedicated directory/naming convention (e.g.,selftests/static-*.shor a dedicatedselftests/checks/directory) to prevent accidental inclusion of future tests.
| @@ -0,0 +1 @@ | |||
| ./static-checks/.pre-commit-config.yaml No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if file exists and show its contents
if [ -f "static-checks/.pre-commit-config.yaml" ]; then
echo "=== File exists ==="
sed -n '1,200p' static-checks/.pre-commit-config.yaml
else
echo "=== File does not exist ==="
# Search for any .pre-commit-config.yaml files in the repo
find . -name ".pre-commit-config.yaml" -type f 2>/dev/null
fiRepository: avocado-framework/avocado
Length of output: 96
🏁 Script executed:
#!/bin/bash
# Search for any .pre-commit-config.yaml files
find . -name ".pre-commit-config.yaml" -type f 2>/dev/null | head -20
# Also check the root and static-checks directory structure
echo "=== Directory structure ==="
ls -la static-checks/ 2>/dev/null || echo "static-checks directory does not exist"
echo ""
ls -la . | grep -E "\.pre-commit|static"Repository: avocado-framework/avocado
Length of output: 581
🏁 Script executed:
#!/bin/bash
# Check the symlink and its target
echo "=== Checking .pre-commit-config.yaml symlink ==="
ls -la .pre-commit-config.yaml
readlink .pre-commit-config.yaml
echo ""
echo "=== Attempting to read the file ==="
cat .pre-commit-config.yaml 2>&1 || echo "Cannot read file"
echo ""
echo "=== Checking if target exists ==="
test -e ./static-checks/.pre-commit-config.yaml && echo "Target exists" || echo "Target does NOT exist (broken symlink)"Repository: avocado-framework/avocado
Length of output: 455
The .pre-commit-config.yaml symlink is broken and points to a non-existent file.
The symlink at .pre-commit-config.yaml points to ./static-checks/.pre-commit-config.yaml, which does not exist. The target file must be created in the static-checks/ directory or the symlink removed.
🤖 Prompt for AI Agents
In @.pre-commit-config.yaml at line 1, The .pre-commit-config.yaml symlink at
the repo root points to a missing target
(./static-checks/.pre-commit-config.yaml); fix by either creating the target
file static-checks/.pre-commit-config.yaml with the intended pre-commit
configuration or removing/updating the symlink .pre-commit-config.yaml to point
to the correct existing config. Ensure the created file contains the expected
pre-commit hook definitions (or adjust the symlink target) so tools that read
.pre-commit-config.yaml at the repo root can find a valid config.
This PR introduces the latest changes in avocado-static-checks repo,
which uses pre-commit hooks instead of current scripts solution.
Reference:
avocado-framework/avocado-static-checks#14
Signed-off-by: Jan Richter [email protected]
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.