diff --git a/pylint/testutils/functional/find_functional_tests.py b/pylint/testutils/functional/find_functional_tests.py index 200cee7ec0..33d84e0f96 100644 --- a/pylint/testutils/functional/find_functional_tests.py +++ b/pylint/testutils/functional/find_functional_tests.py @@ -5,14 +5,16 @@ from __future__ import annotations import os +from collections.abc import Iterator from pathlib import Path from pylint.testutils.functional.test_file import FunctionalTestFile -REASONABLY_DISPLAYABLE_VERTICALLY = 48 -"""'Wet finger' number of files that are reasonable to display by an IDE.""" -SHOULD_BE_IN_THE_SAME_DIRECTORY = 5 -"""'Wet finger' as in 'in my settings there are precisely this many'.""" +REASONABLY_DISPLAYABLE_VERTICALLY = 49 +"""'Wet finger' number of files that are reasonable to display by an IDE. + +'Wet finger' as in 'in my settings there are precisely this many'. +""" IGNORED_PARENT_DIRS = { "deprecated_relative_import", @@ -32,11 +34,12 @@ def get_functional_test_files_from_directory( input_dir: Path | str, + max_file_per_directory: int = REASONABLY_DISPLAYABLE_VERTICALLY, ) -> list[FunctionalTestFile]: """Get all functional tests in the input_dir.""" suite = [] - _check_functional_tests_structure(Path(input_dir)) + _check_functional_tests_structure(Path(input_dir), max_file_per_directory) for dirpath, dirnames, filenames in os.walk(input_dir): if dirpath.endswith("__pycache__"): @@ -49,39 +52,88 @@ def get_functional_test_files_from_directory( return suite -def _check_functional_tests_structure(directory: Path) -> None: - """Check if test directories follow correct file/folder structure.""" - # Ignore underscored directories +def _check_functional_tests_structure( + directory: Path, max_file_per_directory: int +) -> None: + """Check if test directories follow correct file/folder structure. + + Ignore underscored directories or files. + """ if Path(directory).stem.startswith("_"): return files: set[Path] = set() dirs: set[Path] = set() + def _get_files_from_dir( + path: Path, violations: list[tuple[Path, int]] + ) -> 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("_") + ) + if len(files_without_leading_underscore) > max_file_per_directory: + violations.append((path, len(files_without_leading_underscore))) + return files_without_leading_underscore + + def walk(path: Path) -> Iterator[Path]: + violations: list[tuple[Path, int]] = [] + violations_msgs: set[str] = set() + parent_dir_files = _get_files_from_dir(path, violations) + error_msg = ( + "The following directory contains too many functional tests files:\n" + ) + for _file_or_dir in parent_dir_files: + if _file_or_dir.is_dir(): + _files = _get_files_from_dir(_file_or_dir, violations) + yield _file_or_dir.resolve() + try: + yield from walk(_file_or_dir) + except AssertionError as e: + violations_msgs.add(str(e).replace(error_msg, "")) + else: + yield _file_or_dir.resolve() + if violations or violations_msgs: + _msg = error_msg + for offending_file, number in violations: + _msg += f"- {offending_file}: {number} when the max is {max_file_per_directory}\n" + for error_msg in violations_msgs: + _msg += error_msg + raise AssertionError(_msg) + # Collect all sub-directories and files in directory - for file_or_dir in directory.iterdir(): - if file_or_dir.is_file(): - if file_or_dir.suffix == ".py" and not file_or_dir.stem.startswith("_"): - files.add(file_or_dir) - elif file_or_dir.is_dir(): + for file_or_dir in walk(directory): + if file_or_dir.is_dir(): dirs.add(file_or_dir) - _check_functional_tests_structure(file_or_dir) - - assert len(files) <= REASONABLY_DISPLAYABLE_VERTICALLY, ( - f"{directory} contains too many functional tests files " - + f"({len(files)} > {REASONABLY_DISPLAYABLE_VERTICALLY})." - ) + elif file_or_dir.suffix == ".py": + files.add(file_or_dir) + directory_does_not_exists: list[tuple[Path, Path]] = [] + misplaced_file: list[Path] = [] for file in files: possible_dir = file.parent / file.stem.split("_")[0] - assert not possible_dir.exists(), f"{file} should go in {possible_dir}." - + if possible_dir.exists(): + directory_does_not_exists.append((file, possible_dir)) # Exclude some directories as they follow a different structure if ( not len(file.parent.stem) == 1 # First letter sub-directories and file.parent.stem not in IGNORED_PARENT_DIRS and file.parent.parent.stem not in IGNORED_PARENT_PARENT_DIRS ): - assert file.stem.startswith( - file.parent.stem - ), f"{file} should not go in {file.parent}" + if not file.stem.startswith(file.parent.stem): + misplaced_file.append(file) + + if directory_does_not_exists or misplaced_file: + msg = "The following functional tests are disorganized:\n" + for file, possible_dir in directory_does_not_exists: + msg += ( + f"- In '{directory}', '{file.relative_to(directory)}' " + f"should go in '{possible_dir.relative_to(directory)}'\n" + ) + for file in misplaced_file: + msg += ( + f"- In '{directory}', {file.relative_to(directory)} should go in a directory" + f" that starts with the first letters" + f" of '{file.stem}' (not '{file.parent.stem}')\n" + ) + raise AssertionError(msg) diff --git a/pylint/testutils/lint_module_test.py b/pylint/testutils/lint_module_test.py index e139af12bc..e1036170ad 100644 --- a/pylint/testutils/lint_module_test.py +++ b/pylint/testutils/lint_module_test.py @@ -266,7 +266,7 @@ def error_msg_for_unequal_messages( expected_messages: MessageCounter, actual_output: list[OutputLine], ) -> str: - msg = [f'Wrong results for file "{self._test_file.base}":'] + msg = [f'Wrong message(s) raised for "{Path(self._test_file.source).name}":'] missing, unexpected = self.multiset_difference( expected_messages, actual_messages ) @@ -289,7 +289,7 @@ def error_msg_for_unequal_output( ) -> str: missing = set(expected_lines) - set(received_lines) unexpected = set(received_lines) - set(expected_lines) - error_msg = f"Wrong output for '{self._test_file.base}.txt':" + error_msg = f'Wrong output for "{Path(self._test_file.expected_output).name}":' sort_by_line_number = operator.attrgetter("lineno") if missing: error_msg += "\n- Missing lines:\n" diff --git a/pylint/testutils/output_line.py b/pylint/testutils/output_line.py index fb05c8775d..8158d514a0 100644 --- a/pylint/testutils/output_line.py +++ b/pylint/testutils/output_line.py @@ -13,7 +13,6 @@ from pylint.constants import PY38_PLUS from pylint.interfaces import UNDEFINED, Confidence from pylint.message.message import Message -from pylint.testutils.constants import UPDATE_OPTION _T = TypeVar("_T") @@ -139,13 +138,7 @@ def from_csv( ) raise IndexError except Exception: # pylint: disable=broad-except - warnings.warn( - "Expected 'msg-symbolic-name:42:27:MyClass.my_function:The message:" - f"CONFIDENCE' but we got '{':'.join(row)}'. Try updating the expected" - f" output with:\npython tests/test_functional.py {UPDATE_OPTION}", - UserWarning, - stacklevel=2, - ) + # We need this to not fail for the update script to work. return cls("", 0, 0, None, None, "", "", "") def to_csv(self) -> tuple[str, str, str, str, str, str, str, str]: diff --git a/tests/functional/r/regression/regression_property_slots_2439.py b/tests/functional/r/regression_02/regression_property_slots_2439.py similarity index 100% rename from tests/functional/r/regression/regression_property_slots_2439.py rename to tests/functional/r/regression_02/regression_property_slots_2439.py diff --git a/tests/testutils/data/functional/broken_output_ok_test/exec_used.py b/tests/testutils/data/functional/broken_output_ok_test/exec_used.py new file mode 100644 index 0000000000..73d629d95f --- /dev/null +++ b/tests/testutils/data/functional/broken_output_ok_test/exec_used.py @@ -0,0 +1,7 @@ +"""This is an example for a functional test with +- an output that is broken +- the functional test itself is right + +So it should be updated. +""" +exec('a = 42') # [exec-used] diff --git a/tests/testutils/data/functional/broken_output_ok_test/exec_used.txt b/tests/testutils/data/functional/broken_output_ok_test/exec_used.txt new file mode 100644 index 0000000000..d74b208934 --- /dev/null +++ b/tests/testutils/data/functional/broken_output_ok_test/exec_used.txt @@ -0,0 +1 @@ +exec-used:UNDEFINED diff --git a/tests/testutils/data/functional/broken_output_wrong_test/exec_used.py b/tests/testutils/data/functional/broken_output_wrong_test/exec_used.py new file mode 100644 index 0000000000..8b6c06a574 --- /dev/null +++ b/tests/testutils/data/functional/broken_output_wrong_test/exec_used.py @@ -0,0 +1,7 @@ +"""This is an example for a functional test with +- an output that is broken +- the functional test itself is wrong + +So it cannot be updated. +""" +exec('a = 42') diff --git a/tests/testutils/data/functional/broken_output_wrong_test/exec_used.txt b/tests/testutils/data/functional/broken_output_wrong_test/exec_used.txt new file mode 100644 index 0000000000..d74b208934 --- /dev/null +++ b/tests/testutils/data/functional/broken_output_wrong_test/exec_used.txt @@ -0,0 +1 @@ +exec-used:UNDEFINED diff --git a/tests/testutils/data/functional/no_output_ok_test/exec_used.py b/tests/testutils/data/functional/no_output_ok_test/exec_used.py new file mode 100644 index 0000000000..59d19fec0e --- /dev/null +++ b/tests/testutils/data/functional/no_output_ok_test/exec_used.py @@ -0,0 +1,7 @@ +"""This is an example for a functional test with +- an output that does not exist +- the functional test itself is right + +So it should be created +""" +exec('a = 42') # [exec-used] diff --git a/tests/testutils/data/functional/no_output_wrong_test/exec_used.py b/tests/testutils/data/functional/no_output_wrong_test/exec_used.py new file mode 100644 index 0000000000..d3b318ab7d --- /dev/null +++ b/tests/testutils/data/functional/no_output_wrong_test/exec_used.py @@ -0,0 +1,7 @@ +"""This is an example for a functional test with +- an output that does not exist +- the functional test itself is wrong + +So it cannot be created. +""" +exec('a = 42') diff --git a/tests/testutils/data/functional/ok_output_ok_test/exec_used.py b/tests/testutils/data/functional/ok_output_ok_test/exec_used.py new file mode 100644 index 0000000000..049213dd4c --- /dev/null +++ b/tests/testutils/data/functional/ok_output_ok_test/exec_used.py @@ -0,0 +1,7 @@ +"""This is an example for a functional test with +- an output that is right +- the functional test itself is right + +Nothing should be done. +""" +exec('a = 42') # [exec-used] diff --git a/tests/testutils/data/functional/ok_output_ok_test/exec_used.txt b/tests/testutils/data/functional/ok_output_ok_test/exec_used.txt new file mode 100644 index 0000000000..6d0f82baf1 --- /dev/null +++ b/tests/testutils/data/functional/ok_output_ok_test/exec_used.txt @@ -0,0 +1 @@ +exec-used:7:0:7:14::Use of exec:UNDEFINED diff --git a/tests/testutils/data/functional/ok_output_wrong_test/exec_used.py b/tests/testutils/data/functional/ok_output_wrong_test/exec_used.py new file mode 100644 index 0000000000..3f8637228f --- /dev/null +++ b/tests/testutils/data/functional/ok_output_wrong_test/exec_used.py @@ -0,0 +1,7 @@ +"""This is an example for a functional test with +- an output that is right +- the functional test itself is wrong + +Nothing should be done. +""" +exec('a = 42') diff --git a/tests/testutils/data/functional/ok_output_wrong_test/exec_used.txt b/tests/testutils/data/functional/ok_output_wrong_test/exec_used.txt new file mode 100644 index 0000000000..6d0f82baf1 --- /dev/null +++ b/tests/testutils/data/functional/ok_output_wrong_test/exec_used.txt @@ -0,0 +1 @@ +exec-used:7:0:7:14::Use of exec:UNDEFINED diff --git a/tests/testutils/data/functional/wrong_output_ok_test/exec_used.py b/tests/testutils/data/functional/wrong_output_ok_test/exec_used.py new file mode 100644 index 0000000000..0874f0a15f --- /dev/null +++ b/tests/testutils/data/functional/wrong_output_ok_test/exec_used.py @@ -0,0 +1,7 @@ +"""This is an example for a functional test with +- an output that is wrong (but not broken) +- the functional test itself is right + +So it needs to be updated. +""" +exec('a = 42') # [exec-used] diff --git a/tests/testutils/data/functional/wrong_output_ok_test/exec_used.txt b/tests/testutils/data/functional/wrong_output_ok_test/exec_used.txt new file mode 100644 index 0000000000..a737b8d51c --- /dev/null +++ b/tests/testutils/data/functional/wrong_output_ok_test/exec_used.txt @@ -0,0 +1 @@ +missing-docstring:5:0:1:1::Missing docstring in file:HIGH diff --git a/tests/testutils/data/functional/wrong_output_wrong_test/exec_used.py b/tests/testutils/data/functional/wrong_output_wrong_test/exec_used.py new file mode 100644 index 0000000000..fc35114e85 --- /dev/null +++ b/tests/testutils/data/functional/wrong_output_wrong_test/exec_used.py @@ -0,0 +1,7 @@ +"""This is an example for a functional test with +- an output that is wrong (but not broken) +- the functional test itself is wrong + +So it can't be updated. +""" +exec('a = 42') diff --git a/tests/testutils/data/functional/wrong_output_wrong_test/exec_used.txt b/tests/testutils/data/functional/wrong_output_wrong_test/exec_used.txt new file mode 100644 index 0000000000..a737b8d51c --- /dev/null +++ b/tests/testutils/data/functional/wrong_output_wrong_test/exec_used.txt @@ -0,0 +1 @@ +missing-docstring:5:0:1:1::Missing docstring in file:HIGH diff --git a/tests/testutils/data/m/max_overflow/max_overflow_1.py b/tests/testutils/data/m/max_overflow/max_overflow_1.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/testutils/data/m/max_overflow/max_overflow_2.py b/tests/testutils/data/m/max_overflow/max_overflow_2.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/testutils/data/m/max_overflow/max_overflow_3.py b/tests/testutils/data/m/max_overflow/max_overflow_3.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/testutils/data/u/_no_issue_here/_incredibly_bold_mischief.py b/tests/testutils/data/u/_no_issue_here/_incredibly_bold_mischief.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/testutils/test_functional_testutils.py b/tests/testutils/test_functional_testutils.py index 68dad697d4..d4a7a0ce1b 100644 --- a/tests/testutils/test_functional_testutils.py +++ b/tests/testutils/test_functional_testutils.py @@ -41,8 +41,32 @@ def test_parsing_of_pylintrc_init_hook() -> None: def test_get_functional_test_files_from_directory() -> None: """Test that we correctly check the functional test directory structures.""" - with pytest.raises(AssertionError, match="using_dir.py should not go in"): - get_functional_test_files_from_directory(DATA_DIRECTORY) + with pytest.raises(AssertionError) as exc_info: + get_functional_test_files_from_directory(DATA_DIRECTORY / "u") + assert exc_info.match("'use_dir.py' should go in 'use'") + assert exc_info.match( + "using_dir.py should go in a directory that starts with the " + "first letters of 'using_dir'" + ) + assert "incredibly_bold_mischief.py" not in str(exc_info.value) + # Leading underscore mean that this should not fail the assertion + get_functional_test_files_from_directory(DATA_DIRECTORY / "u/_no_issue_here") + + +def test_get_functional_test_files_from_crowded_directory() -> None: + """Test that we correctly check the functional test directory structures.""" + with pytest.raises(AssertionError) as exc_info: + 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") + with pytest.raises(AssertionError) as exc_info: + get_functional_test_files_from_directory( + DATA_DIRECTORY / "m", max_file_per_directory=3 + ) + assert exc_info.match("m: 4 when the max is 3") + assert "max_overflow" not in str(exc_info.value) def test_minimal_messages_config_enabled(pytest_config: MagicMock) -> None: diff --git a/tests/testutils/test_lint_module_output_update.py b/tests/testutils/test_lint_module_output_update.py index 2e387a118c..04c51d8841 100644 --- a/tests/testutils/test_lint_module_output_update.py +++ b/tests/testutils/test_lint_module_output_update.py @@ -6,15 +6,19 @@ from __future__ import annotations +import shutil from collections.abc import Callable from pathlib import Path import pytest -from pylint.constants import PY38_PLUS -from pylint.testutils import FunctionalTestFile +from pylint.constants import IS_PYPY, PY38_PLUS, PY39_PLUS +from pylint.testutils import FunctionalTestFile, LintModuleTest from pylint.testutils.functional import LintModuleOutputUpdate +FIXTURE_DIRECTORY = Path(__file__).parent / "data/functional" +DIRECTORIES = list(FIXTURE_DIRECTORY.iterdir()) + @pytest.fixture() def lint_module_fixture( @@ -76,4 +80,78 @@ def test_lint_module_output_update_remove_useless_txt( expected_output_file.write_text("", encoding="utf8") filename.write_text("", encoding="utf8") lmou.runTest() - assert not (expected_output_file).exists() + assert not expected_output_file.exists() + + +@pytest.mark.skipif( + not PY38_PLUS or (IS_PYPY and not PY39_PLUS), + reason="Requires accurate 'end_col' value to update output", +) +@pytest.mark.parametrize( + "directory_path", DIRECTORIES, ids=[str(p) for p in DIRECTORIES] +) +def test_update_of_functional_output(directory_path: Path, tmp_path: Path) -> None: + """Functional test for the functional tests' helper.""" + + def _check_expected_output(_ftf: FunctionalTestFile) -> None: + new_output_path = _ftf.expected_output + assert Path( + new_output_path + ).exists(), "The expected output file does not exists" + with open(new_output_path, encoding="utf8") as f: + new_output = f.read() + assert ( + new_output == "exec-used:7:0:7:14::Use of exec:UNDEFINED\n" + ), f"The content was wrongly updated in {new_output_path}" + + def _assert_behavior_is_correct( + _ftf: FunctionalTestFile, + _lint_module: LintModuleTest, + _lint_module_output_update: LintModuleOutputUpdate, + _new_path: Path, + ) -> None: + new_path_str = str(_new_path) + if "wrong_test" in new_path_str: + expected = r'Wrong message\(s\) raised for "exec_used.py"' + with pytest.raises(AssertionError, match=expected): + _lint_module.runTest() + # When the tests are wrong we do not update the output at all + # and the test should fail + with pytest.raises(AssertionError, match=expected): + _lint_module_output_update.runTest() + elif "ok_test" in new_path_str: + if any(f"{x}_output" in new_path_str for x in ("wrong", "no", "broken")): + with pytest.raises( + AssertionError, match='Wrong output for "exec_used.txt"' + ): + _lint_module.runTest() + elif "ok_output" in new_path_str: + _lint_module.runTest() + _check_expected_output(_ftf) + else: + raise AssertionError(f"Unhandled test case: {new_path_str}") + + # When the tests are ok we update the output whatever it's state + # was originally + _lint_module_output_update.runTest() + _check_expected_output(_ftf) + else: + raise AssertionError( + f"Do not pollute '{FIXTURE_DIRECTORY}' with unrelated " + f"or badly named test files." + ) + + new_path = tmp_path / directory_path.name + shutil.copytree(directory_path, new_path) + for filename in new_path.iterdir(): + if filename.suffix != ".py": + continue + ftf = FunctionalTestFile(directory=str(new_path), filename=filename.name) + # Standard functional test helper + lint_module = LintModuleTest(ftf) + # Functional test helper that automatically update the output + lint_module_output_update = LintModuleOutputUpdate(ftf) + + _assert_behavior_is_correct( + ftf, lint_module, lint_module_output_update, new_path + ) diff --git a/tests/testutils/test_output_line.py b/tests/testutils/test_output_line.py index 5b2bf1a1bb..a7e677157e 100644 --- a/tests/testutils/test_output_line.py +++ b/tests/testutils/test_output_line.py @@ -134,31 +134,6 @@ def test_output_line_to_csv(confidence: Confidence, message: _MessageCallable) - ) -def test_output_line_from_csv_error() -> None: - """Test that errors are correctly raised for incorrect OutputLine's.""" - # Test a csv-string which does not have a number for line and column - with pytest.warns( - UserWarning, - match="msg-symbolic-name:42:27:MyClass.my_function:The message", - ): - OutputLine.from_csv("'missing-docstring', 'line', 'column', 'obj', 'msg'", True) - # Test a tuple which does not have a number for line and column - with pytest.warns( - UserWarning, match="we got 'missing-docstring:line:column:obj:msg'" - ): - csv = ("missing-docstring", "line", "column", "obj", "msg") - OutputLine.from_csv(csv, True) - # Test a csv-string that is too long - with pytest.warns( - UserWarning, - match="msg-symbolic-name:42:27:MyClass.my_function:The message", - ): - OutputLine.from_csv( - "'missing-docstring', 1, 2, 'obj', 'msg', 'func', 'message', 'conf', 'too_long'", - True, - ) - - @pytest.mark.parametrize( "confidence,expected_confidence", [[None, "UNDEFINED"], ["INFERENCE", "INFERENCE"]] )