Unify, document and fix functional tests internal updating process

pylint/testutils/__init__.py

@@ -39,11 +39,12 @@
     "MinimalTestReporter",
     "set_config",
     "GenericTestReporter",
+    "UPDATE_FILE",
     "UPDATE_OPTION",
 ]
 
 from pylint.testutils.checker_test_case import CheckerTestCase
-from pylint.testutils.constants import UPDATE_OPTION
+from pylint.testutils.constants import UPDATE_FILE, UPDATE_OPTION
 from pylint.testutils.decorator import set_config
 from pylint.testutils.functional_test_file import FunctionalTestFile
 from pylint.testutils.get_test_info import _get_tests_info

pylint/testutils/constants.py

@@ -5,11 +5,13 @@
 import re
 import sys
 from os.path import abspath, dirname
+from pathlib import Path
 
 SYS_VERS_STR = "%d%d%d" % sys.version_info[:3]
 TITLE_UNDERLINES = ["", "=", "-", "."]
 PREFIX = abspath(dirname(__file__))
 UPDATE_OPTION = "--update-functional-output"
+UPDATE_FILE = Path("pylint-functional-test-update")
 # Common sub-expressions.
 _MESSAGE = {"msg": r"[a-z][a-z\-]+"}
 # Matches a #,

tests/test_func.py

@@ -23,15 +23,14 @@
 
 import pytest
 
-from pylint.testutils import _get_tests_info, linter
+from pylint.testutils import UPDATE_FILE, UPDATE_OPTION, _get_tests_info, linter
 
 # Configure paths
 INPUT_DIR = join(dirname(abspath(__file__)), "input")
 MSG_DIR = join(dirname(abspath(__file__)), "messages")
 
 
 FILTER_RGX = None
-UPDATE = False
 INFO_TEST_RGX = re.compile(r"^func_i\d\d\d\d$")
 
 # Classes
@@ -50,7 +49,6 @@ class LintTestUsingModule:
     module = None
     depends = None
     output = None
-    _TEST_TYPE = "module"
 
     def _test_functionality(self):
         tocheck = [self.package + "." + self.module]
@@ -63,7 +61,15 @@ def _test_functionality(self):
         self._test(tocheck)
 
     def _check_result(self, got):
-        assert self._get_expected().strip() + "\n" == got.strip() + "\n"
+        error_msg = (
+            "Wrong output for '{_file}':\n"
+            "You can update the expected output automatically with: '"
+            "python tests/test_func.py {update_option}'\n\n".format(
+                update_option=UPDATE_OPTION,
+                _file=self.output,
+            )
+        )
+        assert self._get_expected() == got, error_msg
 
     def _test(self, tocheck):
         if INFO_TEST_RGX.match(self.module):
@@ -93,18 +99,16 @@ def _get_expected(self):
 
 
 class LintTestUpdate(LintTestUsingModule):
-
-    _TEST_TYPE = "update"
-
     def _check_result(self, got):
-        if self._has_output():
-            try:
-                expected = self._get_expected()
-            except OSError:
-                expected = ""
-            if got != expected:
-                with open(self.output, "w") as fobj:
-                    fobj.write(got)
+        if not self._has_output():
+            return
+        try:
+            expected = self._get_expected()
+        except OSError:
+            expected = ""
+        if got != expected:
+            with open(self.output, "w") as f:
+                f.write(got)
 
 
 def gen_tests(filter_rgx):
@@ -119,7 +123,7 @@ def gen_tests(filter_rgx):
         base = module_file.replace(".py", "").split("_")[1]
         dependencies = _get_tests_info(INPUT_DIR, MSG_DIR, base, ".py")
         tests.append((module_file, messages_file, dependencies))
-    if UPDATE:
+    if UPDATE_FILE.exists():
         return tests
     assert len(tests) < 196, "Please do not add new test cases here."
     return tests
@@ -149,7 +153,7 @@ def test_functionality(module_file, messages_file, dependencies, recwarn):
 
 
 def __test_functionality(module_file, messages_file, dependencies):
-    lint_test = LintTestUpdate() if UPDATE else LintTestUsingModule()
+    lint_test = LintTestUpdate() if UPDATE_FILE.exists() else LintTestUsingModule()
     lint_test.module = module_file.replace(".py", "")
     lint_test.output = messages_file
     lint_test.depends = dependencies or None
@@ -158,11 +162,14 @@ def __test_functionality(module_file, messages_file, dependencies):
 
 
 if __name__ == "__main__":
-    if "-u" in sys.argv:
-        UPDATE = True
-        sys.argv.remove("-u")
-
+    if UPDATE_OPTION in sys.argv:
+        UPDATE_FILE.touch()
+        sys.argv.remove(UPDATE_OPTION)
     if len(sys.argv) > 1:
         FILTER_RGX = sys.argv[1]
         del sys.argv[1]
-    pytest.main(sys.argv)
+    try:
+        pytest.main(sys.argv)
+    finally:
+        if UPDATE_FILE.exists():
+            UPDATE_FILE.unlink()

tests/test_functional.py

@@ -22,71 +22,39 @@
 """Functional full-module tests for PyLint."""
 
 import csv
-import io
 import os
 import sys
-import warnings
-from pathlib import Path
 
 import pytest
 
 from pylint import testutils
+from pylint.testutils import UPDATE_FILE, UPDATE_OPTION
 from pylint.utils import HAS_ISORT_5
 
-
-class test_dialect(csv.excel):
-    delimiter = ":"
-    lineterminator = "\n"
-
-
-csv.register_dialect("test", test_dialect)
-
-
 # Notes:
 # - for the purpose of this test, the confidence levels HIGH and UNDEFINED
 #   are treated as the same.
 
 # TODOs
 #  - implement exhaustivity tests
 
-UPDATE = Path("pylint-functional-test-update")
-
 
 class LintModuleOutputUpdate(testutils.LintModuleTest):
     """If message files should be updated instead of checked."""
 
-    def _open_expected_file(self):
-        try:
-            return super()._open_expected_file()
-        except OSError:
-            return io.StringIO()
-
-    @classmethod
-    def _split_lines(cls, expected_messages, lines):
-        emitted, omitted = [], []
-        for msg in lines:
-            if (msg[1], msg[0]) in expected_messages:
-                emitted.append(msg)
-            else:
-                omitted.append(msg)
-        return emitted, omitted
-
-    def _check_output_text(self, expected_messages, expected_output, actual_output):
-        if not expected_messages:
+    class TestDialect(csv.excel):
+        delimiter = ":"
+        lineterminator = "\n"
+
+    csv.register_dialect("test", TestDialect)
+
+    def _check_output_text(self, _, expected_output, actual_output):
+        if expected_output == actual_output:
             return
-        emitted, remaining = self._split_lines(expected_messages, expected_output)
-        if emitted != actual_output:
-            remaining.extend(actual_output)
-            remaining.sort(key=lambda m: (m[1], m[0], m[3]))
-            warnings.warn(
-                "Updated '{}' with the new content generated from '{}'".format(
-                    self._test_file.expected_output, self._test_file.base
-                )
-            )
-            with open(self._test_file.expected_output, "w") as fobj:
-                writer = csv.writer(fobj, dialect="test")
-                for line in remaining:
-                    writer.writerow(line.to_csv())
+        with open(self._test_file.expected_output, "w") as f:
+            writer = csv.writer(f, dialect="test")
+            for line in actual_output:
+                writer.writerow(line.to_csv())
 
 
 def get_tests():
@@ -115,13 +83,12 @@ def get_tests():
 
 @pytest.mark.parametrize("test_file", TESTS, ids=TESTS_NAMES)
 def test_functional(test_file, recwarn):
-    LintTest = (
-        LintModuleOutputUpdate(test_file)
-        if UPDATE.exists()
-        else testutils.LintModuleTest(test_file)
-    )
-    LintTest.setUp()
-    LintTest._runTest()
+    if UPDATE_FILE.exists():
+        lint_test = LintModuleOutputUpdate(test_file)
+    else:
+        lint_test = testutils.LintModuleTest(test_file)
+    lint_test.setUp()
+    lint_test._runTest()
     warning = None
     try:
         # Catch <unknown>:x: DeprecationWarning: invalid escape sequence
@@ -139,9 +106,11 @@ def test_functional(test_file, recwarn):
 
 
 if __name__ == "__main__":
-    if testutils.UPDATE_OPTION in sys.argv:
-        UPDATE.touch()
-        sys.argv.remove(testutils.UPDATE_OPTION)
-    pytest.main(sys.argv)
-    if UPDATE.exists():
-        UPDATE.unlink()
+    if UPDATE_OPTION in sys.argv:
+        UPDATE_FILE.touch()
+        sys.argv.remove(UPDATE_OPTION)
+    try:
+        pytest.main(sys.argv)
+    finally:
+        if UPDATE_FILE.exists():
+            UPDATE_FILE.unlink()