From ad306d7bf38582eda015d1708a4d7e25d236ee5a Mon Sep 17 00:00:00 2001 From: Saugat Pachhai Date: Thu, 25 Jun 2020 15:10:14 +0000 Subject: [PATCH] Enable most of the pylint checks (#4111) --- .pylintrc | 151 ++++++++++++++++++++------ dvc/api.py | 4 +- dvc/command/base.py | 9 +- dvc/command/destroy.py | 2 +- dvc/command/diff.py | 2 +- dvc/command/install.py | 2 +- dvc/command/status.py | 2 +- dvc/config.py | 4 +- dvc/dvcfile.py | 8 +- dvc/lock.py | 4 +- dvc/logger.py | 2 +- dvc/remote/base.py | 2 +- dvc/remote/http.py | 2 +- dvc/remote/https.py | 2 +- dvc/remote/index.py | 2 +- dvc/remote/local.py | 2 +- dvc/remote/oss.py | 2 +- dvc/repo/status.py | 12 +- dvc/stage/__init__.py | 3 + dvc/state.py | 6 +- dvc/updater.py | 2 +- dvc/utils/stream.py | 2 +- dvc/utils/yaml.py | 3 +- setup.py | 1 + tests/__main__.py | 4 + tests/conftest.py | 4 +- tests/dir_helpers.py | 2 + tests/func/plots/conftest.py | 6 +- tests/func/test_cli.py | 6 +- tests/func/test_dependency.py | 1 + tests/func/test_lock.py | 2 +- tests/func/test_output.py | 1 + tests/func/test_repo.py | 6 +- tests/func/test_repro.py | 4 +- tests/remotes/base.py | 4 - tests/remotes/gdrive.py | 4 +- tests/remotes/gs.py | 4 +- tests/remotes/hdfs.py | 2 + tests/remotes/oss.py | 4 +- tests/remotes/s3.py | 10 +- tests/unit/command/test_git_hook.py | 2 +- tests/unit/output/test_load.py | 2 + tests/unit/output/test_output.py | 2 +- tests/unit/remote/test_gdrive.py | 4 +- tests/unit/remote/test_gs.py | 4 +- tests/unit/remote/test_index.py | 10 +- tests/unit/remote/test_remote_tree.py | 6 +- tests/unit/stage/test_stage.py | 1 + tests/unit/test_analytics.py | 7 +- tests/unit/test_daemon.py | 4 +- tests/unit/test_imports.py | 4 +- tests/unit/test_logger.py | 1 + tests/unit/utils/test_collections.py | 1 + tests/unit/utils/test_fs.py | 1 + 54 files changed, 228 insertions(+), 116 deletions(-) diff --git a/.pylintrc b/.pylintrc index 35a7f2f80..f8f0ed3d8 100644 --- a/.pylintrc +++ b/.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] diff --git a/dvc/api.py b/dvc/api.py index d0da10171..f51e8287d 100644 --- a/dvc/api.py +++ b/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 diff --git a/dvc/command/base.py b/dvc/command/base.py index 3b41a2e14..bde1aaeeb 100644 --- a/dvc/command/base.py +++ b/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 diff --git a/dvc/command/destroy.py b/dvc/command/destroy.py index fc3b4f026..b3ec993d1 100644 --- a/dvc/command/destroy.py +++ b/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 diff --git a/dvc/command/diff.py b/dvc/command/diff.py index 7d83c32a7..a0f8b3419 100644 --- a/dvc/command/diff.py +++ b/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]) diff --git a/dvc/command/install.py b/dvc/command/install.py index 4b906a2ce..612fdd609 100644 --- a/dvc/command/install.py +++ b/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 diff --git a/dvc/command/status.py b/dvc/command/status.py index 97e51513f..c0d67fbcc 100644 --- a/dvc/command/status.py +++ b/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 diff --git a/dvc/config.py b/dvc/config.py index 3326e8194..43bc16516 100644 --- a/dvc/config.py +++ b/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: diff --git a/dvc/dvcfile.py b/dvc/dvcfile.py index 45e623074..3aeefb399 100644 --- a/dvc/dvcfile.py +++ b/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() diff --git a/dvc/lock.py b/dvc/lock.py index 59b8838fb..78cf35bbd 100644 --- a/dvc/lock.py +++ b/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 diff --git a/dvc/logger.py b/dvc/logger.py index 0d038ee01..25671e3d6 100644 --- a/dvc/logger.py +++ b/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) diff --git a/dvc/remote/base.py b/dvc/remote/base.py index 5d26c1f25..1d8a285e7 100644 --- a/dvc/remote/base.py +++ b/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 diff --git a/dvc/remote/http.py b/dvc/remote/http.py index 7affaa780..d87c4932e 100644 --- a/dvc/remote/http.py +++ b/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" diff --git a/dvc/remote/https.py b/dvc/remote/https.py index 1cfad246f..293f64fac 100644 --- a/dvc/remote/https.py +++ b/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 diff --git a/dvc/remote/index.py b/dvc/remote/index.py index aefbe9d8f..cbe66aa15 100644 --- a/dvc/remote/index.py +++ b/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}") diff --git a/dvc/remote/local.py b/dvc/remote/local.py index 23a1dfb58..6e8e996b1 100644 --- a/dvc/remote/local.py +++ b/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: diff --git a/dvc/remote/oss.py b/dvc/remote/oss.py index 11b6f1000..31feaed6a 100644 --- a/dvc/remote/oss.py +++ b/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 diff --git a/dvc/repo/status.py b/dvc/repo/status.py index 71ca7eee5..9efe6f91f 100644 --- a/dvc/repo/status.py +++ b/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 diff --git a/dvc/stage/__init__.py b/dvc/stage/__init__.py index c39969bab..6316793bb 100644 --- a/dvc/stage/__init__.py +++ b/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, diff --git a/dvc/state.py b/dvc/state.py index fd3924f93..c634b2d3a 100644 --- a/dvc/state.py +++ b/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 diff --git a/dvc/updater.py b/dvc/updater.py index 3c6a28597..ca8e7a17c 100644 --- a/dvc/updater.py +++ b/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() diff --git a/dvc/utils/stream.py b/dvc/utils/stream.py index 610947503..cfa18d183 100644 --- a/dvc/utils/stream.py +++ b/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 diff --git a/dvc/utils/yaml.py b/dvc/utils/yaml.py index 4866bd67d..70fb46f31 100644 --- a/dvc/utils/yaml.py +++ b/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 diff --git a/setup.py b/setup.py index eca386871..bf0250bf1 100644 --- a/setup.py +++ b/setup.py @@ -128,6 +128,7 @@ tests_requirements = [ "flake8-comprehensions", "flake8-string-format", "pylint", + "pylint-pytest", ] if (sys.version_info) >= (3, 6): diff --git a/tests/__main__.py b/tests/__main__.py index a93031ca7..05dfddd83 100644 --- a/tests/__main__.py +++ b/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 diff --git a/tests/conftest.py b/tests/conftest.py index 3cc7fc804..c9e5c5340 100644 --- a/tests/conftest.py +++ b/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" diff --git a/tests/dir_helpers.py b/tests/dir_helpers.py index bb892d639..e28bc0c0e 100644 --- a/tests/dir_helpers.py +++ b/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 diff --git a/tests/func/plots/conftest.py b/tests/func/plots/conftest.py index 9f94dcff5..ab9f8035e 100644 --- a/tests/func/plots/conftest.py +++ b/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 diff --git a/tests/func/test_cli.py b/tests/func/test_cli.py index ef1c8db15..76eacf43d 100644 --- a/tests/func/test_cli.py +++ b/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): diff --git a/tests/func/test_dependency.py b/tests/func/test_dependency.py index c02c4e076..7bc92295a 100644 --- a/tests/func/test_dependency.py +++ b/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] diff --git a/tests/func/test_lock.py b/tests/func/test_lock.py index 198193406..3e4aa3aac 100644 --- a/tests/func/test_lock.py +++ b/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") diff --git a/tests/func/test_output.py b/tests/func/test_output.py index df2430a6d..619323add 100644 --- a/tests/func/test_output.py +++ b/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] diff --git a/tests/func/test_repo.py b/tests/func/test_repo.py index 76138cf8d..a76147f59 100644 --- a/tests/func/test_repo.py +++ b/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 diff --git a/tests/func/test_repro.py b/tests/func/test_repro.py index 28a523cd8..160da4a58 100644 --- a/tests/func/test_repro.py +++ b/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: diff --git a/tests/remotes/base.py b/tests/remotes/base.py index ae8268f30..50904c7e5 100644 --- a/tests/remotes/base.py +++ b/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 diff --git a/tests/remotes/gdrive.py b/tests/remotes/gdrive.py index fb36b5fc7..c8dc7fe24 100644 --- a/tests/remotes/gdrive.py +++ b/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 diff --git a/tests/remotes/gs.py b/tests/remotes/gs.py index dc5bcb42d..090baa4d6 100644 --- a/tests/remotes/gs.py +++ b/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 diff --git a/tests/remotes/hdfs.py b/tests/remotes/hdfs.py index 78f1e5501..bade5333c 100644 --- a/tests/remotes/hdfs.py +++ b/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, diff --git a/tests/remotes/oss.py b/tests/remotes/oss.py index 7fe87ca02..e8a59c790 100644 --- a/tests/remotes/oss.py +++ b/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 diff --git a/tests/remotes/s3.py b/tests/remotes/s3.py index 866cd6c37..f6e2c6e15 100644 --- a/tests/remotes/s3.py +++ b/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 ) diff --git a/tests/unit/command/test_git_hook.py b/tests/unit/command/test_git_hook.py index e10779bb5..96000b512 100644 --- a/tests/unit/command/test_git_hook.py +++ b/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 diff --git a/tests/unit/output/test_load.py b/tests/unit/output/test_load.py index 68171a09a..1bc438672 100644 --- a/tests/unit/output/test_load.py +++ b/tests/unit/output/test_load.py @@ -1,3 +1,5 @@ +# pylint: disable=unsubscriptable-object + import pytest from dvc import output diff --git a/tests/unit/output/test_output.py b/tests/unit/output/test_output.py index c09392f37..c8ffc50f7 100644 --- a/tests/unit/output/test_output.py +++ b/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( diff --git a/tests/unit/remote/test_gdrive.py b/tests/unit/remote/test_gdrive.py index 5574c7290..1fcae5dd7 100644 --- a/tests/unit/remote/test_gdrive.py +++ b/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 diff --git a/tests/unit/remote/test_gs.py b/tests/unit/remote/test_gs.py index 1953410ca..ec86cfb5e 100644 --- a/tests/unit/remote/test_gs.py +++ b/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) diff --git a/tests/unit/remote/test_index.py b/tests/unit/remote/test_index.py index 7e475b3aa..2771de7d6 100644 --- a/tests/unit/remote/test_index.py +++ b/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): diff --git a/tests/unit/remote/test_remote_tree.py b/tests/unit/remote/test_remote_tree.py index a52416ad1..7bba2253c 100644 --- a/tests/unit/remote/test_remote_tree.py +++ b/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) diff --git a/tests/unit/stage/test_stage.py b/tests/unit/stage/test_stage.py index 7941322e9..3ca83d9d9 100644 --- a/tests/unit/stage/test_stage.py +++ b/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 diff --git a/tests/unit/test_analytics.py b/tests/unit/test_analytics.py index 633dcf55d..45720d013 100644 --- a/tests/unit/test_analytics.py +++ b/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 diff --git a/tests/unit/test_daemon.py b/tests/unit/test_daemon.py index a9f193eb9..469bc9516 100644 --- a/tests/unit/test_daemon.py +++ b/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()) diff --git a/tests/unit/test_imports.py b/tests/unit/test_imports.py index 12ff4250f..a442d2a5c 100644 --- a/tests/unit/test_imports.py +++ b/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 diff --git a/tests/unit/test_logger.py b/tests/unit/test_logger.py index e85a69ec4..37a725e68 100644 --- a/tests/unit/test_logger.py +++ b/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") diff --git a/tests/unit/utils/test_collections.py b/tests/unit/utils/test_collections.py index 4dd65abaf..3a296bf25 100644 --- a/tests/unit/utils/test_collections.py +++ b/tests/unit/utils/test_collections.py @@ -1,3 +1,4 @@ +# pylint: disable=unidiomatic-typecheck from dvc.utils.collections import apply_diff diff --git a/tests/unit/utils/test_fs.py b/tests/unit/utils/test_fs.py index f69e31d28..2a5cad96b 100644 --- a/tests/unit/utils/test_fs.py +++ b/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)