[testutil] Add proper test for the functional tests helpers

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)

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"

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]:

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]

tests/testutils/data/functional/broken_output_ok_test/exec_used.txt

@@ -0,0 +1 @@
+exec-used:UNDEFINED

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')

tests/testutils/data/functional/broken_output_wrong_test/exec_used.txt

@@ -0,0 +1 @@
+exec-used:UNDEFINED

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]

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')

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]

tests/testutils/data/functional/ok_output_ok_test/exec_used.txt

@@ -0,0 +1 @@
+exec-used:7:0:7:14::Use of exec:UNDEFINED

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')

tests/testutils/data/functional/ok_output_wrong_test/exec_used.txt

@@ -0,0 +1 @@
+exec-used:7:0:7:14::Use of exec:UNDEFINED

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]

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

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')

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

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:

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
+        )