[CI] Add three pre-commit pygrep hooks - #12933
Conversation
https://github.com/pre-commit/pygrep-hooks Official hooks from pre-commit some more basic checks or tests we can run
|
@blueorangutan package |
|
@jbampton a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12933 +/- ##
=========================================
Coverage 19.78% 19.78%
+ Complexity 19995 19993 -2
=========================================
Files 6371 6371
Lines 575909 575909
Branches 70509 70509
=========================================
+ Hits 113950 113951 +1
- Misses 449526 449529 +3
+ Partials 12433 12429 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17311 |
There was a problem hiding this comment.
Pull request overview
Adds additional upstream pygrep-hooks checks to the repository’s pre-commit configuration to catch a few common Python/text pitfalls earlier in CI and local developer workflows.
Changes:
- Add
python-check-mock-methodsto detect common invalidunittest.mockmethod usages. - Add
python-use-type-annotationsto discourage PEP 484 type comments in favor of annotations (with a targeted exclude). - Add
text-unicode-replacement-charto forbid UTF-8 replacement characters in committed files.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟢 Approval recommended
The changes are limited to CI/pre-commit configuration, with only minor wording/spelling fixes suggested for clarity.
Review details
Suppressed comments (1)
.pre-commit-config.yaml:200
- The hook name uses the abbreviation "char" while the description spells out "character"; keeping these consistent avoids confusion.
name: run check for no unicode replacement char
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
| name: run check for not-real mock methods | ||
| description: Prevent common mistakes of assert mck.not_called(), assert mck.called_once_with(...) and mck.assert_called |
| - repo: https://github.com/pre-commit/pygrep-hooks | ||
| rev: 3a6eb0fadf60b3cccfd80bad9dbb6fae7e47b316 # frozen: v1.10.0 | ||
| hooks: | ||
| - id: python-check-mock-methods |
There was a problem hiding this comment.
Is this one adding anything for us right now?
| - id: python-use-type-annotations | ||
| name: run check for type annotations not comments | ||
| description: Enforce that python3.6+ type annotations are used instead of type comments | ||
| exclude: ^scripts/vm/hypervisor/xenserver/xenserver56/InterfaceReconfigure\.py$ |
There was a problem hiding this comment.
Why is this one file skipped? I looked and it has no type comment in it. It just has a normal sentence that starts with the word "type". So any comment written like that in future will fail and end up added to this list. Is that ok?
| name: run check for type annotations not comments | ||
| description: Enforce that python3.6+ type annotations are used instead of type comments | ||
| exclude: ^scripts/vm/hypervisor/xenserver/xenserver56/InterfaceReconfigure\.py$ | ||
| - id: text-unicode-replacement-char |
There was a problem hiding this comment.
This runs over every file in the repo. Is there a chance some test data has one of these characters on purpose?
https://github.com/pre-commit/pygrep-hooks
Official hooks from pre-commit some more basic checks or tests we can run
Description
This PR...
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?