Merge pull request #116 from rwe/test-pylint

test + pylint: enable pylint in tests/

gitrevise/merge.py

@@ -23,7 +23,7 @@
 from .utils import edit_file
 
 
-T = TypeVar("T")  # pylint: disable=C0103
+T = TypeVar("T")  # pylint: disable=invalid-name
 
 
 class MergeConflict(Exception):

gitrevise/utils.py

@@ -1,7 +1,7 @@
 from __future__ import annotations
 
-from typing import Any, List, Optional, Sequence, Tuple
-from subprocess import CompletedProcess, run, CalledProcessError
+from typing import Any, List, Optional, Sequence, Tuple, TYPE_CHECKING
+from subprocess import run, CalledProcessError
 from pathlib import Path
 import textwrap
 import sys
@@ -11,6 +11,10 @@
 from .odb import Repository, Commit, Tree, Oid, Reference
 
 
+if TYPE_CHECKING:
+    from subprocess import CompletedProcess
+
+
 class EditorError(Exception):
     pass
 
@@ -308,7 +312,7 @@ def sh_run(
     cmd: Sequence[Any],
     *args: Any,
     **kwargs: Any,
-) -> CompletedProcess[Any]:
+) -> "CompletedProcess[Any]":
     """Run a command within git's shell environment. This is the same as
     subprocess.run on most platforms, but will enter the git-bash mingw
     environment on Windows."""

pylintrc

@@ -1,3 +1,11 @@
+[MASTER]
+# Make sure things like "import gitrevise.odb" are resolved to the source
+# when running pylint from the command line or IDE.
+init-hook=
+  import os, sys, pylint.config as plc
+  sys.path.append(os.path.dirname(plc.PYLINTRC))
+
+
 [MESSAGES CONTROL]
 
 disable=
@@ -14,3 +22,13 @@ disable=
     # Currently broken analyses which are also handled (better) by mypy
     class-variable-slots-conflict,
     no-member
+
+good-names=
+  # "Exception as e" is perfectly fine.
+  e,
+  # "with open(…) as f" is idiomatic.
+  f,
+  # Other contextually-unambiguous names.
+  fn,
+  repo,
+  ed

tests/conftest.py

@@ -1,14 +1,15 @@
-# pylint: skip-file
-
-import pytest
-import shlex
 import os
-import py.path
+import shlex
+import subprocess
 import sys
 import tempfile
 import textwrap
-import subprocess
 import traceback
+
+from contextlib import contextmanager
+from http.server import HTTPServer, BaseHTTPRequestHandler
+from pathlib import Path
+from threading import Thread, Event
 from types import TracebackType
 from typing import (
     Any,
@@ -21,20 +22,20 @@
     Union,
     TYPE_CHECKING,
 )
+
+import pytest
+
 from gitrevise.odb import Repository
 from gitrevise.utils import sh_path
-from contextlib import contextmanager
-from threading import Thread, Event
-from http.server import HTTPServer, BaseHTTPRequestHandler
-import dummy_editor
+from . import dummy_editor
 
 
 if TYPE_CHECKING:
     from _typeshed import StrPath
 
 
-@pytest.fixture(autouse=True)
-def hermetic_seal(
+@pytest.fixture(name="hermetic_seal", autouse=True)
+def fixture_hermetic_seal(
     tmp_path_factory: pytest.TempPathFactory,
     monkeypatch: pytest.MonkeyPatch,
 ) -> None:
@@ -77,16 +78,17 @@ def hermetic_seal(
     bash("git init -q")
 
 
-@pytest.fixture
-def repo(hermetic_seal: None) -> Generator[Repository, None, None]:
+@pytest.fixture(name="repo")
+# pylint: disable=unused-argument
+def fixture_repo(hermetic_seal: None) -> Generator[Repository, None, None]:
     with Repository() as repo:
         yield repo
 
 
-@pytest.fixture
-def short_tmpdir() -> Generator[py.path.local, None, None]:
+@pytest.fixture(name="short_tmpdir")
+def fixture_short_tmpdir() -> Generator[Path, None, None]:
     with tempfile.TemporaryDirectory() as tdir:
-        yield py.path.local(tdir)
+        yield Path(tdir)
 
 
 @contextmanager
@@ -141,6 +143,7 @@ def changeline(path: "StrPath", lineno: int, newline: bytes) -> None:
 def main(
     args: Sequence[str],
     cwd: Optional["StrPath"] = None,
+    # pylint: disable=redefined-builtin
     input: Optional[bytes] = None,
     check: bool = True,
 ) -> "subprocess.CompletedProcess[bytes]":
@@ -153,23 +156,25 @@ def main(
 def editor_main(
     args: Sequence[str],
     cwd: Optional["StrPath"] = None,
+    # pylint: disable=redefined-builtin
     input: Optional[bytes] = None,
 ) -> "Generator[Editor, None, None]":
-    with pytest.MonkeyPatch().context() as m, Editor() as ed:
+    with pytest.MonkeyPatch().context() as monkeypatch, Editor() as ed:
+        host, port = ed.server_address
         editor_cmd = " ".join(
             shlex.quote(p)
             for p in (
                 sys.executable,
                 dummy_editor.__file__,
-                "http://{0}:{1}/".format(*ed.server_address[:2]),
+                f"http://{host}:{port}/",
             )
         )
-        m.setenv("GIT_EDITOR", editor_cmd)
+        monkeypatch.setenv("GIT_EDITOR", editor_cmd)
 
         def main_wrapper() -> Optional["subprocess.CompletedProcess[bytes]"]:
             try:
                 return main(args, cwd=cwd, input=input)
-            except Exception as e:
+            except Exception as e:  # pylint: disable=broad-except
                 ed.exception = e
                 return None
             finally:
@@ -200,6 +205,7 @@ def __init__(
         self.exception = None
         super().__init__(request=request, client_address=client_address, server=server)
 
+    # pylint: disable=invalid-name
     def do_POST(self) -> None:
         length = int(self.headers.get("content-length"))
         self.indata = self.rfile.read(length)
@@ -247,6 +253,10 @@ def replace_dedent(self, text: Union[str, bytes]) -> None:
             text = textwrap.dedent(text).encode()
         self.outdata = text
 
+    # pylint does not recognize these, for some reason, complaining:
+    # E1129: Context manager 'NoneType' doesn't implement __enter__ and
+    # __exit__. (not-context-manager) I suspect it gets looped up when tracing
+    # types due to the mutual touching between it and Editor.
     def __enter__(self) -> "EditorFile":
         return self
 
@@ -263,7 +273,7 @@ def __exit__(
             exc = "".join(traceback.format_exception(etype, evalue, tb)).encode()
             try:
                 self.send_editor_reply(500, exc)
-            except:
+            except:  # pylint: disable=bare-except
                 pass
 
     def __repr__(self) -> str:
@@ -310,6 +320,8 @@ def is_idle(self) -> bool:
     def __enter__(self) -> "Editor":
         return self
 
+    # The super class just defines this as *args.
+    # pylint: disable=arguments-differ
     def __exit__(
         self,
         etype: Optional[Type[BaseException]],
@@ -324,3 +336,12 @@ def __exit__(
             self.server_close()
             if self.current:
                 self.current.send_editor_reply(500, b"editor server was shut down")
+
+
+__all__ = (
+    "bash",
+    "changeline",
+    "editor_main",
+    "main",
+    "Editor",
+)

tests/dummy_editor.py

@@ -4,11 +4,10 @@
 
 
 def run_editor(path: Path, url: str) -> None:
-    # pylint: disable=invalid-name
-    with urlopen(url, data=path.read_bytes(), timeout=10) as r:
-        length = int(r.headers.get("content-length"))
-        data = r.read(length)
-        if r.status != 200:
+    with urlopen(url, data=path.read_bytes(), timeout=10) as request:
+        length = int(request.headers.get("content-length"))
+        data = request.read(length)
+        if request.status != 200:
             raise Exception(data.decode())
     path.write_bytes(data)
 

tests/test_cut.py

@@ -1,6 +1,7 @@
-# pylint: skip-file
+# pylint: disable=not-context-manager
 
-from conftest import *
+from gitrevise.odb import Repository
+from .conftest import bash, editor_main
 
 
 def test_cut(repo: Repository) -> None:

tests/test_fixup.py

@@ -1,13 +1,21 @@
-# pylint: skip-file
+# pylint: disable=not-context-manager
 
-from conftest import *
+import os
+from contextlib import contextmanager
+from typing import (
+    Generator,
+    Optional,
+    Sequence,
+)
+import pytest
+from gitrevise.odb import Repository
 from gitrevise.utils import commit_range
 from gitrevise.todo import StepKind, build_todos, autosquash_todos
-import os
+from .conftest import bash, main, editor_main, Editor
 
 
-@pytest.fixture
-def basic_repo(repo: Repository) -> Repository:
+@pytest.fixture(name="basic_repo")
+def fixture_basic_repo(repo: Repository) -> Repository:
     bash(
         """
         cat <<EOF >file1
@@ -337,7 +345,7 @@ def test_fixup_order_transitive(repo: Repository) -> None:
     assert tip.persisted
 
     todos = build_todos(commit_range(old, tip), index=None)
-    [target, a, b, c] = autosquash_todos(todos)
+    [target, a, b, c] = autosquash_todos(todos)  # pylint: disable=invalid-name
 
     assert b"target commit" in target.commit.message
     assert b"1.0" in a.commit.message

tests/test_gpgsign.py

@@ -1,13 +1,15 @@
-# pylint: skip-file
-
-from conftest import *
+import os
+from pathlib import Path
 from subprocess import CalledProcessError
+import pytest
+from gitrevise.odb import Repository
 from gitrevise.utils import sh_run
+from .conftest import bash, main
 
 
 def test_gpgsign(
     repo: Repository,
-    short_tmpdir: py.path.local,
+    short_tmpdir: Path,
     monkeypatch: pytest.MonkeyPatch,
 ) -> None:
     bash("git commit --allow-empty -m 'commit 1'")
@@ -30,7 +32,7 @@ def test_gpgsign(
         monkeypatch.setenv("GNUPGHOME", str(gnupghome))
 
     gnupghome.chmod(0o700)
-    (gnupghome / "gpg.conf").write("pinentry-mode loopback")
+    (gnupghome / "gpg.conf").write_text("pinentry-mode loopback")
     user_ident = repo.default_author.signing_key
     sh_run(
         ["gpg", "--batch", "--passphrase", "", "--quick-gen-key", user_ident],

tests/test_interactive.py

@@ -1,7 +1,17 @@
-# pylint: skip-file
+# pylint: disable=not-context-manager
 
+from typing import (
+    Optional,
+    Sequence,
+    TYPE_CHECKING,
+)
 import pytest
-from conftest import *
+from gitrevise.odb import Repository
+from .conftest import bash, editor_main
+
+
+if TYPE_CHECKING:
+    from _typeshed import StrPath
 
 
 def interactive_reorder_helper(repo: Repository, cwd: "StrPath") -> None:

tests/test_rerere.py

@@ -1,16 +1,17 @@
-# pylint: skip-file
+# pylint: disable=not-context-manager
 
 import textwrap
 
-from conftest import *
+from gitrevise.odb import Repository
 from gitrevise.merge import normalize_conflicted_file
+from .conftest import bash, changeline, main, editor_main, Editor
 
 
-def history_with_two_conflicting_commits(autoUpdate: bool = False) -> None:
+def history_with_two_conflicting_commits(auto_update: bool = False) -> None:
     bash(
         f"""
         git config rerere.enabled true
-        git config rerere.autoUpdate {"true" if autoUpdate else "false"}
+        git config rerere.autoUpdate {"true" if auto_update else "false"}
         echo > file; git add file; git commit -m 'initial commit'
         echo one > file; git commit -am 'commit one'
         echo two > file; git commit -am 'commit two'
@@ -19,7 +20,7 @@ def history_with_two_conflicting_commits(autoUpdate: bool = False) -> None:
 
 
 def test_reuse_recorded_resolution(repo: Repository) -> None:
-    history_with_two_conflicting_commits(autoUpdate=True)
+    history_with_two_conflicting_commits(auto_update=True)
 
     with editor_main(("-i", "HEAD~~"), input=b"y\ny\ny\ny\n") as ed:
         flip_last_two_commits(repo, ed)
@@ -57,7 +58,7 @@ def test_reuse_recorded_resolution(repo: Repository) -> None:
 
 
 def test_rerere_no_autoupdate(repo: Repository) -> None:
-    history_with_two_conflicting_commits(autoUpdate=False)
+    history_with_two_conflicting_commits(auto_update=False)
 
     with editor_main(("-i", "HEAD~~"), input=b"y\ny\ny\ny\n") as ed:
         flip_last_two_commits(repo, ed)
@@ -151,7 +152,7 @@ def test_rerere_merge(repo: Repository) -> None:
 
 
 def test_replay_resolution_recorded_by_git(repo: Repository) -> None:
-    history_with_two_conflicting_commits(autoUpdate=True)
+    history_with_two_conflicting_commits(auto_update=True)
     # Switch the order of the last two commits, recording the conflict
     # resolution with Git itself.
     bash(

tests/test_reword.py

@@ -1,7 +1,9 @@
-# pylint: skip-file
+# pylint: disable=not-context-manager
 
 import textwrap
-from conftest import *
+import pytest
+from gitrevise.odb import Repository
+from .conftest import bash, main, editor_main
 
 
 @pytest.mark.parametrize("target", ["HEAD", "HEAD~", "HEAD~~"])