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.

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

Disable warnings related to the use of assert in the tests directory as reported with lint-vetting #792

Merged
merged 8 commits into from
Jan 25, 2022

Conversation

cidrblock
Copy link
Collaborator

@cidrblock cidrblock commented Jan 23, 2022

This adds a few additional files to the per-file-ignore list within the flake8 configuration with an exemption for S101 which is reported by flake8-bandit as part of the wemake style guide which is being vetted.

The list was also alphabetized to ensure new entries were placed in a sane location.

A note was added to the top of the flake8 per-file-ignores indicating that the glob for the tests folder ignoring s101 can be added later. Currently there is a conflict between globs and per-file entries. See PyCQA/flake8#670

This eliminates the S101 entry from the output of tox -e lint-vetting so users of it can see messages only related to the changes they are making and not for files that were previously added to the repository:

1     C812 missing trailing comma
1     RST304 Unknown interpreted text role "mod".
- 13    S101 Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
1     S404 Consider possible security implications associated with the subprocess module.
1     S603 subprocess call - check for execution of untrusted input.
2     S604 Function call with shell=True parameter identified, possible security issue.
2     WPS111 Found too short name: p < 2
2     WPS115 Found upper-case constant in a class: KEGEX
3     WPS201 Found module with too many imports: 13 > 12
6     WPS210 Found too many local variables: 6 > 5
3     WPS220 Found too deep nesting: 24 > 20
2     WPS221 Found line with high Jones Complexity: 16 > 14
3     WPS226 Found string literal over-use: test_option > 3
9     WPS305 Found `f` string
1     WPS316 Found context manager with too many assignments
6     WPS317 Found incorrect multi-line parameters
2     WPS323 Found `%` string formatting
4     WPS324 Found inconsistent `return` statement
2     WPS414 Found incorrect unpacking target
1     WPS430 Found nested function: getcwd
14    WPS436 Found protected module import: _curses
3     WPS450 Found protected object import: _CursesWindow
1     WPS602 Found using `@staticmethod`

Related #713

@cidrblock cidrblock changed the title Disable warning related to the use of assert in the tests directory Disable warnings related to the use of assert in the tests directory as report with lint-vetting Jan 23, 2022
@cidrblock cidrblock requested review from a team, relrod, ssbarnea and priyamsahoo January 23, 2022 14:32
@cidrblock cidrblock self-assigned this Jan 23, 2022
@cidrblock cidrblock changed the title Disable warnings related to the use of assert in the tests directory as report with lint-vetting Disable warnings related to the use of assert in the tests directory as reported with lint-vetting Jan 23, 2022
@softwarefactory-project-zuul

This comment has been minimized.

@cidrblock

This comment has been minimized.

@softwarefactory-project-zuul

This comment has been minimized.

.flake8 Outdated Show resolved Hide resolved
.flake8 Show resolved Hide resolved
.flake8 Outdated Show resolved Hide resolved
.flake8 Outdated Show resolved Hide resolved
@softwarefactory-project-zuul

This comment has been minimized.

@softwarefactory-project-zuul

This comment has been minimized.

@cidrblock cidrblock marked this pull request as draft January 24, 2022 13:10
@softwarefactory-project-zuul

This comment has been minimized.

cidrblock and others added 6 commits January 24, 2022 09:16
Co-authored-by: Sviatoslav Sydorenko <[email protected]>
Co-authored-by: Sviatoslav Sydorenko <[email protected]>
Sort the entries in the flake8 `per_file_ignore` list

Sort the entries in the flake8 per_file_ingore list and add a note asking they are sorted in the future.
This should make it easier to find and add entries in the future in a predictable spot.
No changes were made to any line, just re-odered.

Reviewed-by: Sviatoslav Sydorenko <webknjaz+github/[email protected]>
Reviewed-by: None <None>
@cidrblock cidrblock marked this pull request as ready for review January 24, 2022 17:22
@cidrblock cidrblock requested a review from webknjaz January 24, 2022 17:24
@softwarefactory-project-zuul

This comment has been minimized.

@webknjaz
Copy link
Member

@cidrblock FYI in diffs there's usually some indentation in the unchanged lines.
It's (like in typical diff output)

  unchanged
- dropped
+ added
  lines

vs (your version)

unchanged
- dropped
+ added
lines

The idea is that all the text content has the same starting point and is aligned accordingly.
I know that it's inconvenient to do in GH comment inputs so I often produce such diffs externally to make it easier. It's possible to do by creating two text files before and after and feeding them to the diff like diff -u before after. Alternatively, text editors like Vim allow producing block indents. So I add two spaces to all the lines like this and then replace some of the spaces with -/+ and duplicate them where necessary.

@cidrblock cidrblock added the gate label Jan 25, 2022
Copy link
Contributor

@ansible-zuul ansible-zuul bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ansible-zuul ansible-zuul bot merged commit 4d97f97 into ansible:main Jan 25, 2022
ansible-zuul bot pushed a commit that referenced this pull request Jan 25, 2022
Move `WPSXXX` errors reported by `lint-vetting` into `per-file-ignores`

When running tox -e lint-vetting a number of errors are reported by the wemake python style guide.
This moves those errors into the per-file-ignore list within the flake8 configuration file.
No attempt was made to review or remediate the errors as WPS is just being vetted.
(S101 was also added to account for #792 where needed when a per-file addition was newly added)
This should make it easier for users of tox -e lint-vetting to identify errors from linters being vetted for the changes they are making without needing to parse all errors that previously existed in the code.
Follow up PRs should be expected to migrate some per file ignores into the extended ignore list)
The list of errors was
src/ansible_navigator/action_runner.py:17:5: WPS436 Found protected module import: _curses
src/ansible_navigator/action_runner.py:17:5: WPS450 Found protected object import: _CursesWindow
src/ansible_navigator/actions/_actions.py:0:1: WPS201 Found module with too many imports: 13 > 12
src/ansible_navigator/actions/_actions.py:105:22: WPS323 Found `%` string formatting
src/ansible_navigator/actions/_actions.py:98:1: WPS210 Found too many local variables: 6 > 5
src/ansible_navigator/actions/collections.py:335:5: WPS324 Found inconsistent `return` statement
src/ansible_navigator/actions/config.py:286:5: WPS324 Found inconsistent `return` statement
src/ansible_navigator/actions/exec.py:14:1: WPS450 Found protected object import: _actions
src/ansible_navigator/actions/exec.py:45:5: WPS115 Found upper-case constant in a class: KEGEX
src/ansible_navigator/actions/exec.py:66:5: WPS210 Found too many local variables: 6 > 5
src/ansible_navigator/actions/exec.py:83:17: WPS323 Found `%` string formatting
src/ansible_navigator/actions/open_file.py:104:25: WPS220 Found too deep nesting: 24 > 20
src/ansible_navigator/actions/open_file.py:56:5: WPS324 Found inconsistent `return` statement
src/ansible_navigator/actions/open_file.py:90:25: WPS220 Found too deep nesting: 24 > 20
src/ansible_navigator/actions/open_file.py:99:25: WPS220 Found too deep nesting: 24 > 20
src/ansible_navigator/actions/write_file.py:28:5: WPS324 Found inconsistent `return` statement
src/ansible_navigator/app.py:86:17: WPS305 Found `f` string
src/ansible_navigator/app.py:89:17: WPS305 Found `f` string
src/ansible_navigator/configuration_subsystem/navigator_post_processor.py:62:5: WPS115 Found upper-case constant in a class: Z
src/ansible_navigator/initialization.py:166:5: WPS414 Found incorrect unpacking target
src/ansible_navigator/initialization.py:166:5: WPS414 Found incorrect unpacking target
tests/integration/_action_run_test.py:144:9: WPS316 Found context manager with too many assignments
tests/integration/actions/exec/base.py:0:1: WPS201 Found module with too many imports: 13 > 12
tests/integration/actions/exec/base.py:100:23: WPS305 Found `f` string
tests/integration/actions/exec/base.py:111:56: WPS305 Found `f` string
tests/integration/actions/exec/base.py:14:1: WPS436 Found protected module import: _common
tests/integration/actions/exec/base.py:15:1: WPS436 Found protected module import: _common
tests/integration/actions/exec/base.py:16:1: WPS436 Found protected module import: _interactions
tests/integration/actions/exec/base.py:17:1: WPS436 Found protected module import: _interactions
tests/integration/actions/exec/base.py:18:1: WPS436 Found protected module import: _tmux_session
tests/integration/actions/exec/base.py:55:5: WPS210 Found too many local variables: 9 > 5
tests/integration/actions/exec/test_stdout_config_cli.py:5:1: WPS436 Found protected module import: _interactions
tests/integration/actions/exec/test_stdout_config_cli.py:6:1: WPS436 Found protected module import: _interactions
tests/integration/actions/exec/test_stdout_config_cli.py:71:12: WPS305 Found `f` string
tests/integration/actions/exec/test_stdout_config_cli.py:7:1: WPS436 Found protected module import: _interactions
tests/integration/actions/exec/test_stdout_config_cli.py:8:1: WPS436 Found protected module import: _interactions
tests/integration/actions/exec/test_stdout_config_file.py:5:1: WPS436 Found protected module import: _interactions
tests/integration/actions/exec/test_stdout_config_file.py:55:12: WPS305 Found `f` string
tests/integration/actions/exec/test_stdout_config_file.py:6:1: WPS436 Found protected module import: _interactions
tests/integration/actions/exec/test_stdout_config_file.py:7:1: WPS436 Found protected module import: _interactions
tests/integration/actions/exec/test_stdout_config_file.py:8:1: WPS436 Found protected module import: _interactions
tests/integration/actions/images/base.py:17:1: WPS221 Found line with high Jones Complexity: 16 > 14
tests/integration/actions/images/base.py:50:5: WPS602 Found using `@staticmethod`
tests/integration/actions/replay/base.py:41:5: WPS210 Found too many local variables: 6 > 5
tests/integration/actions/stdout/base.py:41:5: WPS210 Found too many local variables: 6 > 5
tests/unit/actions/test_exec.py:27:12: WPS305 Found `f` string
tests/unit/actions/test_exec.py:8:1: WPS450 Found protected object import: _generate_command
tests/unit/configuration_subsystem/test_internals.py:31:5: WPS430 Found nested function: getcwd
tests/unit/configuration_subsystem/test_navigator_post_processor.py:0:1: WPS226 Found string literal over-use: /bar > 3
tests/unit/configuration_subsystem/test_navigator_post_processor.py:0:1: WPS226 Found string literal over-use: /foo > 3
tests/unit/configuration_subsystem/test_navigator_post_processor.py:0:1: WPS226 Found string literal over-use: test_option > 3
tests/unit/configuration_subsystem/test_navigator_post_processor.py:13:2: WPS317 Found incorrect multi-line parameters
tests/unit/configuration_subsystem/test_precedence.py:146:9: WPS221 Found line with high Jones Complexity: 15 > 14
tests/unit/configuration_subsystem/test_previous_cli.py:0:1: WPS201 Found module with too many imports: 13 > 12
tests/unit/runner/test_api.py:9:2: WPS317 Found incorrect multi-line parameters
tests/unit/test_circular_imports.py:101:9: WPS305 Found `f` string
tests/unit/test_circular_imports.py:44:1: WPS210 Found too many local variables: 7 > 5
tests/unit/test_circular_imports.py:67:35: WPS317 Found incorrect multi-line parameters
tests/unit/test_circular_imports.py:69:24: WPS305 Found `f` string
tests/unit/test_cli.py:16:2: WPS317 Found incorrect multi-line parameters
tests/unit/test_utils.py:61:2: WPS317 Found incorrect multi-line parameters
tests/unit/test_utils.py:88:2: WPS317 Found incorrect multi-line parameters

and the output of tox -e lint-vetting after:
1     C812 missing trailing comma
1     RST304 Unknown interpreted text role "mod".
13    S101 Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
1     S404 Consider possible security implications associated with the subprocess module.
1     S603 subprocess call - check for execution of untrusted input.
2     S604 Function call with shell=True parameter identified, possible security issue.
- 2     WPS115 Found upper-case constant in a class: KEGEX
- 3     WPS201 Found module with too many imports: 13 > 12
- 6     WPS210 Found too many local variables: 6 > 5
- 3     WPS220 Found too deep nesting: 24 > 20
- 2     WPS221 Found line with high Jones Complexity: 16 > 14
- 3     WPS226 Found string literal over-use: test_option > 3
- 9     WPS305 Found `f` string
- 1     WPS316 Found context manager with too many assignments
- 6     WPS317 Found incorrect multi-line parameters
- 2     WPS323 Found `%` string formatting
- 4     WPS324 Found inconsistent `return` statement
- 2     WPS414 Found incorrect unpacking target
- 1     WPS430 Found nested function: getcwd
- 14    WPS436 Found protected module import: _curses
- 3     WPS450 Found protected object import: _CursesWindow
- 1     WPS602 Found using `@staticmethod`

Reviewed-by: Sviatoslav Sydorenko <webknjaz+github/[email protected]>
Reviewed-by: None <None>
ssbarnea pushed a commit to ssbarnea/ansible-navigator that referenced this pull request Feb 12, 2022
…as reported with `lint-vetting` (ansible#792)

Disable warnings related to the use of assert in the tests directory as reported with `lint-vetting`

This adds a few additional files to the per-file-ignore list within the flake8 configuration with an exemption for S101 which is reported by flake8-bandit as part of the wemake style guide which is being vetted.
The list was also alphabetized to ensure new entries were placed in a sane location.
A note was added to the top of the flake8 per-file-ignores indicating that the glob for the tests folder ignoring s101 can be added later. Currently there is a conflict between globs and per-file entries.  See PyCQA/flake8#670
This eliminates the S101 entry from the output of tox -e lint-vetting so users of it can see messages only related to the changes they are making and not for files that were previously added to the repository:
1     C812 missing trailing comma
1     RST304 Unknown interpreted text role "mod".
- 13    S101 Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
1     S404 Consider possible security implications associated with the subprocess module.
1     S603 subprocess call - check for execution of untrusted input.
2     S604 Function call with shell=True parameter identified, possible security issue.
2     WPS111 Found too short name: p < 2
2     WPS115 Found upper-case constant in a class: KEGEX
3     WPS201 Found module with too many imports: 13 > 12
6     WPS210 Found too many local variables: 6 > 5
3     WPS220 Found too deep nesting: 24 > 20
2     WPS221 Found line with high Jones Complexity: 16 > 14
3     WPS226 Found string literal over-use: test_option > 3
9     WPS305 Found `f` string
1     WPS316 Found context manager with too many assignments
6     WPS317 Found incorrect multi-line parameters
2     WPS323 Found `%` string formatting
4     WPS324 Found inconsistent `return` statement
2     WPS414 Found incorrect unpacking target
1     WPS430 Found nested function: getcwd
14    WPS436 Found protected module import: _curses
3     WPS450 Found protected object import: _CursesWindow
1     WPS602 Found using `@staticmethod`
Related ansible#713

Reviewed-by: Sviatoslav Sydorenko <webknjaz+github/[email protected]>
Reviewed-by: Bradley A. Thornton <[email protected]>
Reviewed-by: None <None>
ssbarnea pushed a commit to ssbarnea/ansible-navigator that referenced this pull request Feb 12, 2022
Move `WPSXXX` errors reported by `lint-vetting` into `per-file-ignores`

When running tox -e lint-vetting a number of errors are reported by the wemake python style guide.
This moves those errors into the per-file-ignore list within the flake8 configuration file.
No attempt was made to review or remediate the errors as WPS is just being vetted.
(S101 was also added to account for ansible#792 where needed when a per-file addition was newly added)
This should make it easier for users of tox -e lint-vetting to identify errors from linters being vetted for the changes they are making without needing to parse all errors that previously existed in the code.
Follow up PRs should be expected to migrate some per file ignores into the extended ignore list)
The list of errors was
src/ansible_navigator/action_runner.py:17:5: WPS436 Found protected module import: _curses
src/ansible_navigator/action_runner.py:17:5: WPS450 Found protected object import: _CursesWindow
src/ansible_navigator/actions/_actions.py:0:1: WPS201 Found module with too many imports: 13 > 12
src/ansible_navigator/actions/_actions.py:105:22: WPS323 Found `%` string formatting
src/ansible_navigator/actions/_actions.py:98:1: WPS210 Found too many local variables: 6 > 5
src/ansible_navigator/actions/collections.py:335:5: WPS324 Found inconsistent `return` statement
src/ansible_navigator/actions/config.py:286:5: WPS324 Found inconsistent `return` statement
src/ansible_navigator/actions/exec.py:14:1: WPS450 Found protected object import: _actions
src/ansible_navigator/actions/exec.py:45:5: WPS115 Found upper-case constant in a class: KEGEX
src/ansible_navigator/actions/exec.py:66:5: WPS210 Found too many local variables: 6 > 5
src/ansible_navigator/actions/exec.py:83:17: WPS323 Found `%` string formatting
src/ansible_navigator/actions/open_file.py:104:25: WPS220 Found too deep nesting: 24 > 20
src/ansible_navigator/actions/open_file.py:56:5: WPS324 Found inconsistent `return` statement
src/ansible_navigator/actions/open_file.py:90:25: WPS220 Found too deep nesting: 24 > 20
src/ansible_navigator/actions/open_file.py:99:25: WPS220 Found too deep nesting: 24 > 20
src/ansible_navigator/actions/write_file.py:28:5: WPS324 Found inconsistent `return` statement
src/ansible_navigator/app.py:86:17: WPS305 Found `f` string
src/ansible_navigator/app.py:89:17: WPS305 Found `f` string
src/ansible_navigator/configuration_subsystem/navigator_post_processor.py:62:5: WPS115 Found upper-case constant in a class: Z
src/ansible_navigator/initialization.py:166:5: WPS414 Found incorrect unpacking target
src/ansible_navigator/initialization.py:166:5: WPS414 Found incorrect unpacking target
tests/integration/_action_run_test.py:144:9: WPS316 Found context manager with too many assignments
tests/integration/actions/exec/base.py:0:1: WPS201 Found module with too many imports: 13 > 12
tests/integration/actions/exec/base.py:100:23: WPS305 Found `f` string
tests/integration/actions/exec/base.py:111:56: WPS305 Found `f` string
tests/integration/actions/exec/base.py:14:1: WPS436 Found protected module import: _common
tests/integration/actions/exec/base.py:15:1: WPS436 Found protected module import: _common
tests/integration/actions/exec/base.py:16:1: WPS436 Found protected module import: _interactions
tests/integration/actions/exec/base.py:17:1: WPS436 Found protected module import: _interactions
tests/integration/actions/exec/base.py:18:1: WPS436 Found protected module import: _tmux_session
tests/integration/actions/exec/base.py:55:5: WPS210 Found too many local variables: 9 > 5
tests/integration/actions/exec/test_stdout_config_cli.py:5:1: WPS436 Found protected module import: _interactions
tests/integration/actions/exec/test_stdout_config_cli.py:6:1: WPS436 Found protected module import: _interactions
tests/integration/actions/exec/test_stdout_config_cli.py:71:12: WPS305 Found `f` string
tests/integration/actions/exec/test_stdout_config_cli.py:7:1: WPS436 Found protected module import: _interactions
tests/integration/actions/exec/test_stdout_config_cli.py:8:1: WPS436 Found protected module import: _interactions
tests/integration/actions/exec/test_stdout_config_file.py:5:1: WPS436 Found protected module import: _interactions
tests/integration/actions/exec/test_stdout_config_file.py:55:12: WPS305 Found `f` string
tests/integration/actions/exec/test_stdout_config_file.py:6:1: WPS436 Found protected module import: _interactions
tests/integration/actions/exec/test_stdout_config_file.py:7:1: WPS436 Found protected module import: _interactions
tests/integration/actions/exec/test_stdout_config_file.py:8:1: WPS436 Found protected module import: _interactions
tests/integration/actions/images/base.py:17:1: WPS221 Found line with high Jones Complexity: 16 > 14
tests/integration/actions/images/base.py:50:5: WPS602 Found using `@staticmethod`
tests/integration/actions/replay/base.py:41:5: WPS210 Found too many local variables: 6 > 5
tests/integration/actions/stdout/base.py:41:5: WPS210 Found too many local variables: 6 > 5
tests/unit/actions/test_exec.py:27:12: WPS305 Found `f` string
tests/unit/actions/test_exec.py:8:1: WPS450 Found protected object import: _generate_command
tests/unit/configuration_subsystem/test_internals.py:31:5: WPS430 Found nested function: getcwd
tests/unit/configuration_subsystem/test_navigator_post_processor.py:0:1: WPS226 Found string literal over-use: /bar > 3
tests/unit/configuration_subsystem/test_navigator_post_processor.py:0:1: WPS226 Found string literal over-use: /foo > 3
tests/unit/configuration_subsystem/test_navigator_post_processor.py:0:1: WPS226 Found string literal over-use: test_option > 3
tests/unit/configuration_subsystem/test_navigator_post_processor.py:13:2: WPS317 Found incorrect multi-line parameters
tests/unit/configuration_subsystem/test_precedence.py:146:9: WPS221 Found line with high Jones Complexity: 15 > 14
tests/unit/configuration_subsystem/test_previous_cli.py:0:1: WPS201 Found module with too many imports: 13 > 12
tests/unit/runner/test_api.py:9:2: WPS317 Found incorrect multi-line parameters
tests/unit/test_circular_imports.py:101:9: WPS305 Found `f` string
tests/unit/test_circular_imports.py:44:1: WPS210 Found too many local variables: 7 > 5
tests/unit/test_circular_imports.py:67:35: WPS317 Found incorrect multi-line parameters
tests/unit/test_circular_imports.py:69:24: WPS305 Found `f` string
tests/unit/test_cli.py:16:2: WPS317 Found incorrect multi-line parameters
tests/unit/test_utils.py:61:2: WPS317 Found incorrect multi-line parameters
tests/unit/test_utils.py:88:2: WPS317 Found incorrect multi-line parameters

and the output of tox -e lint-vetting after:
1     C812 missing trailing comma
1     RST304 Unknown interpreted text role "mod".
13    S101 Use of assert detected. The enclosed code will be removed when compiling to optimised byte code.
1     S404 Consider possible security implications associated with the subprocess module.
1     S603 subprocess call - check for execution of untrusted input.
2     S604 Function call with shell=True parameter identified, possible security issue.
- 2     WPS115 Found upper-case constant in a class: KEGEX
- 3     WPS201 Found module with too many imports: 13 > 12
- 6     WPS210 Found too many local variables: 6 > 5
- 3     WPS220 Found too deep nesting: 24 > 20
- 2     WPS221 Found line with high Jones Complexity: 16 > 14
- 3     WPS226 Found string literal over-use: test_option > 3
- 9     WPS305 Found `f` string
- 1     WPS316 Found context manager with too many assignments
- 6     WPS317 Found incorrect multi-line parameters
- 2     WPS323 Found `%` string formatting
- 4     WPS324 Found inconsistent `return` statement
- 2     WPS414 Found incorrect unpacking target
- 1     WPS430 Found nested function: getcwd
- 14    WPS436 Found protected module import: _curses
- 3     WPS450 Found protected object import: _CursesWindow
- 1     WPS602 Found using `@staticmethod`

Reviewed-by: Sviatoslav Sydorenko <webknjaz+github/[email protected]>
Reviewed-by: None <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants