Refactor precommit

CHANGELOG.md

@@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 ### Added
 ### Changed
+* Reworked file by file linting, improved the code and the tests. [#54](https://github.com/greenbone/autohooks-plugin-pylint/pull/54)
+
 ### Deprecated
 ### Removed
 ### Fixed

autohooks/plugins/pylint/pylint.py

@@ -1,4 +1,4 @@
-# Copyright (C) 2019 - 2020 Greenbone Networks GmbH
+# Copyright (C) 2019 - 2021 Greenbone Networks GmbH
 #
 # SPDX-License-Identifier: GPL-3.0-or-later
 #
@@ -75,6 +75,7 @@ def precommit(config=None, **kwargs):  # pylint: disable=unused-argument
     check_pylint_installed()
 
     include = get_include_from_config(config)
+
     files = [f for f in get_staged_status() if match(f.path, include)]
 
     if not files:
@@ -89,19 +90,17 @@ def precommit(config=None, **kwargs):  # pylint: disable=unused-argument
             cmd = ['pylint']
             cmd.extend(arguments)
             cmd.append(str(f.absolute_path()))
-            proc = subprocess.Popen(
-                cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE
-            )
-            out_, _ = proc.communicate()
-            if out_:
-                out_ = out_.decode(
+            try:
+                subprocess.run(cmd, check=True, capture_output=True)
+            except subprocess.CalledProcessError as e:
+                ret = e.returncode
+                error('Linting error(s) found in {}:'.format(str(f.path)))
+                lint_errors = e.stdout.decode(
                     encoding=sys.getdefaultencoding(), errors='replace'
                 ).split('\n')
-                for line in out_:
+                # Skip the first line that only shows ******** Module blah
+                for line in lint_errors[1:]:
                     out(line)
-            if proc.returncode:
-                ret = proc.returncode
-                error('Linting error(s) found in {}.'.format(str(f.path)))
             else:
                 ok('Linting {} was successful.'.format(str(f.path)))
 

pyproject.toml

@@ -1,7 +1,7 @@
 [build-system]
 requires = ["poetry>=0.12"]
 build-backend = "poetry.masonry.api"
-authors = ["Greenbone Networks <info@greenbone.net>"]
+authors = ["Greenbone Networks GmbH <info@greenbone.net>"]
 readme = "README.md"
 license = "AGPL-3.0-or-later"
 
@@ -44,13 +44,14 @@ keywords = [
 [tool.poetry.dependencies]
 python = "^3.7"
 pylint = "^2.5.3"
-black = {version = "20.8b1", python = "^3.6"}
 autohooks = ">=2.2.0"
 
 [tool.poetry.dev-dependencies]
-black = {version = "20.8b1", python = "^3.6"}
+black = "20.8b1"
 pylint = "^2.4.4"
-pontos = {version="^0.3.1", python = "^3.7"}
+pontos = "^0.3.1"
+autohooks-plugin-black = "^1.2.0"
+
 
 [tool.black]
 line-length = 80

tests/test_pylint.py

@@ -1,4 +1,4 @@
-# Copyright (C) 2020 Greenbone Networks GmbH
+# Copyright (C) 2020-2021 Greenbone Networks GmbH
 #
 # SPDX-License-Identifier: GPL-3.0-or-later
 #
@@ -46,9 +46,11 @@ def get_test_config_path(name):
 
 class AutohooksPylintTestCase(TestCase):
     def test_pylint_installed(self):
+        pylint = sys.modules['pylint']
         sys.modules['pylint'] = None
         with self.assertRaises(Exception):
             check_pylint_installed()
+        sys.modules['pylint'] = pylint
 
     def test_get_pylint_arguments(self):
         args = get_pylint_arguments(config=None)
@@ -78,18 +80,78 @@ def test_get_include_from_config(self):
         self.assertEqual(include, DEFAULT_INCLUDE)
 
     @patch('autohooks.plugins.pylint.pylint.ok')
-    def test_precommit(self, _ok_mock):
+    def test_precommit_no_files(self, _ok_mock):
         ret = precommit()
         self.assertFalse(ret)
 
     # these Terminal output functions don't run in the CI ...
+    # @patch('sys.stdout', new_callable=StringIO)
     @patch('autohooks.plugins.pylint.pylint.ok')
     @patch('autohooks.plugins.pylint.pylint.out')
     @patch('autohooks.plugins.pylint.pylint.error')
     @patch('autohooks.plugins.pylint.pylint.get_staged_status')
-    def test_precommit_staged(
-        self, staged_mock, _error_mock, _out_mock, _ok_mock
+    def test_precommit_errors(
+        self,
+        staged_mock,
+        _error_mock,
+        _out_mock,
+        _ok_mock,  # _mock_stdout
     ):
-        staged_mock.return_value = [StatusEntry('M  lint_test.py')]
+
+        code = (
+            "from io import StringIO, BytesIO, FileIO"
+            "import sys"
+            "cmd = ['pylint', 'autohooks/plugins/pylint/pylint.py']"
+            "import subprocess  # pylint: disable="
+            "# status = subprocess.call(cmd)"
+            "iofile = 'tmp.txt'"
+            "# status = subprocess.call(cmd, stdout=iofile)"
+            "# blah blah lots of code ..."
+            "status = subprocess.Popen("
+            "    cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)"
+            "out, err = status.communicate()"
+            "print(out.decode(encoding='utf-8'))"
+            "print(err.decode(encoding='utf-8'))"
+        )
+
+        test_file = Path(__file__).parent / "lint_test.py"
+        with open(test_file, 'a') as fp:
+            fp.writelines(code)
+
+        staged_mock.return_value = [
+            StatusEntry(
+                status_string='M  lint_test.py',
+                root_path=Path(__file__).parent,
+            )
+        ]
+
         ret = precommit()
+
+        # Returncode != 0 -> errors
         self.assertTrue(ret)
+        test_file.unlink()
+
+    # these Terminal output functions don't run in the CI ...
+    # @patch('sys.stdout', new_callable=StringIO)
+    @patch('autohooks.plugins.pylint.pylint.ok')
+    @patch('autohooks.plugins.pylint.pylint.out')
+    @patch('autohooks.plugins.pylint.pylint.error')
+    @patch('autohooks.plugins.pylint.pylint.get_staged_status')
+    def test_precommit_ok(
+        self,
+        staged_mock,
+        _error_mock,
+        _out_mock,
+        _ok_mock,  # _mock_stdout
+    ):
+        staged_mock.return_value = [
+            StatusEntry(
+                status_string='M  test_pylint.py',
+                root_path=Path(__file__).parent,
+            )
+        ]
+
+        ret = precommit()
+
+        # Returncode 0 -> no errors
+        self.assertFalse(ret)