Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Refactor MissingFilePermissionsRule testing #1568

Merged
merged 3 commits into from May 21, 2021
Merged

Refactor MissingFilePermissionsRule testing #1568

merged 3 commits into from May 21, 2021

Conversation

konstruktoid
Copy link
Contributor

This PR:

  • was meant to verify Tests erroneously pass when commented out code is present #1532, but # mode: 0755 fails correctly when tested
  • merges tests into src/ansiblelint/rules/MissingFilePermissionsRule.py and removes test/TestMissingFilePermissionsRule.py
  • adds a comment test to src/ansiblelint/rules/NoLogPasswordsRule.py as additional verification

@konstruktoid
add user_password comment test
3ac5218 
Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>
@konstruktoid
- This PR was meant to test #1532
1e9b270 
- Merge tests into `src/ansiblelint/rules/MissingFilePermissionsRule.py`

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>
@konstruktoid
noqa: C901 due to the number of tests
476ca00 
Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>
@konstruktoid
Copy link
Contributor Author

It could most likely be optimized but I added noqa: C901 (https://www.flake8rules.com/rules/C901.html), which is triggered due to the number of tests.

@ssbarnea ssbarnea changed the title Merge and expand MissingFilePermissionsRule testing Refactor MissingFilePermissionsRule testing May 21, 2021
@ssbarnea
Copy link
Member

I think that case is ok for an exclusion, especially as I think is impossible to avoid with that approach.

@ssbarnea ssbarnea merged commit d6f3240 into ansible:master May 21, 2021
@konstruktoid konstruktoid deleted the ISSUE1532 branch May 21, 2021 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ssbarnea ssbarnea ssbarnea approved these changes

Assignees
No one assigned
Labels
refactoring
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@konstruktoid @ssbarnea