Skip to content

[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

Merged
merged 5 commits into from
Aug 15, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions pylint/testutils/functional/find_functional_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,19 @@
from pylint.testutils.functional.test_file import FunctionalTestFile

REASONABLY_DISPLAYABLE_VERTICALLY = 49
Copy link
Member

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.

Copy link
Member Author

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 ?

"""'Wet finger' number of files that are reasonable to display by an IDE.
"""'Wet finger' number of python files that are reasonable to have in a functional test
directory.

'Wet finger' as in 'in my settings there are precisely this many'.
Initially the total number of files then we started counting only the python files to
avoid moving a lot of files.
"""

IGNORED_PARENT_DIRS = {
"deprecated_relative_import",
"ext",
"regression",
"regression_02",
"used_02",
}
"""Direct parent directories that should be ignored."""

Expand Down Expand Up @@ -71,7 +73,9 @@ def _get_files_from_dir(
) -> list[Path]:
"""Return directories and files from a directory and handles violations."""
files_without_leading_underscore = list(
p for p in path.iterdir() if not p.stem.startswith("_")
p
for p in path.iterdir()
if not (p.stem.startswith("_") or (p.is_file() and p.suffix != ".py"))
)
if len(files_without_leading_underscore) > max_file_per_directory:
violations.append((path, len(files_without_leading_underscore)))
Expand All @@ -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)
Copy link
Member

@cdce8p cdce8p Aug 15, 2025

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

yield _file_or_dir.resolve()
try:
yield from walk(_file_or_dir)
Expand Down
Empty file.
9 changes: 5 additions & 4 deletions tests/testutils/test_functional_testutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,14 @@ def test_get_functional_test_files_from_crowded_directory() -> None:
get_functional_test_files_from_directory(
DATA_DIRECTORY / "m", max_file_per_directory=1
)
assert exc_info.match("m: 4 when the max is 1")
assert exc_info.match("max_overflow: 3 when the max is 1")
assert exc_info.match("m: 3 when the max is 1")
assert exc_info.match("max_overflow: 2 when the max is 1")
assert len(exc_info.value.args[0].splitlines()) == 3
with pytest.raises(AssertionError) as exc_info:
get_functional_test_files_from_directory(
DATA_DIRECTORY / "m", max_file_per_directory=3
DATA_DIRECTORY / "m", max_file_per_directory=2
)
assert exc_info.match("m: 4 when the max is 3")
assert exc_info.match("m: 3 when the max is 2")
assert "max_overflow" not in str(exc_info.value)


Expand Down
Loading