Merge pull request #122 from xenserver-next/add-type-annotation-updat…

…e-pre-commit

Add type annotation update pre commit

.pre-commit-config.yaml

@@ -1,11 +1,40 @@
-# Installation as pre-commit hook:
-# $ pip install pre-commit
+#
+# This is the configuration file of the pre-commit framework for this repository:
+# https://pypi.org/project/pre-commit
+#
+# pre-commit runs in the GitHub Workflow of this project on each push and PR.
+# Additionally, you can run it locally for faster fixing of issues using
+# $ pip3 install pre-commit -r requirements-dev.txt
+#
+# On the initial run, pre-commit downloads its checkers, subsequent runs are fast:
+#
+# $ pre-commit run    # automatically checks which checks are needed.
+# $ pre-commit run -a # runs all fixes and checks, even when not needed
+#
+# When this works, you can enable it as the pre-commit hook for this git clone:
 # $ pre-commit install
+# $ pre-commit install --hook-type pre-push
+#
 # Global installation as a git-template (for new clones):
 # $ git config --global init.templateDir ~/.git-template
 # $ pre-commit init-templatedir ~/.git-template
-# See: https://pre-commit.com/#automatically-enabling-pre-commit-on-repositories
-# All hooks: https://pre-commit.com/hooks.html
+#
+# You can skip checks if you commit very often you don't want them to run, e.g:
+# export SKIP=mypy,pylint;git commit -m "quick save" (or for --amend)
+#
+# For more customisation, see https://pre-commit.com/#temporarily-disabling-hooks
+# and https://pre-commit.com/#confining-hooks-to-run-at-certain-stages (e.g push)
+#
+# After this, the pre-commit fixes and checks run when you commit an update.
+#
+# Install pre-commit as a global pre-commit hook(for all repos with a config):
+# $ git config --global init.templateDir ~/.git-template
+# $ pre-commit init-templatedir ~/.git-template
+#
+# Detailed Usage information for all possible situations:
+# https://github.com/pre-commit/pre-commit.com/blob/main/sections/advanced.md
+# Cheat sheet on the config file keywords:
+# https://github.com/dexpota/cheatsheets/blob/master/pre-commit
 exclude: "^tests/data"
 fail_fast: true
 default_stages: [commit]
@@ -40,29 +69,31 @@ repos:
         pass_filenames: true
     -   id: shellcheck
     -   id: mdformat-check
+        exclude: README-Unicode.md
 -   repo: https://github.com/pycqa/pylint
     rev: v2.17.4
     hooks:
     -   id: pylint
         args:
             [
                 -sn, # Don't display the score
-                --rcfile=pylintrc,
                 --load-plugins=pylint.extensions.eq_without_hash,
                 --ignore-imports=yes,
-                "--disable=duplicate-code,superfluous-parens,line-too-long",
+                "--disable=duplicate-code,line-too-long",
             ]
         log_file: ".git/pre-commit-pylint.log"
         additional_dependencies:
         -   pyfakefs
         -   six
         -   mock
         -   pandas
+        -   pytest_forked
+        -   toml
 -   repo: local
     hooks:
     -   id: run-pyre
         name: run-pyre
-        entry: python run-pyre.py
+        entry: python pyre_runner.py
         types: [python]
         language: python
         log_file: ".git/pre-commit-pyre.log"

pylintrc → .pylintrc

@@ -6,7 +6,7 @@
 # Python code to execute, usually for sys.path manipulation such as
 # pygtk.require().
 # For absolute sys.path, use this for getting our stubs path dynamically:
-init-hook="from pylint.config import find_pylintrc; import os, sys; sys.path.append(os.path.dirname(find_pylintrc()) + '/stubs')"
+init-hook="import os,sys;sys.path.append(os.path.dirname('stubs'))"
 
 # Profiled execution.
 profile=no
@@ -50,7 +50,7 @@ load-plugins=
 #
 disable=W0142,W0703,C0111,R0201,W0603,W0613,W0212,W0141,
     bad-option-value,           # Skip complaining about suppressions for older option values
-    unrecognized-option,        # Skip complaining on pylintrc options only in pylint2/pylint3
+    unrecognized-option,        # Skip complaining on .pylintrc options only in pylint2/pylint3
     unknown-option-value,       # Skip complaining about checkers only in pylint2/pylint3
     useless-object-inheritance, # "object" is not obsolete for supporting Python2
     super-with-arguments,       # super() with arguments is a Python3-only feature
@@ -61,6 +61,7 @@ disable=W0142,W0703,C0111,R0201,W0603,W0613,W0212,W0141,
     consider-using-dict-items,
     consider-using-enumerate,
     consider-using-ternary,
+    duplicate-code,
     len-as-condition,
     no-else-return,
     raise-missing-from,

CONTRIBUTING.md

@@ -21,10 +21,10 @@ Installation of additional python versions for testing different versions:
 - If `deadsnakes/ppa` does not work, e.g. for Python 3.6, `conda` or `pyenv` can be used.
   For instructions, see https://realpython.com/intro-to-pyenv:
   ```yaml
-  sudo apt install -y build-essential xz-utils zlib1g-dev
+  sudo apt install -y build-essential xz-utils zlib1g-dev \
                       lib{bz2,ffi,lzma,readline,ssl,sqlite3}-dev
   curl https://pyenv.run | bash  # add displayed commands to .bashrc
-  pyenv install 3.{6,8,11} && pyenv local 3.{6,8,11} # builds them
+  ~/.pyenv/bin/pyenv install 3.{6,8,11} && ~/.pyenv/bin/pyenv local 3.{6,8,11} # builds them
   ```
 - For testing on newer Ubuntu which has `python2-dev`, but not `pip2`, install `pip2` this way:
   ```yml
@@ -37,7 +37,7 @@ You may want to install `pytype` in your user environment to run it directly wit
 # On Python != 3.8, pytype can't import xml.dom.minidom, use 3.8:
 python3.8 -m pip install pytype
 python -m pip install tabulate
-./run-pytype.py
+./pytype_runner.py
 ```
 
 ## Installation of dependencies using `pip`
@@ -79,8 +79,8 @@ Using pip-tools, you can extract the requirements and extras from `pyptoject.tom
 ```bash
 PYTHON=python3.10
 EXTRAS=.,test,mypy,pyre,pytype,tox
-PFLAGS="--no-warn-conflicts --resolver=backtracking"
-$PYTHON -m pip install pip-tools
+PFLAGS="--no-warn-conflicts"
+$PYTHON -m pip install pip-tools==7.3.0
 $PYTHON -m piptools compile --extra=$EXTRAS -o - pyproject.toml |
     $PYTHON -m pip install -r /dev/stdin $PFLAGS
 ```

Makefile

@@ -28,7 +28,7 @@ $(MY_OUTPUT_DIR)/python-libs.inc: $(MY_OUTPUT_DIR)/.dirstamp Makefile
 
 .PHONY: pylint
 pylint:
-	run-pylint.sh $(PYTHON_LIBS_SOURCES)
+	pylint_runner.sh $(PYTHON_LIBS_SOURCES)
 
 .PHONY: sources
 sources: $(RPM_SRPMSDIR)/$(PYTHON_LIB_SRPM)

README.md

@@ -27,13 +27,13 @@ The Continuous Integration Tests feature:
 - Automatic Upload of the combined coverage to CodeCov (from the GitHub Workflow)
 - Checking of the combined coverage against the diff to master: Fails if changes are not covered!
 - Pylint report in the GitHub Action Summary page, with Warning and Error annotatios, even in the code review.
-- Check that changes don't generate pylint warnings (if warning classes which are enabled in pylintrc)
+- Check that changes don't generate pylint warnings (if warning classes which are enabled in .pylintrc)
 - Static analysis using `mypy`, `pyre` and `pytype`
 
 This enforces that any change (besides whitespace):
 
 - has code coverage and
-- does not introduce a `pylint` warning which is not disabled in `pylintrc`
+- does not introduce a `pylint` warning which is not disabled in `.pylintrc`
 - does not introduce a type of static analysis warning which is currently suppressed.
 
 ## Status Summary
@@ -54,7 +54,7 @@ open a recent workflow run the latest and scroll down until you see the tables!
 - `pytest.ini`: The defaults used by `pytest` unless overruled by command line options
 - `.github/workflows/main.yml`: Configuration of the GitHub CI matrix jobs and coverage upload
 - `.github/act-serial.yaml`: Configuration for the jobs run by the local GitHub actions runner `act`
-- `pylintrc`: Configuration file of `Pylint`
+- `.pylintrc`: Configuration file of `Pylint`
 
 ## Installation and Setup of a development environment
 
@@ -71,8 +71,8 @@ For the installation of the general development dependencies, visit [INSTALL.md]
   issues by Python3.6
 - Test with `python2.7 -m pytest`
 - Run `mypy` (without any arguments - The configuration is in `pyproject.toml`)
-- Run `./run-pytype.py`
-- Run `./run-pyre.py`
+- Run `./pytype_runner.py`
+- Run `./pyre_runner.py`
 - Run `tox -e py36-lint` and fix any `Pylint` warnings
 - Run `tox -e py310-covcombine-check` and fix any missing diff-coverage.
 - Run `tox` for the full CI test suite

run-pylint.py → pylint_runner.py

@@ -32,12 +32,11 @@
 from io import StringIO
 from typing import List, TextIO
 
+import pandas as pd  # type: ignore[import]
 from pylint.lint import Run  # type: ignore[import]
 from pylint.reporters import JSONReporter  # type: ignore[import]
 from toml import load
 
-import pandas as pd  # type: ignore[import]
-
 
 def del_dict_keys(r, *args):
     for arg in args:
@@ -108,11 +107,11 @@ def cleanup_results_dict(r, sym):
 # https://pylint.readthedocs.io/en/latest/user_guide/messages/warning/eq-without-hash.html
 #
 pylint_options: List[str] = [
-    "--load-plugins", "pylint.extensions.eq_without_hash",
+    "--load-plugins", "pylint.extensions.eq_without_hash"
 ]
 
-def pylint_project(check_dirs: List[str], errorlog: TextIO, branch_url: str):
 
+def pylint_project(check_dirs: List[str], errorlog: TextIO, branch_url: str):
     pylint_overview = []
     pylint_results = []
     pylint_paths = []
@@ -125,13 +124,13 @@ def pylint_project(check_dirs: List[str], errorlog: TextIO, branch_url: str):
     smells_total = 0
     for path in pylint_paths:
         filename = path.rsplit("/", maxsplit=1)[-1]
-        if filename in ["__init__.py", "pylintrc"]:
+        if filename in ["__init__.py", ".pylintrc"]:
             continue
         reporter_buffer = StringIO()
         results = Run(
             [path] + pylint_options,
             reporter=JSONReporter(reporter_buffer),
-            do_exit=False,
+            exit=False,
         )
         score = results.linter.stats.global_note
         file_results = json.loads(reporter_buffer.getvalue())
@@ -169,11 +168,11 @@ def pylint_project(check_dirs: List[str], errorlog: TextIO, branch_url: str):
 
             message_ids[sym] = ""  # Populate a dict of the pylint message symbols seen in this file
             # https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-a-notice-message
-            endline = r['endLine']
+            endline = r["endLine"]
             end = f",endLine={endline}" if endline and endline != lineno else ""
             print(
                 f"::{cls} file={path},line={lineno}{end},"
-                f"title=pylint {msg_id}: {sym}::{msg}"
+                f"title=pylint {msg_id}: {sym}::{msg}",
             )
             r["path"] = f"[{linktext}]({branch_url}/{path}#L{lineno})"
             cleanup_results_dict(r, sym)

pyproject.toml

@@ -108,7 +108,7 @@ build-backend = "setuptools.build_meta"
 packages = ["xcp", "xcp.net", "xcp.net.ifrename"]
 
 [tool.black]
-line-length = 100
+line-length = 96
 
 [tool.mypy]
 pretty = true

run-pytype.py → pytype_runner.py

@@ -137,6 +137,7 @@ def run_pytype_and_parse_annotations(xfail_files: List[str], branch_url: str):
             print("No errors in", xfail_file)
     return err_code or len(results), results
 
+
 def to_markdown(me, fp, returncode, results, branch_url):
     mylink = f"[{me}]({branch_url}/{me}.py)"
     pytype_link = "[pytype](https://google.github.io/pytype)"
@@ -162,7 +163,7 @@ def setup_and_run_pytype_action(scriptname: str):
         branch = os.environ.get("GITHUB_HEAD_REF", None) or os.environ.get("GITHUB_REF_NAME", None)
         filelink_baseurl = f"{server_url}/{repository}/blob/{branch}"
     retcode, results = run_pytype_and_parse_annotations(xfail_files, filelink_baseurl)
-        # Write the panda dable to a markdown output file:
+    # Write the panda dable to a markdown output file:
     summary_file = os.environ.get("GITHUB_STEP_SUMMARY", None)
     if summary_file:
         with open(summary_file, "w", encoding="utf-8") as fp:
@@ -174,4 +175,4 @@ def setup_and_run_pytype_action(scriptname: str):
 if __name__ == "__main__":
     script_basename = os.path.basename(__file__).split(".")[0]
     basicConfig(format=script_basename + ": %(message)s", level=INFO)
-    sys.exit(setup_and_run_pytype_action(scriptname = script_basename))
+    sys.exit(setup_and_run_pytype_action(scriptname=script_basename))

tests/conftest.py

@@ -6,9 +6,10 @@
 """
 
 # pyre-ignore-all-errors[21]
-import pytest  # pyre does not find the module when run by tox -e py311-pyre
 import warnings
 
+import pytest  # pyre does not find the module when run by tox -e py311-pyre
+
 @pytest.fixture(autouse=True)
 def set_warnings():
     """

tests/test_accessor.py

@@ -11,6 +11,7 @@ def test_file_accessor(fs):
     # type:(FakeFilesystem) -> None
     """Test FileAccessor.writeFile(), .openAddress and .access using pyfakefs"""
     accessor = xcp.accessor.createAccessor("file://repo/", False)
+    assert isinstance(accessor, xcp.accessor.FileAccessor)
     check_binary_read(accessor, "/repo", fs)
     check_binary_write(accessor, "/repo", fs)
 

tests/test_cpiofile.py

@@ -30,9 +30,10 @@ def test_cpiofile_modes(fs):
     assert exc_info.type == TypeError
     if sys.version_info < (3, 0):
         # Test Python2 pattern from host-upgrade-plugin:/etc/xapi.d/plugins/prepare_host_upgrade.py
-        import StringIO
+        # Import the Python2-only StringIO.StringIO module, imported as Py2StringIO:
+        from StringIO import Py2StringIO  # pylint: disable=import-outside-toplevel
 
-        stringio = StringIO.StringIO()
+        stringio = Py2StringIO()
         archive = CpioFile.open(fileobj=stringio, mode="w|gz")  # type: ignore[arg-type]
         archive.hardlinks = False
         archive.close()

tests/test_httpaccessor.py

@@ -3,15 +3,16 @@
 import base64
 import sys
 from contextlib import contextmanager
-from typing import IO, Generator, Tuple
+from io import BufferedReader, TextIOWrapper
+from typing import Generator, Tuple
 
 from six.moves import urllib  # pyright: ignore
 
-from xcp.accessor import HTTPAccessor, createAccessor
+from xcp.accessor import createAccessor, HTTPAccessor
 
 from .httpserver_testcase import ErrorHandler, HTTPServerTestCase, Response
 
-HTTPAccessorGenerator = Generator[Tuple[HTTPAccessor, IO[bytes]], None, None]
+HTTPAccessorGenerator = Generator[Tuple[HTTPAccessor, BufferedReader], None, None]
 
 UTF8TEXT_LITERAL = "✋Hello accessor from the 🗺, download and verify me! ✅"
 
@@ -22,6 +23,7 @@ class HTTPAccessorTestCase(HTTPServerTestCase):
     def test_404(self):
         self.httpserver.expect_request("/404").respond_with_data("", status=404)
         httpaccessor = createAccessor(self.httpserver.url_for("/"), True)
+        assert isinstance(httpaccessor, HTTPAccessor)
         self.assertFalse(httpaccessor.access("404"))
         self.httpserver.check_assertions()
         self.assertEqual(httpaccessor.lastError, 404)
@@ -33,7 +35,7 @@ def http_get_request_data(self, url, read_file, error_handler):
         self.serve_file(self.document_root, read_file, error_handler)
 
         httpaccessor = createAccessor(url, True)
-        self.assertEqual(type(httpaccessor), HTTPAccessor)
+        assert isinstance(httpaccessor, HTTPAccessor)
 
         with open(self.document_root + read_file, "rb") as ref:
             yield httpaccessor, ref
@@ -98,5 +100,7 @@ def test_httpaccessor_open_text(self):
         """Get text containing UTF-8 and compare the returned decoded string contents"""
         self.httpserver.expect_request("/textfile").respond_with_data(UTF8TEXT_LITERAL)
         accessor = createAccessor(self.httpserver.url_for("/"), True)
+        assert isinstance(accessor, HTTPAccessor)
         with accessor.openText("textfile") as textfile:
+            assert isinstance(textfile, TextIOWrapper)
             assert textfile.read() == UTF8TEXT_LITERAL

tests/test_ifrename_dynamic.py

@@ -20,9 +20,8 @@ def setUp(self):
         openLog(self.logbuf, logging.NOTSET)
 
     def tearDown(self):
-
-        closeLogs()
         self.logbuf.close()
+        closeLogs()
 
     def test_null(self):