Various python fixes #10345
Merged
Various python fixes #10345
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi @maage. Thanks for your PR. I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Start a new ephemeral environment with changes proposed in this pull request: Fedora Environment Oracle Linux 8 Environment |
|
I think these are unrelated, issue probably was before, and change, if any, is not obvious |
Mab879
requested changes
Mar 22, 2023
Directory creation needs to be TOCTTOU resistant because tests etc can be parallel. Make mkdir_p return true if creation succeeded. If directory was not created, return false. In many cases checked existence of a entity (os.path.exists), not a directory. Error if target is not directory. On error raise.
When having '-' in '[...]' you need to put it first or last.
[+-.] is actually '+', ',', '-', '.'
053 43 2B +
054 44 2C ,
055 45 2D -
056 46 2E .
…ment From pyflakes: ssg/xml.py:431:5: redefinition of unused 'get_test_action_ref_element' from line 427
pyflakes: tests/ssg_test_suite/log.py:52:15: '...'.format(...) has unused arguments at position(s): 1 As format string does not have 2nd replacement, this must be mistake. See: https://docs.python.org/3/library/stdtypes.html#str.format
pyflakes: ssg/entities/profile_base.py:68:24: undefined name 'CPEDoesNotExist' ssg/entities/profile_base.py:72:27: undefined name 'CPEDoesNotExist'
pyflakes: ssg/content_diff.py:96:79: undefined name 'idref' ssg/content_diff.py:99:64: undefined name 'idref' ...
…ObjectException pyflakes: utils/ansible_playbook_to_role.py:436:19: undefined name 'UnknownObjectException'
At least: ./automatus.py rule <rule> ... Seems to work just fine with openscap-scanner v1.3.6.
pyflakes: tests/ssg_test_suite/common.py:435:13: '...'.format(...) has unused named argument(s): test_dir_config tests/test_profile_stability.py:140:13: '...'.format(...) has unused named argument(s): test_root tests/unit/ssg-module/test_id_translate.py:182:13: '...'.format(...) has unused named argument(s): ou
pyflakes: utils/srg_audit.py:100:26: f-string is missing placeholders utils/srg_audit.py:104:26: f-string is missing placeholders
pyflakes: tests/unit/ssg-module/test_build_remediations.py:71:5: local variable 'fixes' is assigned to but never used
flake8: utils/add_platform_rule.py:232:12: E711 comparison to None should be 'if cond is None:' utils/add_platform_rule.py:323:20: E711 comparison to None should be 'if cond is None:'
…o compare to truthy flake8: tests/unit/ssg_test_suite/test_matches_platform.py:9:72: E712 comparison to True should be 'if cond is True:' or 'if cond:' tests/unit/ssg_test_suite/test_matches_platform.py:15:72: E712 comparison to False should be 'if cond is False:' or 'if not cond:' tests/unit/ssg_test_suite/test_matches_platform.py:21:72: E712 comparison to True should be 'if cond is True:' or 'if cond:' tests/unit/ssg_test_suite/test_matches_platform.py:27:72: E712 comparison to True should be 'if cond is True:' or 'if cond:' tests/unit/ssg_test_suite/test_matches_platform.py:33:72: E712 comparison to False should be 'if cond is False:' or 'if not cond:' tests/unit/ssg_test_suite/test_matches_platform.py:40:72: E712 comparison to True should be 'if cond is True:' or 'if cond:' tests/unit/ssg_test_suite/test_matches_platform.py:47:72: E712 comparison to True should be 'if cond is True:' or 'if cond:' tests/unit/ssg_test_suite/test_matches_platform.py:54:72: E712 comparison to False should be 'if cond is False:' or 'if not cond:' tests/unit/ssg_test_suite/test_matches_platform.py:60:72: E712 comparison to True should be 'if cond is True:' or 'if cond:' tests/unit/ssg_test_suite/test_matches_platform.py:66:72: E712 comparison to True should be 'if cond is True:' or 'if cond:' tests/unit/ssg_test_suite/test_matches_platform.py:72:72: E712 comparison to False should be 'if cond is False:' or 'if not cond:' tests/unit/ssg_test_suite/test_matches_platform.py:79:72: E712 comparison to True should be 'if cond is True:' or 'if cond:' tests/unit/ssg_test_suite/test_matches_platform.py:86:72: E712 comparison to True should be 'if cond is True:' or 'if cond:' tests/unit/ssg_test_suite/test_matches_platform.py:93:72: E712 comparison to False should be 'if cond is False:' or 'if not cond:' tests/unit/ssg_test_suite/test_matches_platform.py:101:72: E712 comparison to True should be 'if cond is True:' or 'if cond:' tests/unit/ssg_test_suite/test_matches_platform.py:108:72: E712 comparison to True should be 'if cond is True:' or 'if cond:' tests/unit/ssg_test_suite/test_matches_platform.py:116:72: E712 comparison to True should be 'if cond is True:' or 'if cond:' tests/unit/ssg_test_suite/test_matches_platform.py:123:72: E712 comparison to False should be 'if cond is False:' or 'if not cond:' tests/unit/ssg_test_suite/test_matches_platform.py:131:72: E712 comparison to False should be 'if cond is False:' or 'if not cond:'
flake8: build-scripts/verify_references.py:255:21: E713 test for membership should be 'not in' build-scripts/verify_references.py:260:21: E713 test for membership should be 'not in'
utils/mod_checks.py:159:8: E713 test for membership should be 'not in' utils/mod_fixes.py:163:8: E713 test for membership should be 'not in' utils/mod_prodtype.py:169:8: E713 test for membership should be 'not in'
Now should be rebased to fresh master w/o merge commits. |
|
Code Climate has analyzed commit d386ef3 and detected 2 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 43.4% (50% is the threshold). This pull request will bring the total coverage in the repository to 51.9% (0.1% change). View more on Code Climate. |
Mab879
approved these changes
Mar 24, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
Collection of various python code fixes and some style changes.
Rationale:
Improve
mkdir_pand then use it everywhere. There was duplicate implementations. Now also handle errors when directory already exists if there is issues with parallelism for example. Patch could further improved if only recent python versions are used.Fix
escape_regexto match comment.Probably search and replace fix with xccdf version.
Some duplicate code. Some codes I wonder how it was previously working as intended.
Bad helper in automatus.
And then some style changes that I see as obvious.
Review Hints:
pyflakes / flake8 issues are only reduced before and after this patch. If you ignore file length.
Parallel builds to check
mkdir_pbefore and after. It might be possible to effectively and repeatedly to see issues this might require cmake modifications to allow further parallelism.escape_regexchange might cause xml modifications.Sofar only built and run ctest with this.