Enable most of the pylint checks (#4111)

.pylintrc

@@ -28,7 +28,7 @@ limit-inference-results=100
 
 # List of plugins (as comma separated values of python modules names) to load,
 # usually to register additional checkers.
-load-plugins=
+load-plugins=pylint_pytest
 
 # Pickle collected data for later comparisons.
 persistent=yes
@@ -60,41 +60,130 @@ confidence=
 # --enable=similarities". If you want to run only the classes checker, but have
 # no Warning level messages displayed, use "--disable=all --enable=classes
 # --disable=W".
-disable=all
+disable=format,
+        # ^ most of them are covered by flake8,
+        # and are conventions rather than warnings or errors
+        refactoring,  # will be noisy if it starts giving suggestions
+        spelling,
+        # design is too-few-* and too-many-* checks, too-much-* for us right now
+        design,
+        # following were generated by default
+        # most of them are redundant Py3 checks for Py2+3 codebase
+        print-statement,
+        parameter-unpacking,
+        unpacking-in-except,
+        old-raise-syntax,
+        backtick,
+        long-suffix,
+        old-ne-operator,
+        old-octal-literal,
+        import-star-module-level,
+        non-ascii-bytes-literal,
+        raw-checker-failed,
+        bad-inline-option,
+        locally-disabled,
+        file-ignored,
+        suppressed-message,
+        useless-suppression,
+        deprecated-pragma,
+        use-symbolic-message-instead,
+        apply-builtin,
+        basestring-builtin,
+        buffer-builtin,
+        cmp-builtin,
+        coerce-builtin,
+        execfile-builtin,
+        file-builtin,
+        long-builtin,
+        raw_input-builtin,
+        reduce-builtin,
+        standarderror-builtin,
+        unicode-builtin,
+        xrange-builtin,
+        coerce-method,
+        delslice-method,
+        getslice-method,
+        setslice-method,
+        no-absolute-import,
+        old-division,
+        dict-iter-method,
+        dict-view-method,
+        next-method-called,
+        metaclass-assignment,
+        indexing-exception,
+        raising-string,
+        reload-builtin,
+        oct-method,
+        hex-method,
+        nonzero-method,
+        cmp-method,
+        input-builtin,
+        round-builtin,
+        intern-builtin,
+        unichr-builtin,
+        map-builtin-not-iterating,
+        zip-builtin-not-iterating,
+        range-builtin-not-iterating,
+        filter-builtin-not-iterating,
+        using-cmp-argument,
+        eq-without-hash,
+        div-method,
+        idiv-method,
+        rdiv-method,
+        exception-message-attribute,
+        invalid-str-codec,
+        sys-max-int,
+        bad-python3-import,
+        deprecated-string-function,
+        deprecated-str-translate-call,
+        deprecated-itertools-function,
+        deprecated-types-field,
+        next-method-defined,
+        dict-items-not-iterating,
+        dict-keys-not-iterating,
+        dict-values-not-iterating,
+        deprecated-operator-function,
+        deprecated-urllib-function,
+        xreadlines-attribute,
+        deprecated-sys-function,
+        exception-escape,
+        comprehension-escape,
+        # custom disables start here
+        invalid-name,
+        blacklisted-name,
+        unused-wildcard-import,
+        misplaced-comparison-constant,
+        # probably soon: https://github.com/iterative/dvc/issues/1843
+        logging-format-interpolation,
+        logging-fstring-interpolation,
+        # we don't have proper docstrings yet
+        # even if we do, we'll be doing it gradually
+        missing-function-docstring,
+        missing-module-docstring,
+        missing-class-docstring,
+        duplicate-code,
+        cyclic-import,
+        wrong-import-order,  # using isort
+        wrong-import-position, # using isort
+        ungrouped-imports, # using isort
+        multiple-imports,  # isort
+        import-outside-toplevel, # we use it to speedup/optimize
+        fixme,
+        # Enabling these soon:
+        attribute-defined-outside-init,  # needs-refactor
+        # Enabling someday:
+        no-member,  # needs-refactor
+        unused-argument, # need to cleanup
+        protected-access,  # needs-refactor
+        arguments-differ,
+        no-self-use,
+
 
 # Enable the message, report, category or checker with the given id(s). You can
 # either give multiple identifier separated by comma (,) or put this option
 # multiple time (only on the command line, not in the configuration file where
 # it should appear only once). See also the "--disable" option for examples.
-enable=print-statement,
-       c-extension-no-member,
-       import-error,
-       import-self,
-       reimported,
-       deprecated-module,
-       wildcard-import,
-       useless-import-alias,
-       unpacking-in-except,
-       parameter-unpacking,
-       unpacking-non-sequence,
-       invalid-all-object,
-       no-name-in-module,
-       undefined-variable,
-       undefined-all-variable,
-       used-before-assignment,
-       cell-var-from-loop,
-       global-variable-undefined,
-       self-cls-assignment,
-       unbalanced-tuple-unpacking,
-       possibly-unused-variable,
-       redefined-builtin,
-       redefine-in-handler,
-       unused-import,
-
-# Needs Cleanup
-# unused-argument,
-# redefined-outer-name,
-
+enable=c-extension-no-member
 
 
 [REPORTS]

dvc/api.py

@@ -60,7 +60,9 @@ def open(  # noqa, pylint: disable=redefined-builtin
 
 
 class _OpenContextManager(GCM):
-    def __init__(self, func, args, kwds):
+    def __init__(
+        self, func, args, kwds
+    ):  # pylint: disable=super-init-not-called
         self.gen = func(*args, **kwds)
         self.func, self.args, self.kwds = func, args, kwds
 

dvc/command/base.py

@@ -1,4 +1,5 @@
 import logging
+from abc import ABC, abstractmethod
 
 logger = logging.getLogger(__name__)
 
@@ -26,7 +27,7 @@ def append_doc_link(help_message, path):
     )
 
 
-class CmdBase:
+class CmdBase(ABC):
     def __init__(self, args):
         from dvc.repo import Repo
         from dvc.updater import Updater
@@ -46,11 +47,11 @@ class CmdBase:
         logger.debug(f"assuming default target '{PIPELINE_FILE}'.")
         return [PIPELINE_FILE]
 
-    # Abstract methods that have to be implemented by any inheritance class
+    @abstractmethod
     def run(self):
-        raise NotImplementedError
+        pass
 
 
 class CmdBaseNoRepo(CmdBase):
-    def __init__(self, args):
+    def __init__(self, args):  # pylint: disable=super-init-not-called
         self.args = args

dvc/command/destroy.py

@@ -25,7 +25,7 @@ class CmdDestroy(CmdBase):
                 )
 
             self.repo.destroy()
-        except Exception:
+        except Exception:  # noqa, pylint: disable=broad-except
             logger.exception("failed to destroy DVC")
             return 1
         return 0

dvc/command/diff.py

@@ -56,7 +56,7 @@ class CmdDiff(CmdBase):
         """
 
         def _digest(checksum):
-            if type(checksum) is str:
+            if isinstance(checksum, str):
                 return checksum[0:8]
             return "{}..{}".format(checksum["old"][0:8], checksum["new"][0:8])
 

dvc/command/install.py

@@ -10,7 +10,7 @@ class CmdInstall(CmdBase):
     def run(self):
         try:
             self.repo.install(self.args.use_pre_commit_tool)
-        except Exception:
+        except Exception:  # noqa, pylint:disable=broad-except
             logger.exception("failed to install DVC Git hooks")
             return 1
         return 0

dvc/command/status.py

@@ -63,7 +63,7 @@ class CmdDataStatus(CmdDataBase):
             else:
                 logger.info(self.UP_TO_DATE_MSG)
 
-        except Exception:
+        except Exception:  # noqa, pylint: disable=broad-except
             logger.exception("failed to obtain data status")
             return 1
         return 0

dvc/config.py

@@ -231,7 +231,9 @@ class Config(dict):
     CONFIG = "config"
     CONFIG_LOCAL = "config.local"
 
-    def __init__(self, dvc_dir=None, validate=True):
+    def __init__(
+        self, dvc_dir=None, validate=True
+    ):  # pylint: disable=super-init-not-called
         self.dvc_dir = dvc_dir
 
         if not dvc_dir:

dvc/dvcfile.py

@@ -1,3 +1,4 @@
+import collections
 import contextlib
 import logging
 import os
@@ -101,9 +102,9 @@ class FileMixin:
 
     @classmethod
     def validate(cls, d, fname=None):
-        assert cls.SCHEMA
+        assert isinstance(cls.SCHEMA, collections.abc.Callable)
         try:
-            cls.SCHEMA(d)
+            cls.SCHEMA(d)  # pylint: disable=not-callable
         except MultipleInvalid as exc:
             raise StageFileFormatError(f"'{fname}' format error: {exc}")
 
@@ -118,9 +119,6 @@ class FileMixin:
 class SingleStageFile(FileMixin):
     from dvc.schema import COMPILED_SINGLE_STAGE_SCHEMA as SCHEMA
 
-    def __init__(self, repo, path):
-        super().__init__(repo, path)
-
     @property
     def stage(self):
         data, raw = self._load()

dvc/lock.py

@@ -88,7 +88,9 @@ class HardlinkLock(flufl.lock.Lock):
         tmp_dir (str): a directory to store claim files.
     """
 
-    def __init__(self, lockfile, tmp_dir=None, **kwargs):
+    def __init__(
+        self, lockfile, tmp_dir=None, **kwargs
+    ):  # pylint: disable=super-init-not-called
         import socket
 
         self._tmp_dir = tmp_dir

dvc/logger.py

@@ -137,7 +137,7 @@ class LoggerHandler(logging.StreamHandler):
             self.flush()
         except RecursionError:
             raise
-        except Exception:
+        except Exception:  # noqa, pylint: disable=broad-except
             self.handleError(record)
 
 

dvc/remote/base.py

@@ -270,10 +270,10 @@ class BaseRemoteTree:
             return None
 
         if tree == self:
+            # pylint: disable=assignment-from-none
             hash_ = self.state.get(path_info)
         else:
             hash_ = None
-
         # If we have dir hash in state db, but dir cache file is lost,
         # then we need to recollect the dir via .get_dir_hash() call below,
         # see https://github.com/iterative/dvc/issues/2219 for context

dvc/remote/http.py

@@ -23,7 +23,7 @@ def ask_password(host, user):
     )
 
 
-class HTTPRemoteTree(BaseRemoteTree):
+class HTTPRemoteTree(BaseRemoteTree):  # pylint:disable=abstract-method
     scheme = Schemes.HTTP
     PATH_CLS = HTTPURLInfo
     PARAM_CHECKSUM = "etag"

dvc/remote/https.py

@@ -3,5 +3,5 @@ from dvc.scheme import Schemes
 from .http import HTTPRemoteTree
 
 
-class HTTPSRemoteTree(HTTPRemoteTree):
+class HTTPSRemoteTree(HTTPRemoteTree):  # pylint:disable=abstract-method
     scheme = Schemes.HTTPS

dvc/remote/index.py

@@ -66,7 +66,7 @@ class RemoteIndex:
     INDEX_SUFFIX = ".idx"
     VERSION = 1
     INDEX_TABLE = "remote_index"
-    INDEX_TABLE_LAYOUT = "checksum TEXT PRIMARY KEY, " "dir INTEGER NOT NULL"
+    INDEX_TABLE_LAYOUT = "checksum TEXT PRIMARY KEY, dir INTEGER NOT NULL"
 
     def __init__(self, repo, name, dir_suffix=".dir"):
         self.path = os.path.join(repo.index_dir, f"{name}{self.INDEX_SUFFIX}")

dvc/remote/local.py

@@ -327,7 +327,7 @@ def _log_exceptions(func, operation):
         try:
             func(from_info, to_info, *args, **kwargs)
             return 0
-        except Exception as exc:
+        except Exception as exc:  # pylint: disable=broad-except
             # NOTE: this means we ran out of file descriptors and there is no
             # reason to try to proceed, as we will hit this error anyways.
             if isinstance(exc, OSError) and exc.errno == errno.EMFILE:

dvc/remote/oss.py

@@ -12,7 +12,7 @@ from dvc.scheme import Schemes
 logger = logging.getLogger(__name__)
 
 
-class OSSRemoteTree(BaseRemoteTree):
+class OSSRemoteTree(BaseRemoteTree):  # pylint:disable=abstract-method
     """
     oss2 document:
     https://www.alibabacloud.com/help/doc-detail/32026.htm

dvc/repo/status.py

@@ -11,7 +11,7 @@ logger = logging.getLogger(__name__)
 
 
 def _joint_status(stages):
-    status = {}
+    status_info = {}
 
     for stage in stages:
         if stage.frozen and not stage.is_repo_import:
@@ -20,9 +20,9 @@ def _joint_status(stages):
                 " not going to be shown in the status output.".format(stage)
             )
 
-        status.update(stage.status(check_updates=True))
+        status_info.update(stage.status(check_updates=True))
 
-    return status
+    return status_info
 
 
 def _local_status(self, targets=None, with_deps=False, recursive=False):
@@ -92,13 +92,13 @@ def _cloud_status(
     status_info = self.cloud.status(used, jobs, remote=remote)
     for info in status_info.values():
         name = info["name"]
-        status = info["status"]
-        if status in [cloud.STATUS_OK, cloud.STATUS_MISSING]:
+        status_ = info["status"]
+        if status_ in [cloud.STATUS_OK, cloud.STATUS_MISSING]:
             continue
 
         prefix_map = {cloud.STATUS_DELETED: "deleted", cloud.STATUS_NEW: "new"}
 
-        ret[name] = prefix_map[status]
+        ret[name] = prefix_map[status_]
 
     return ret
 

dvc/stage/__init__.py

@@ -88,6 +88,9 @@ def create_stage(cls, repo, path, external=False, **kwargs):
 
 
 class Stage(params.StageParams):
+    # pylint:disable=no-value-for-parameter
+    # rwlocked() confuses pylint
+
     def __init__(
         self,
         repo,

dvc/state.py

@@ -445,9 +445,9 @@ class State:  # pylint: disable=too-many-instance-attributes
 
         self._execute(f"SELECT * FROM {self.LINK_STATE_TABLE}")
         for row in self.cursor:
-            relpath, inode, mtime = row
+            relative_path, inode, mtime = row
             inode = self._from_sqlite(inode)
-            path = os.path.join(self.root_dir, relpath)
+            path = os.path.join(self.root_dir, relative_path)
 
             if path in used or not self.tree.exists(path):
                 continue
@@ -457,7 +457,7 @@ class State:  # pylint: disable=too-many-instance-attributes
 
             if (inode, mtime) == (actual_inode, actual_mtime):
                 logger.debug("Removing '%s' as unused link.", path)
-                unused.append(relpath)
+                unused.append(relative_path)
 
         return unused
 

dvc/updater.py

@@ -62,7 +62,7 @@ class Updater:  # pragma: no cover
             try:
                 info = json.load(fobj)
                 self.latest = info["version"]
-            except Exception as exc:
+            except Exception as exc:  # pylint: disable=broad-except
                 msg = "'{}' is not a valid json: {}"
                 logger.debug(msg.format(self.updater_file, exc))
                 self.fetch()

dvc/utils/stream.py

@@ -4,7 +4,7 @@ import io
 class IterStream(io.RawIOBase):
     """Wraps an iterator yielding bytes as a file object"""
 
-    def __init__(self, iterator):
+    def __init__(self, iterator):  # pylint: disable=super-init-not-called
         self.iterator = iterator
         self.leftover = None
 

dvc/utils/yaml.py

@@ -1,6 +1,5 @@
 from collections import OrderedDict
 
-import yaml
 from ruamel.yaml import YAML
 from ruamel.yaml.error import YAMLError
 
@@ -19,6 +18,8 @@ def load_yaml(path):
 
 def parse_yaml(text, path):
     try:
+        import yaml
+
         return yaml.load(text, Loader=SafeLoader) or {}
     except yaml.error.YAMLError as exc:
         raise YAMLFileCorruptedError(path) from exc

setup.py

@@ -128,6 +128,7 @@ tests_requirements = [
     "flake8-comprehensions",
     "flake8-string-format",
     "pylint",
+    "pylint-pytest",
 ]
 
 if (sys.version_info) >= (3, 6):

tests/__main__.py

@@ -1,3 +1,7 @@
+# Needs investigation, pylint was running tests
+# Could be `pylint_pytest` plugin, skipping check for this file for now
+# pylint: skip-file
+
 import os
 import sys
 from subprocess import check_call

tests/conftest.py

@@ -2,8 +2,8 @@ import os
 
 import pytest
 
-from .dir_helpers import *  # noqa, pylint:disable=wildcard-import
-from .remotes import *  # noqa, pylint:disable=wildcard-import
+from .dir_helpers import *  # noqa, pylint: disable=wildcard-import
+from .remotes import *  # noqa, pylint: disable=wildcard-import
 
 # Prevent updater and analytics from running their processes
 os.environ["DVC_TEST"] = "true"

tests/dir_helpers.py

@@ -43,6 +43,8 @@ from global repo template to creating everything inplace, which:
     - does not create unnecessary files
 """
 
+# pylint: disable=redefined-outer-name
+
 import os
 import pathlib
 from contextlib import contextmanager

tests/func/plots/conftest.py

@@ -5,8 +5,8 @@ import pytest
 
 @pytest.fixture()
 def custom_template(tmp_dir, dvc):
-    custom_template = tmp_dir / "custom_template.json"
+    template = tmp_dir / "custom_template.json"
     shutil.copy(
-        tmp_dir / ".dvc" / "plots" / "default.json", custom_template,
+        tmp_dir / ".dvc" / "plots" / "default.json", template,
     )
-    return custom_template
+    return template

tests/func/test_cli.py

@@ -162,13 +162,17 @@ class TestFindRoot(TestDvc):
     def test(self):
         os.chdir("..")
 
+        class Cmd(CmdBase):
+            def run(self):
+                pass
+
         class A:
             quiet = False
             verbose = True
 
         args = A()
         with self.assertRaises(DvcException):
-            CmdBase(args)
+            Cmd(args)
 
 
 def test_unknown_command_help(capsys):

tests/func/test_dependency.py

@@ -17,4 +17,5 @@ def _get_dep(dvc, path):
 
 @pytest.mark.parametrize("url,scheme", TESTS)
 def test_scheme(dvc, url, scheme):
+    # pylint: disable=unidiomatic-typecheck
     assert type(_get_dep(dvc, url)) == DEP_MAP[scheme]

tests/func/test_lock.py

@@ -13,7 +13,7 @@ class TestLock(TestDvc):
             with self.assertRaises(LockError):
                 lock2 = Lock(lockfile)
                 with lock2:
-                    self.assertTrue(False)
+                    pass
 
     def test_cli(self):
         lockfile = os.path.join(self.dvc.dvc_dir, "tmp", "lock")

tests/func/test_output.py

@@ -25,4 +25,5 @@ def _get_out(dvc, path):
 
 @pytest.mark.parametrize("url,scheme", TESTS)
 def test_scheme(dvc, url, scheme):
+    # pylint: disable=unidiomatic-typecheck
     assert type(_get_out(dvc, url)) == OUTS_MAP[scheme]

tests/func/test_repo.py

@@ -136,12 +136,12 @@ def test_collect_with_not_existing_output_or_stage_name(
 
 
 def test_stages(tmp_dir, dvc):
-    def stages():
+    def collect_stages():
         return {stage.relpath for stage in Repo(os.fspath(tmp_dir)).stages}
 
     tmp_dir.dvc_gen({"file": "a", "dir/file": "b", "dir/subdir/file": "c"})
 
-    assert stages() == {
+    assert collect_stages() == {
         "file.dvc",
         os.path.join("dir", "file.dvc"),
         os.path.join("dir", "subdir", "file.dvc"),
@@ -149,7 +149,7 @@ def test_stages(tmp_dir, dvc):
 
     tmp_dir.gen(".dvcignore", "dir")
 
-    assert stages() == {"file.dvc"}
+    assert collect_stages() == {"file.dvc"}
 
 
 @pytest.fixture

tests/func/test_repro.py

@@ -1131,9 +1131,7 @@ class TestReproExternalSSH(SSH, TestReproExternalBase):
             except OSError:
                 pass
 
-            stdin, stdout, stderr = ssh.exec_command(
-                f"mkdir -p $(dirname {path})"
-            )
+            _, stdout, _ = ssh.exec_command(f"mkdir -p $(dirname {path})")
             self.assertEqual(stdout.channel.recv_exit_status(), 0)
 
             with sftp.open(path, "w+") as fobj:

tests/remotes/base.py

@@ -6,10 +6,6 @@ class Base:
     def should_test():
         return True
 
-    @staticmethod
-    def get_storagepath():
-        raise NotImplementedError
-
     @staticmethod
     def get_url():
         raise NotImplementedError

tests/remotes/gdrive.py

@@ -30,13 +30,13 @@ class GDrive(Base):
         tree._gdrive_create_dir("root", tree.path_info.path)
 
     @staticmethod
-    def get_storagepath():
+    def _get_storagepath():
         return TEST_GDRIVE_REPO_BUCKET + "/" + str(uuid.uuid4())
 
     @staticmethod
     def get_url():
         # NOTE: `get_url` should always return new random url
-        return "gdrive://" + GDrive.get_storagepath()
+        return "gdrive://" + GDrive._get_storagepath()
 
 
 @pytest.fixture

tests/remotes/gs.py

@@ -57,12 +57,12 @@ class GCP(Base):
         }
 
     @staticmethod
-    def get_storagepath():
+    def _get_storagepath():
         return TEST_GCP_REPO_BUCKET + "/" + str(uuid.uuid4())
 
     @staticmethod
     def get_url():
-        return "gs://" + GCP.get_storagepath()
+        return "gs://" + GCP._get_storagepath()
 
     @classmethod
     @contextmanager

tests/remotes/hdfs.py

@@ -16,6 +16,8 @@ class HDFS(Base):
             return False
 
         try:
+            # pylint: disable=unexpected-keyword-arg
+            # see: https://github.com/PyCQA/pylint/issues/3645
             check_output(
                 ["hadoop", "version"],
                 shell=True,

tests/remotes/oss.py

@@ -24,12 +24,12 @@ class OSS(Base):
         )
 
     @staticmethod
-    def get_storagepath():
+    def _get_storagepath():
         return f"{TEST_OSS_REPO_BUCKET}/{uuid.uuid4()}"
 
     @staticmethod
     def get_url():
-        return f"oss://{OSS.get_storagepath()}"
+        return f"oss://{OSS._get_storagepath()}"
 
 
 @pytest.fixture

tests/remotes/s3.py

@@ -29,12 +29,12 @@ class S3(Base):
         return False
 
     @staticmethod
-    def get_storagepath():
+    def _get_storagepath():
         return TEST_AWS_REPO_BUCKET + "/" + str(uuid.uuid4())
 
     @staticmethod
     def get_url():
-        return "s3://" + S3.get_storagepath()
+        return "s3://" + S3._get_storagepath()
 
 
 @pytest.fixture
@@ -59,10 +59,10 @@ class S3Mocked(S3):
 
     @staticmethod
     def put_objects(remote, objects):
-        s3 = remote.tree.s3
+        client = remote.tree.s3
         bucket = remote.path_info.bucket
-        s3.create_bucket(Bucket=bucket)
+        client.create_bucket(Bucket=bucket)
         for key, body in objects.items():
-            s3.put_object(
+            client.put_object(
                 Bucket=bucket, Key=(remote.path_info / key).path, Body=body
             )

tests/unit/command/test_git_hook.py

@@ -14,7 +14,7 @@ from dvc.command.git_hook import CmdPostCheckout, CmdPreCommit, CmdPrePush
 )
 def test_out_of_repo(tmp_dir, hook, cls, mocker):
     cli_args = parse_args(["git-hook", hook])
-    cli_args.func == cls
+    assert cli_args.func == cls
     cmd = cli_args.func(cli_args)
     mock_main = mocker.patch("dvc.main.main")
     assert cmd.run() == 0

tests/unit/output/test_load.py

@@ -1,3 +1,5 @@
+# pylint: disable=unsubscriptable-object
+
 import pytest
 
 from dvc import output

tests/unit/output/test_output.py

@@ -42,7 +42,7 @@ def test_checksum_schema(value, expected):
 @pytest.mark.parametrize("value", ["1", "11", {}, {"a": "b"}, [], [1, 2]])
 def test_checksum_schema_fail(value):
     with pytest.raises(MultipleInvalid):
-        Schema(CHECKSUM_SCHEMA)(value)["md5"]
+        assert Schema(CHECKSUM_SCHEMA)(value)
 
 
 @pytest.mark.parametrize(

tests/unit/remote/test_gdrive.py

@@ -26,7 +26,7 @@ class TestRemoteGDrive:
             GDriveRemoteTree.GDRIVE_CREDENTIALS_DATA
         ] = USER_CREDS_TOKEN_REFRESH_ERROR
         with pytest.raises(GDriveAuthError):
-            tree._drive
+            assert tree._drive
 
         os.environ[GDriveRemoteTree.GDRIVE_CREDENTIALS_DATA] = ""
         tree = GDriveRemoteTree(dvc, self.CONFIG)
@@ -34,4 +34,4 @@ class TestRemoteGDrive:
             GDriveRemoteTree.GDRIVE_CREDENTIALS_DATA
         ] = USER_CREDS_MISSED_KEY_ERROR
         with pytest.raises(GDriveAuthError):
-            tree._drive
+            assert tree._drive

tests/unit/remote/test_gs.py

@@ -27,7 +27,7 @@ def test_init(dvc):
 def test_gs(mock_client, dvc):
     tree = GSRemoteTree(dvc, CONFIG)
     assert tree.credentialpath
-    tree.gs()
+    assert tree.gs
     mock_client.assert_called_once_with(CREDENTIALPATH)
 
 
@@ -36,7 +36,7 @@ def test_gs_no_credspath(mock_client, dvc):
     config = CONFIG.copy()
     del config["credentialpath"]
     tree = GSRemoteTree(dvc, config)
-    tree.gs()
+    assert tree.gs
     mock_client.assert_called_with(PROJECT)
 
 

tests/unit/remote/test_index.py

@@ -8,11 +8,11 @@ from dvc.remote.index import RemoteIndex
 
 @pytest.fixture(scope="function")
 def index(dvc):
-    index = RemoteIndex(dvc, "foo")
-    index.load()
-    yield index
-    index.dump()
-    os.unlink(index.path)
+    idx = RemoteIndex(dvc, "foo")
+    idx.load()
+    yield idx
+    idx.dump()
+    os.unlink(idx.path)
 
 
 def test_init(dvc, index):

tests/unit/remote/test_remote_tree.py

@@ -29,9 +29,9 @@ FILE_WITH_CONTENTS = {
 def remote(request, dvc):
     if not request.param.should_test():
         raise pytest.skip()
-    with request.param.remote(dvc) as remote:
-        request.param.put_objects(remote, FILE_WITH_CONTENTS)
-        yield remote
+    with request.param.remote(dvc) as _remote:
+        request.param.put_objects(_remote, FILE_WITH_CONTENTS)
+        yield _remote
 
 
 @pytest.mark.parametrize("remote", remotes, indirect=True)

tests/unit/stage/test_stage.py

@@ -91,6 +91,7 @@ def test_stage_run_ignore_sigint(dvc, mocker):
     assert popen.called_once()
     assert communicate.called_once_with()
     signal_mock.assert_any_call(signal.SIGINT, signal.SIG_IGN)
+    # pylint: disable=comparison-with-callable
     assert signal.getsignal(signal.SIGINT) == signal.default_int_handler
 
 

tests/unit/test_analytics.py

@@ -24,11 +24,8 @@ def test_collect_and_send_report(mock_json, mock_daemon, tmp_global_dir):
     analytics.collect_and_send_report()
     report = mock_json.call_args[0][0]
 
-    with pytest.raises(KeyError):
-        report["cmd_class"]
-
-    with pytest.raises(KeyError):
-        report["cmd_return_code"]
+    assert not report.get("cmd_class")
+    assert not report.get("cmd_return_code")
 
     args = parse_args(["add", "foo"])
     return_code = 0

tests/unit/test_daemon.py

@@ -16,11 +16,11 @@ class TestDaemon(TestCase):
         if os.name == "nt":
             mock_posix.assert_not_called()
             mock_windows.assert_called()
-            args, kwargs = mock_windows.call_args
+            args = mock_windows.call_args[0]
         else:
             mock_windows.assert_not_called()
             mock_posix.assert_called()
-            args, kwargs = mock_posix.call_args
+            args = mock_posix.call_args[0]
 
         env = args[1]
         self.assertTrue("PYTHONPATH" in env.keys())

tests/unit/test_imports.py

@@ -14,6 +14,8 @@ def test_no_remote_imports():
     }
 
     code = "import dvc.main, sys; print(' '.join(sys.modules))"
-    res = subprocess.run(["python", "-c", code], stdout=subprocess.PIPE)
+    res = subprocess.run(
+        ["python", "-c", code], stdout=subprocess.PIPE, check=True
+    )
     modules = res.stdout.decode().split()
     assert not set(modules) & remote_modules

tests/unit/test_logger.py

@@ -29,6 +29,7 @@ def dt(mocker):
 
 
 class TestColorFormatter:
+    # pylint: disable=broad-except
     def test_debug(self, caplog, dt):
         with caplog.at_level(logging.DEBUG, logger="dvc"):
             logger.debug("message")

tests/unit/utils/test_collections.py

@@ -1,3 +1,4 @@
+# pylint: disable=unidiomatic-typecheck
 from dvc.utils.collections import apply_diff
 
 

tests/unit/utils/test_fs.py

@@ -206,6 +206,7 @@ def test_path_isin_accepts_pathinfo():
     parent = PathInfo(child) / ".."
 
     assert path_isin(child, parent)
+    # pylint: disable=arguments-out-of-order
     assert not path_isin(parent, child)