-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[functional tests] Only count python files for the reasonably displayable limit #10490
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
[functional tests] Only count python files for the reasonably displayable limit #10490
Conversation
…nably displayable limit
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
5784fd7
to
ae1e433
Compare
@@ -11,17 +11,19 @@ | |||
from pylint.testutils.functional.test_file import FunctionalTestFile | |||
|
|||
REASONABLY_DISPLAYABLE_VERTICALLY = 49 |
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.
Should we adjust the limit here? If I set it to 20
, this is the output
E AssertionError: The following directory contains too many functional tests files:
E - /.../pylint/tests/functional: 21 when the max is 20
E - /.../pylint/tests/functional/r/regression_02: 33 when the max is 20
E - /.../pylint/tests/functional/r/regression: 37 when the max is 20
E - /.../pylint/tests/functional/n: 26 when the max is 20
E - /.../pylint/tests/functional/n/no: 22 when the max is 20
E - /.../pylint/tests/functional/b: 22 when the max is 20
E - /.../pylint/tests/functional/ext: 26 when the max is 20
E - /.../pylint/tests/functional/u: 27 when the max is 20
E - /.../pylint/tests/functional/u/unused: 24 when the max is 20
E - /.../pylint/tests/functional/u/used: 26 when the max is 20
E - /.../pylint/tests/functional/t/too: 22 when the max is 20
E - /.../pylint/tests/functional/c: 23 when the max is 20
E - /.../pylint/tests/functional/w: 24 when the max is 20
E - /.../pylint/tests/functional/i: 21 when the max is 20
E - /.../pylint/tests/functional/i/invalid: 24 when the max is 20
E - /.../pylint/tests/functional/s: 26 when the max is 20
Good to see that u/used
now only counts as 26
when it hit the limit before.
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.
My ide now handle 33 file names vertically now, if you want a new arbitrary number that will prevent having too much work to do right now 😄 (but it doesn't make much sense to base the value on that now). Round number like 40 maybe ?
@@ -86,7 +90,6 @@ def walk(path: Path) -> Iterator[Path]: | |||
) | |||
for _file_or_dir in parent_dir_files: | |||
if _file_or_dir.is_dir(): | |||
_files = _get_files_from_dir(_file_or_dir, violations) |
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.
_files
here isn't used anywhere. Instead this line lead to a duplicate warning for subpackages which are walked anyway a few lines below. Example output from one of the tests:
The following directory contains too many functional tests files:
- /.../pylint/tests/testutils/data/m: 3 when the max is 1
- /.../pylint/tests/testutils/data/m/max_overflow: 2 when the max is 1
- /.../pylint/tests/testutils/data/m/max_overflow: 2 when the max is 1
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #10490 +/- ##
==========================================
- Coverage 95.85% 95.85% -0.01%
==========================================
Files 177 177
Lines 19199 19198 -1
==========================================
- Hits 18403 18402 -1
Misses 796 796
🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
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.
Thank you for the fixes !
🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉 This comment was generated for commit 15a3527 |
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.
Feel free to merge it if it looks good to you.
Type of Changes
Description
Refs #10488