diff --git a/.travis.yml b/.travis.yml index 66be424aa..9aa2cca1f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,8 +13,6 @@ stages: - check - test - build - - name: deploy - if: (branch = master OR tag is present) AND repo = iterative/dvc # Travis doesn't support testing python on Mac yet, so we need # to workaround it by installing it directly with brew. # @@ -71,34 +69,11 @@ jobs: before_install: install: script: ./scripts/build_posix.sh - - name: "Pypi pkgs" + - name: "PyPI pkgs" language: python python: 3.7 script: ./scripts/build_package.sh - # deploy jobs - - name: "Pypi pkgs" - if: tag is present - stage: deploy - language: python - python: 3.7 - script: ./scripts/build_package.sh - deploy: - - provider: pypi - user: - secure: "IOzNfgPoG3ICg3+s4AuZCCnawgd/KqMzeiAB9R0B9Xb6ProPQJp9O3GBZrtg5OJMHdWQQtwULeOh/LWk4sFdXMcX1iNEgm5Tb7DwO7TDtltW14jUgN9AqfY2twj4UIsW/ON8w0P8v36ZoQq1kO6Pfgjijdm3w+9h6rpps8rJ6jXYpVZVO+SSzzijWddanMkEz4GgctZ2r7U5MWlmm1UogF60kEZIZCQykz0yJnhTpP9nMFDKw6Maki/mcfVwTQ/v3RqjzttHei/aaEMGQXYr3TOX8NPYJXerBpjShDZqcgcCfqPzhi7uCwVYaPvRsMMYH5jgtMz/SO3J6ToeGKT0f1F1ihRyCdusg325TMK7Vv8yHJzL4ngeUbb/oqxWtxhLY7f77Lkx6hBwe6ROpTqiQHIfZad5qyIHihKcDcIHo4FwLHVzxR4y+Ds2js5dXwBkUBaEAx0Dmc6ZdgXqxd815GDwv+VVpzXJhtO6qXmKE36F/Oe6eCNG3dk5MdOK4C2DrM9M8MXllqeQtX6JrM2stWlBalAc9Jn4b0xxAoghX3x3G3hNsLc9Uist1VinY2+IcPozusqj66c+Vbi5YngWSbICOjlIcq2GaOlBi3xioGts396442ztf1eLjMVuvykCI3b4Yz/vJYZOZjUvG/6nLfaFQTJLSXkTvI4do3foeY4=" - password: - secure: "ts/3FSPL+w4euXryip9wCJWUqUwuJ2CqHi0nEpYVM10KL0cp9aMkQXN/7C8ih+j/JZ25kaO+tis1nx0JERVnKrg/1F/fJhxHTpakCP4OSIlhKRXDt7yUUHcKKt5QdM2XCKhApptP6RTpWL1vlc36CMrhdvTcMYp7hlDOiWjdhhim24FGQzabL42Ko59ttyyCROqgcnjHqj+WlhJl7iYO+lv2PLOtslpkIYY5EMjgBLuGyOvxTqZsAXtworsASdB2CrsTE6iMRcrM1afgkyBlUqDezoiv9TwfZrZ85oRoAxPB7su4QWK7lEP0wGQV/SDz/RL7c+cNfRaauxUKVcC8I2LfaEpudv8vi205+v/08wzPbS4xSOnf89irUJnyGxUy+bYLoXPPVPMyzcQRAUnrQl+ENCmN62HU+/2pkIyiUJvt0wYYP0vp+3pIbXbYQGmAIMq7nZJd7K3rikfnQIGKI03xZsFe3PEdjzhDI9Ej/tRUbJx4pgg084QuYYca6MSYn/zJ5pqIG4A7ngCSxoxoWZT0duo8PrUWQeT17L73Q86HeEv3OzZhWfEF2fNDN3Qbsu4CllSZnbE7DXgAuysTwTYHS/na6lp7wejOqdRUXrHbidVla9Mwxyc9JdWNWzUDxALvNxAdNbV6l85mO8izy8wjQF6M1O7idqTfjQeR4pc=" - skip_existing: true - file_glob: true - file: - - dist/dvc*.whl - - dist/dvc*.tar.gz - skip_cleanup: true - on: - tags: true - repo: iterative/dvc - name: Snapcraft snap - stage: deploy addons: snaps: - name: snapcraft @@ -117,14 +92,6 @@ jobs: - sudo snapcraft --use-lxd after_failure: - sudo journalctl -u snapd - deploy: - - provider: snap - snap: dvc_*.snap - channel: ${SOURCE_TAG:+stable}${SOURCE_TAG:-edge} - skip_cleanup: true - on: - branch: master - repo: iterative/dvc before_install: - bash ./scripts/ci/before_install.sh @@ -136,7 +103,6 @@ script: - bash ./scripts/ci/script.sh deploy: - provider: releases - condition: "[[ $(./scripts/ci/deploy_condition.sh) == 'true' ]]" api_key: secure: nyo17gF4Xl6DFL/KVkMewC805Cv+eK99L7SiYCU+WUAYDIzzFL3YJm7fm8bznPeor7DmpO7uSLSveMnJETBtdurW6JWhCTUss4yD66yFzpk6Z5cvHf/1UA5pdMHkkMM54fRaTRt6L2Yal7RPwGEQTBECLXW1Xmf78Y6+0l8lTvXQ9jXOQSRO/63VDKI5Zww2+XytVicUFlr0hLMGKUuaCjjXcQlAPzBgQz3/5Iz6EtzeTf2o+gTPjocbRbSFPikfiBXUn8XlJ8jBj7i114XJRJffw59KxYGDzxp+enfLBULGNoW0Np2hZqkKkPUIdB08C8XVbRXlFYX8cEYBczjqhv8A05p5F5CnltnqQ0KazcoUP2awPVqn68dWNRF+kqrnMBus8euYKIZ4dy/6bVEKJRa84ESJXZVDcwXTAXRqyovSxOlXqg9ZgAImMKBfKn/fpDX2xIvQPaXcnJxKbmcePfdbC1d0HYJ5jkbNaxKE/T15VvhzIho7mJ2WAv1r8QouQUuzqtpzIGbRn/KXHRUzCtyP0k4+eMKMLFOPzg848WsDarP3v8VYiSy4nFY7ZP7DRd3KttvW5bgVbSeO4jPmTgS6TnnpRwual9BJXSG7bjbfpIMii3bNZ7jWL4zpKT9Jfm8w9RGKLcX9HqkZ3+y60+LtYdhEfBvqsIlWQoIJBXg= file_glob: true @@ -147,6 +113,32 @@ deploy: - dvc*.exe skip_cleanup: true on: + condition: "$(./scripts/ci/deploy_condition.sh dvc*.rpm dvc*.deb dvc*.pkg dvc*.exe)" tags: true repo: iterative/dvc stage: build + - provider: pypi + user: + secure: "IOzNfgPoG3ICg3+s4AuZCCnawgd/KqMzeiAB9R0B9Xb6ProPQJp9O3GBZrtg5OJMHdWQQtwULeOh/LWk4sFdXMcX1iNEgm5Tb7DwO7TDtltW14jUgN9AqfY2twj4UIsW/ON8w0P8v36ZoQq1kO6Pfgjijdm3w+9h6rpps8rJ6jXYpVZVO+SSzzijWddanMkEz4GgctZ2r7U5MWlmm1UogF60kEZIZCQykz0yJnhTpP9nMFDKw6Maki/mcfVwTQ/v3RqjzttHei/aaEMGQXYr3TOX8NPYJXerBpjShDZqcgcCfqPzhi7uCwVYaPvRsMMYH5jgtMz/SO3J6ToeGKT0f1F1ihRyCdusg325TMK7Vv8yHJzL4ngeUbb/oqxWtxhLY7f77Lkx6hBwe6ROpTqiQHIfZad5qyIHihKcDcIHo4FwLHVzxR4y+Ds2js5dXwBkUBaEAx0Dmc6ZdgXqxd815GDwv+VVpzXJhtO6qXmKE36F/Oe6eCNG3dk5MdOK4C2DrM9M8MXllqeQtX6JrM2stWlBalAc9Jn4b0xxAoghX3x3G3hNsLc9Uist1VinY2+IcPozusqj66c+Vbi5YngWSbICOjlIcq2GaOlBi3xioGts396442ztf1eLjMVuvykCI3b4Yz/vJYZOZjUvG/6nLfaFQTJLSXkTvI4do3foeY4=" + password: + secure: "ts/3FSPL+w4euXryip9wCJWUqUwuJ2CqHi0nEpYVM10KL0cp9aMkQXN/7C8ih+j/JZ25kaO+tis1nx0JERVnKrg/1F/fJhxHTpakCP4OSIlhKRXDt7yUUHcKKt5QdM2XCKhApptP6RTpWL1vlc36CMrhdvTcMYp7hlDOiWjdhhim24FGQzabL42Ko59ttyyCROqgcnjHqj+WlhJl7iYO+lv2PLOtslpkIYY5EMjgBLuGyOvxTqZsAXtworsASdB2CrsTE6iMRcrM1afgkyBlUqDezoiv9TwfZrZ85oRoAxPB7su4QWK7lEP0wGQV/SDz/RL7c+cNfRaauxUKVcC8I2LfaEpudv8vi205+v/08wzPbS4xSOnf89irUJnyGxUy+bYLoXPPVPMyzcQRAUnrQl+ENCmN62HU+/2pkIyiUJvt0wYYP0vp+3pIbXbYQGmAIMq7nZJd7K3rikfnQIGKI03xZsFe3PEdjzhDI9Ej/tRUbJx4pgg084QuYYca6MSYn/zJ5pqIG4A7ngCSxoxoWZT0duo8PrUWQeT17L73Q86HeEv3OzZhWfEF2fNDN3Qbsu4CllSZnbE7DXgAuysTwTYHS/na6lp7wejOqdRUXrHbidVla9Mwxyc9JdWNWzUDxALvNxAdNbV6l85mO8izy8wjQF6M1O7idqTfjQeR4pc=" + skip_existing: true + file_glob: true + file: + - dist/dvc*.whl + - dist/dvc*.tar.gz + skip_cleanup: true + on: + condition: "$(./scripts/ci/deploy_condition.sh dist/dvc*.whl dist/dvc*.tar.gz)" + tags: true + repo: iterative/dvc + stage: build + - provider: snap + snap: dvc_*.snap + channel: ${TRAVIS_TAG:+stable}${TRAVIS_TAG:-edge} + skip_cleanup: true + on: + all_branches: true + condition: "$(./scripts/ci/deploy_condition.sh dvc_*.snap) && ($TRAVIS_BRANCH = master || -n $TRAVIS_TAG)" + repo: iterative/dvc + stage: build diff --git a/dvc/api.py b/dvc/api.py index 00bd312a5..bac8c99da 100644 --- a/dvc/api.py +++ b/dvc/api.py @@ -2,7 +2,6 @@ from builtins import open as builtin_open import importlib import os import sys -from urllib.parse import urlparse from contextlib import contextmanager, _GeneratorContextManager as GCM import threading @@ -11,7 +10,7 @@ import ruamel.yaml from voluptuous import Schema, Required, Invalid from dvc.repo import Repo -from dvc.exceptions import DvcException +from dvc.exceptions import DvcException, NotDvcRepoError from dvc.external_repo import external_repo @@ -44,16 +43,30 @@ class SummonError(DvcException): pass +class UrlNotDvcRepoError(DvcException): + """Thrown if given url is not a DVC repository. + + Args: + url (str): url to the repository. + """ + + def __init__(self, url): + super().__init__("URL '{}' is not a dvc repository.".format(url)) + + def get_url(path, repo=None, rev=None, remote=None): """ Returns the full URL to the data artifact specified by its `path` in a `repo`. """ - with _make_repo(repo, rev=rev) as _repo: - abspath = os.path.join(_repo.root_dir, path) - out, = _repo.find_outs_by_path(abspath) - remote_obj = _repo.cloud.get_remote(remote) - return str(remote_obj.checksum_to_path_info(out.checksum)) + try: + with _make_repo(repo, rev=rev) as _repo: + abspath = os.path.join(_repo.root_dir, path) + out, = _repo.find_outs_by_path(abspath) + remote_obj = _repo.cloud.get_remote(remote) + return str(remote_obj.checksum_to_path_info(out.checksum)) + except NotDvcRepoError: + raise UrlNotDvcRepoError(repo) def open(path, repo=None, rev=None, remote=None, mode="r", encoding=None): @@ -99,7 +112,7 @@ def read(path, repo=None, rev=None, remote=None, mode="r", encoding=None): @contextmanager def _make_repo(repo_url, rev=None): - if not repo_url or urlparse(repo_url).scheme == "": + if not repo_url or os.path.exists(repo_url): assert ( rev is None ), "Git revisions are not supported for local DVC projects." diff --git a/dvc/command/get.py b/dvc/command/get.py index 22856f1c0..1386d8ca4 100644 --- a/dvc/command/get.py +++ b/dvc/command/get.py @@ -10,7 +10,27 @@ logger = logging.getLogger(__name__) class CmdGet(CmdBaseNoRepo): + def _show_url(self): + from dvc.api import get_url + + try: + url = get_url( + self.args.path, repo=self.args.url, rev=self.args.rev + ) + logger.info(url) + except DvcException: + logger.exception("failed to show url") + return 1 + + return 0 + def run(self): + if self.args.show_url: + return self._show_url() + + return self._get_file_from_repo() + + def _get_file_from_repo(self): from dvc.repo import Repo try: @@ -55,4 +75,9 @@ def add_parser(subparsers, parent_parser): get_parser.add_argument( "--rev", nargs="?", help="Git revision (e.g. branch, tag, SHA)" ) + get_parser.add_argument( + "--show-url", + action="store_true", + help="Returns path/url to the location in remote for specified path", + ) get_parser.set_defaults(func=CmdGet) diff --git a/dvc/command/metrics.py b/dvc/command/metrics.py index a2bc2587a..ef79392fb 100644 --- a/dvc/command/metrics.py +++ b/dvc/command/metrics.py @@ -26,7 +26,12 @@ def show_metrics(metrics, all_branches=False, all_tags=False): logger.info("{branch}:".format(branch=branch)) for fname, metric in val.items(): - lines = metric if type(metric) is list else metric.splitlines() + if isinstance(metric, dict): + lines = list(metric.values()) + elif isinstance(metric, list): + lines = metric + else: + lines = metric.splitlines() if len(lines) > 1: logger.info("\t{fname}:".format(fname=fname)) @@ -100,6 +105,59 @@ class CmdMetricsRemove(CmdBase): return 0 +def _show_diff(diff): + from texttable import Texttable + + if not diff: + return "No changes." + + table = Texttable() + + # remove borders to make it easier for users to copy stuff + table.set_chars(("", "", "", "")) + table.set_deco(0) + + rows = [["Path", "Metric", "Value", "Change"]] + for fname, mdiff in diff.items(): + for metric, change in mdiff.items(): + rows.append( + [ + fname, + metric, + change["new"], + change.get("diff", "diff not supported"), + ] + ) + table.add_rows(rows) + return table.draw() + + +class CmdMetricsDiff(CmdBase): + def run(self): + try: + diff = self.repo.metrics.diff( + a_ref=self.args.a_ref, + b_ref=self.args.b_ref, + targets=self.args.targets, + typ=self.args.type, + xpath=self.args.xpath, + recursive=self.args.recursive, + ) + + if self.args.show_json: + import json + + logger.info(json.dumps(diff)) + else: + logger.info(_show_diff(diff)) + + except DvcException: + logger.exception("failed to show metrics diff") + return 1 + + return 0 + + def add_parser(subparsers, parent_parser): METRICS_HELP = "Commands to add, manage, collect and display metrics." @@ -214,3 +272,65 @@ def add_parser(subparsers, parent_parser): ) metrics_remove_parser.add_argument("path", help="Path to a metric file.") metrics_remove_parser.set_defaults(func=CmdMetricsRemove) + + METRICS_DIFF_HELP = "Output metric values." + metrics_diff_parser = metrics_subparsers.add_parser( + "diff", + parents=[parent_parser], + description=append_doc_link(METRICS_DIFF_HELP, "metrics/diff"), + help=METRICS_DIFF_HELP, + formatter_class=argparse.RawDescriptionHelpFormatter, + ) + metrics_diff_parser.add_argument( + "a_ref", + nargs="?", + help=( + "Git reference from which diff is calculated. " + "If omitted `HEAD`(latest commit) is used." + ), + ) + metrics_diff_parser.add_argument( + "b_ref", + nargs="?", + help=( + "Git reference to which diff is calculated. " + "If omitted current working tree is used." + ), + ) + metrics_diff_parser.add_argument( + "--targets", + nargs="*", + help=( + "Metric files or directories (see -R) to show diff for. " + "Shows diff for all metric files by default." + ), + ) + metrics_diff_parser.add_argument( + "-t", + "--type", + help=( + "Type of metrics (json/tsv/htsv/csv/hcsv). " + "It can be detected by the file extension automatically. " + "Unsupported types will be treated as raw." + ), + ) + metrics_diff_parser.add_argument( + "-x", "--xpath", help="json/tsv/htsv/csv/hcsv path." + ) + metrics_diff_parser.add_argument( + "-R", + "--recursive", + action="store_true", + default=False, + help=( + "If any target is a directory, recursively search and process " + "metric files." + ), + ) + metrics_diff_parser.add_argument( + "--show-json", + action="store_true", + default=False, + help="Show output in JSON format.", + ) + metrics_diff_parser.set_defaults(func=CmdMetricsDiff) diff --git a/dvc/config.py b/dvc/config.py index ad2070939..0fd312aa1 100644 --- a/dvc/config.py +++ b/dvc/config.py @@ -186,6 +186,7 @@ class Config(object): # pylint: disable=too-many-instance-attributes SECTION_GDRIVE_CLIENT_SECRET = "gdrive_client_secret" SECTION_GDRIVE_USER_CREDENTIALS_FILE = "gdrive_user_credentials_file" + SECTION_REMOTE_CHECKSUM_JOBS = "checksum_jobs" SECTION_REMOTE_REGEX = r'^\s*remote\s*"(?P.*)"\s*$' SECTION_REMOTE_FMT = 'remote "{}"' SECTION_REMOTE_URL = "url" @@ -214,6 +215,7 @@ class Config(object): # pylint: disable=too-many-instance-attributes SECTION_GCP_PROJECTNAME: str, SECTION_CACHE_TYPE: supported_cache_type, Optional(SECTION_CACHE_PROTECTED, default=False): Bool, + SECTION_REMOTE_CHECKSUM_JOBS: All(Coerce(int), Range(1)), SECTION_REMOTE_USER: str, SECTION_REMOTE_PORT: Coerce(int), SECTION_REMOTE_KEY_FILE: str, diff --git a/dvc/dependency/repo.py b/dvc/dependency/repo.py index 57244bc4f..3bcfd3e65 100644 --- a/dvc/dependency/repo.py +++ b/dvc/dependency/repo.py @@ -9,8 +9,11 @@ from dvc.external_repo import cached_clone from dvc.external_repo import external_repo from dvc.exceptions import NotDvcRepoError from dvc.exceptions import OutputNotFoundError +from dvc.exceptions import NoOutputInExternalRepoError from dvc.exceptions import PathMissingError from dvc.utils.fs import fs_copy +from dvc.path_info import PathInfo +from dvc.scm import SCM class DependencyREPO(DependencyLOCAL): @@ -45,14 +48,31 @@ class DependencyREPO(DependencyLOCAL): with external_repo(**merge(self.def_repo, overrides)) as repo: yield repo + def _get_checksum(self, updated=False): + rev_lock = None + if not updated: + rev_lock = self.def_repo.get(self.PARAM_REV_LOCK) + + try: + with self._make_repo(rev_lock=rev_lock) as repo: + return repo.find_out_by_relpath(self.def_path).info["md5"] + except (NotDvcRepoError, NoOutputInExternalRepoError): + # Fall through and clone + pass + + repo_path = cached_clone( + self.def_repo[self.PARAM_URL], + rev=rev_lock or self.def_repo.get(self.PARAM_REV), + ) + path = PathInfo(os.path.join(repo_path, self.def_path)) + + return self.repo.cache.local.get_checksum(path) + def status(self): - with self._make_repo() as repo: - current = repo.find_out_by_relpath(self.def_path).info + current_checksum = self._get_checksum(updated=False) + updated_checksum = self._get_checksum(updated=True) - with self._make_repo(rev_lock=None) as repo: - updated = repo.find_out_by_relpath(self.def_path).info - - if current != updated: + if current_checksum != updated_checksum: return {str(self): "update available"} return {} @@ -105,6 +125,7 @@ class DependencyREPO(DependencyLOCAL): src_full_path = os.path.join(repo_dir, src_path) dst_full_path = os.path.abspath(to_path) fs_copy(src_full_path, dst_full_path) + self.def_repo[self.PARAM_REV_LOCK] = SCM(repo_dir).get_rev() return True def download(self, to): diff --git a/dvc/main.py b/dvc/main.py index 1a29f5874..829908af6 100644 --- a/dvc/main.py +++ b/dvc/main.py @@ -10,11 +10,12 @@ from dvc.exceptions import DvcParserError from dvc.exceptions import NotDvcRepoError from dvc.external_repo import clean_repos from dvc.logger import FOOTER - +from dvc.utils import format_link # Workaround for CPython bug. See [1] and [2] for more info. # [1] https://github.com/aws/aws-cli/blob/1.16.277/awscli/clidriver.py#L55 # [2] https://bugs.python.org/issue29288 + "".encode("idna") @@ -59,7 +60,11 @@ def main(argv=None): except Exception as exc: # pylint: disable=broad-except if isinstance(exc, OSError) and exc.errno == errno.EMFILE: logger.exception( - "too many open files, please increase your `ulimit`", + "too many open files, please visit " + "{} to see how to handle this " + "problem".format( + format_link("https://error.dvc.org/many-files") + ), extra={"tb_only": True}, ) else: diff --git a/dvc/remote/base.py b/dvc/remote/base.py index 36409645e..9f47779c8 100644 --- a/dvc/remote/base.py +++ b/dvc/remote/base.py @@ -86,11 +86,7 @@ class RemoteBASE(object): self.repo = repo self._check_requires(config) - - core = config.get(Config.SECTION_CORE, {}) - self.checksum_jobs = core.get( - Config.SECTION_CORE_CHECKSUM_JOBS, self.CHECKSUM_JOBS - ) + self.checksum_jobs = self._get_checksum_jobs(config) self.protected = False self.no_traverse = config.get(Config.SECTION_REMOTE_NO_TRAVERSE, True) @@ -142,6 +138,19 @@ class RemoteBASE(object): ).format(url, missing, " ".join(missing), self.scheme) raise RemoteMissingDepsError(msg) + def _get_checksum_jobs(self, config): + checksum_jobs = config.get(Config.SECTION_REMOTE_CHECKSUM_JOBS) + if checksum_jobs: + return checksum_jobs + + if self.repo: + core = self.repo.config.config.get(Config.SECTION_CORE, {}) + return core.get( + Config.SECTION_CORE_CHECKSUM_JOBS, self.CHECKSUM_JOBS + ) + + return self.CHECKSUM_JOBS + def __repr__(self): return "{class_name}: '{path_info}'".format( class_name=type(self).__name__, diff --git a/dvc/remote/config.py b/dvc/remote/config.py index 87572bd1d..3401d3e82 100644 --- a/dvc/remote/config.py +++ b/dvc/remote/config.py @@ -3,8 +3,7 @@ import os import posixpath from urllib.parse import urlparse -from dvc.config import Config -from dvc.config import ConfigError +from dvc.config import Config, ConfigError from dvc.utils import relpath @@ -134,6 +133,9 @@ class RemoteConfig(object): break def modify(self, name, option, value, level=None): + # try to get remote settings to verify that section exists + self.get_settings(name) + self.config.set( Config.SECTION_REMOTE_FMT.format(name), option, value, level=level ) diff --git a/dvc/remote/gdrive.py b/dvc/remote/gdrive.py index 06e4f0814..aa87f1bbd 100644 --- a/dvc/remote/gdrive.py +++ b/dvc/remote/gdrive.py @@ -34,7 +34,7 @@ class GDriveMissedCredentialKeyError(DvcException): @decorator def _wrap_pydrive_retriable(call): from apiclient import errors - from pydrive.files import ApiRequestError + from pydrive2.files import ApiRequestError try: result = call() @@ -61,7 +61,7 @@ gdrive_retry = compose( class RemoteGDrive(RemoteBASE): scheme = Schemes.GDRIVE path_cls = CloudURLInfo - REQUIRES = {"pydrive": "pydrive"} + REQUIRES = {"pydrive2": "pydrive2"} GDRIVE_USER_CREDENTIALS_DATA = "GDRIVE_USER_CREDENTIALS_DATA" DEFAULT_USER_CREDENTIALS_FILE = "gdrive-user-credentials.json" @@ -177,11 +177,11 @@ class RemoteGDrive(RemoteBASE): @property @wrap_with(threading.RLock()) def drive(self): - from pydrive.auth import RefreshError + from pydrive2.auth import RefreshError if not hasattr(self, "_gdrive"): - from pydrive.auth import GoogleAuth - from pydrive.drive import GoogleDrive + from pydrive2.auth import GoogleAuth + from pydrive2.drive import GoogleDrive if os.getenv(RemoteGDrive.GDRIVE_USER_CREDENTIALS_DATA): with open( @@ -227,7 +227,7 @@ class RemoteGDrive(RemoteBASE): self.gdrive_user_credentials_path, str(exc) ) ) - # Handle pydrive.auth.AuthenticationError and others auth failures + # Handle pydrive2.auth.AuthenticationError and others auth failures except Exception as exc: raise DvcException( "Google Drive authentication failed" @@ -261,8 +261,11 @@ class RemoteGDrive(RemoteBASE): def get_remote_item(self, name, parents_ids): if not parents_ids: return None - query = " or ".join( - "'{}' in parents".format(parent_id) for parent_id in parents_ids + query = "({})".format( + " or ".join( + "'{}' in parents".format(parent_id) + for parent_id in parents_ids + ) ) query += " and trashed=false and title='{}'".format(name) @@ -331,8 +334,10 @@ class RemoteGDrive(RemoteBASE): if not self.cached_ids: return - query = " or ".join( - "'{}' in parents".format(dir_id) for dir_id in self.cached_ids + query = "({})".format( + " or ".join( + "'{}' in parents".format(dir_id) for dir_id in self.cached_ids + ) ) query += " and trashed=false" diff --git a/dvc/remote/s3.py b/dvc/remote/s3.py index ba043afcd..bcc1cffb8 100644 --- a/dvc/remote/s3.py +++ b/dvc/remote/s3.py @@ -3,6 +3,7 @@ import logging import os import threading +from boto3.s3.transfer import TransferConfig from funcy import cached_property, wrap_prop @@ -169,7 +170,13 @@ class RemoteS3(RemoteBASE): ) else: source = {"Bucket": from_info.bucket, "Key": from_info.path} - s3.copy(source, to_info.bucket, to_info.path, ExtraArgs=extra_args) + s3.copy( + source, + to_info.bucket, + to_info.path, + ExtraArgs=extra_args, + Config=TransferConfig(multipart_threshold=size + 1), + ) cached_etag = cls.get_etag(s3, to_info.bucket, to_info.path) if etag != cached_etag: diff --git a/dvc/repo/brancher.py b/dvc/repo/brancher.py index 8b096726c..f64bf41b7 100644 --- a/dvc/repo/brancher.py +++ b/dvc/repo/brancher.py @@ -4,7 +4,7 @@ from dvc.scm.tree import WorkingTree def brancher( # noqa: E302 - self, all_branches=False, all_tags=False, all_commits=False + self, revs=None, all_branches=False, all_tags=False, all_commits=False ): """Generator that iterates over specified revisions. @@ -20,12 +20,12 @@ def brancher( # noqa: E302 - empty string it there is no branches to iterate over - "Working Tree" if there are uncommitted changes in the SCM repo """ - if not any([all_branches, all_tags, all_commits]): + if not any([revs, all_branches, all_tags, all_commits]): yield "" return saved_tree = self.tree - revs = [] + revs = revs or [] scm = self.scm diff --git a/dvc/repo/metrics/__init__.py b/dvc/repo/metrics/__init__.py index 8904a00b9..7fce413da 100644 --- a/dvc/repo/metrics/__init__.py +++ b/dvc/repo/metrics/__init__.py @@ -21,3 +21,8 @@ class Metrics(object): from dvc.repo.metrics.remove import remove return remove(self.repo, *args, **kwargs) + + def diff(self, *args, **kwargs): + from .diff import diff + + return diff(self.repo, *args, **kwargs) diff --git a/dvc/repo/metrics/diff.py b/dvc/repo/metrics/diff.py new file mode 100644 index 000000000..aa056cadc --- /dev/null +++ b/dvc/repo/metrics/diff.py @@ -0,0 +1,105 @@ +import json +from collections import defaultdict + +from flatten_dict import flatten + +from dvc.exceptions import NoMetricsError + + +def _parse(raw): + if isinstance(raw, (dict, list, int, float)): + return raw + + assert isinstance(raw, str) + try: + return json.loads(raw) + except json.JSONDecodeError: + return raw + + +def _diff_vals(old, new): + if ( + isinstance(new, list) + and isinstance(old, list) + and len(old) == len(new) == 1 + ): + return _diff_vals(old[0], new[0]) + + if old == new: + return {} + + res = {"old": old, "new": new} + if isinstance(new, (int, float)) and isinstance(old, (int, float)): + res["diff"] = new - old + return res + + +# dot_reducer is not released yet (flatten-dict > 0.2.0) +def _dot(k1, k2): + if k1 is None: + return k2 + return "{0}.{1}".format(k1, k2) + + +def _diff_dicts(old_dict, new_dict): + old_default = None + new_default = None + + if isinstance(new_dict, dict): + new = flatten(new_dict, reducer=_dot) + else: + new = defaultdict(lambda: "not a dict") + new_default = "unable to parse" + + if isinstance(old_dict, dict): + old = flatten(old_dict, reducer=_dot) + else: + old = defaultdict(lambda: "not a dict") + old_default = "unable to parse" + + res = defaultdict(dict) + + xpaths = set(old.keys()) + xpaths.update(set(new.keys())) + for xpath in xpaths: + old_val = old.get(xpath, old_default) + new_val = new.get(xpath, new_default) + val_diff = _diff_vals(old_val, new_val) + if val_diff: + res[xpath] = val_diff + return dict(res) + + +def _diff(old_raw, new_raw): + old = _parse(old_raw) + new = _parse(new_raw) + + if isinstance(new, dict) or isinstance(old, dict): + return _diff_dicts(old, new) + + return {"": _diff_vals(old, new)} + + +def _get_metrics(repo, *args, rev=None, **kwargs): + try: + metrics = repo.metrics.show( + *args, **kwargs, revs=[rev] if rev else None + ) + return metrics[rev or ""] + except NoMetricsError: + return {} + + +def diff(repo, *args, a_ref=None, b_ref=None, **kwargs): + old = _get_metrics(repo, *args, **kwargs, rev=(a_ref or "HEAD")) + new = _get_metrics(repo, *args, **kwargs, rev=b_ref) + + paths = set(old.keys()) + paths.update(set(new.keys())) + + res = defaultdict(dict) + for path in paths: + path_diff = _diff(old[path], new[path]) + if path_diff: + res[path] = path_diff + return dict(res) diff --git a/dvc/repo/metrics/show.py b/dvc/repo/metrics/show.py index 01df0e86d..9f0f3ef13 100644 --- a/dvc/repo/metrics/show.py +++ b/dvc/repo/metrics/show.py @@ -20,7 +20,7 @@ logger = logging.getLogger(__name__) def _read_metric_json(fd, json_path): parser = parse(json_path) - return [x.value for x in parser.find(json.load(fd))] + return {str(x.full_path): x.value for x in parser.find(json.load(fd))} def _get_values(row): @@ -266,6 +266,7 @@ def show( all_branches=False, all_tags=False, recursive=False, + revs=None, ): res = {} found = set() @@ -274,7 +275,9 @@ def show( # Iterate once to call `_collect_metrics` on all the stages targets = [None] - for branch in repo.brancher(all_branches=all_branches, all_tags=all_tags): + for branch in repo.brancher( + revs=revs, all_branches=all_branches, all_tags=all_tags + ): metrics = {} for target in targets: diff --git a/dvc/repo/status.py b/dvc/repo/status.py index 2e9a7a948..3528e6243 100644 --- a/dvc/repo/status.py +++ b/dvc/repo/status.py @@ -10,16 +10,11 @@ from . import locked logger = logging.getLogger(__name__) -def _local_status(self, targets=None, with_deps=False): +def _joint_status(stages): status = {} - if targets: - stages = cat(self.collect(t, with_deps=with_deps) for t in targets) - else: - stages = self.collect(None, with_deps=with_deps) - for stage in stages: - if stage.locked: + if stage.locked and not stage.is_repo_import: logger.warning( "DVC-file '{path}' is locked. Its dependencies are" " not going to be shown in the status output.".format( @@ -27,11 +22,20 @@ def _local_status(self, targets=None, with_deps=False): ) ) - status.update(stage.status()) + status.update(stage.status(check_updates=True)) return status +def _local_status(self, targets=None, with_deps=False): + if targets: + stages = cat(self.collect(t, with_deps=with_deps) for t in targets) + else: + stages = self.collect(None, with_deps=with_deps) + + return _joint_status(stages) + + def _cloud_status( self, targets=None, diff --git a/dvc/stage.py b/dvc/stage.py index 449cb261f..28076bad6 100644 --- a/dvc/stage.py +++ b/dvc/stage.py @@ -1006,10 +1006,12 @@ class Stage(object): return ret @rwlocked(read=["deps", "outs"]) - def status(self): + def status(self, check_updates=False): ret = [] - if not self.locked: + show_import = self.is_repo_import and check_updates + + if not self.locked or show_import: deps_status = self._status(self.deps) if deps_status: ret.append({"changed deps": deps_status}) diff --git a/dvc/system.py b/dvc/system.py index d32663b4a..c300daac5 100644 --- a/dvc/system.py +++ b/dvc/system.py @@ -1,14 +1,31 @@ import errno import logging import os +import platform import shutil from dvc.compat import fspath from dvc.exceptions import DvcException - logger = logging.getLogger(__name__) +if platform.system() == "Windows": + try: + import speedcopy + + speedcopy.patch_copyfile() + except ImportError: + pass +else: + import sys + + if sys.version_info < (3, 8): + try: + # Importing the module monkey-patches shutil.copyfile + import pyfastcopy # noqa: F401 + except ImportError: + pass + class System(object): @staticmethod @@ -90,8 +107,6 @@ class System(object): @staticmethod def reflink(source, link_name): - import platform - source, link_name = fspath(source), fspath(link_name) system = platform.system() diff --git a/dvc/utils/__init__.py b/dvc/utils/__init__.py index edb77e93b..bf02ecdb2 100644 --- a/dvc/utils/__init__.py +++ b/dvc/utils/__init__.py @@ -336,7 +336,7 @@ def env2bool(var, undefined=False): def resolve_output(inp, out): from urllib.parse import urlparse - name = os.path.basename(urlparse(inp).path) + name = os.path.basename(os.path.normpath(urlparse(inp).path)) if not out: return name if os.path.isdir(out): diff --git a/dvc/version.py b/dvc/version.py index a1ebd240b..a24e36253 100644 --- a/dvc/version.py +++ b/dvc/version.py @@ -7,7 +7,7 @@ import os import subprocess -_BASE_VERSION = "0.81.0" +_BASE_VERSION = "0.81.3" def _generate_version(base_version): @@ -49,7 +49,7 @@ def _is_release(dir_path, base_version): ["git", "describe", "--tags", "--exact-match"], cwd=dir_path, stderr=subprocess.STDOUT, - ) + ).decode("utf-8") tag = output.strip() return tag == base_version except subprocess.CalledProcessError: diff --git a/scripts/ci/deploy_condition.sh b/scripts/ci/deploy_condition.sh index 1446aec2e..9c2a600b5 100755 --- a/scripts/ci/deploy_condition.sh +++ b/scripts/ci/deploy_condition.sh @@ -2,9 +2,14 @@ set -e -if [[ $TRAVIS_OS_NAME == "osx" && $TRAVIS_OSX_IMAGE != "xcode8.3" ]]; then - echo false - exit 0 +if [[ $TRAVIS_EVENT_TYPE = pull_request || $TRAVIS_EVENT_TPYE = cron ]]; then + exit 2 +fi + +# positional args are assumed to be file glob patterns to deploy +if [[ ${#} -ge 1 ]]; then + # ensure at least one file exists + [[ -n "$(ls $@ 2>/dev/null)" ]] || exit 3 fi echo true diff --git a/setup.py b/setup.py index f8c4fda59..c3b5320c0 100644 --- a/setup.py +++ b/setup.py @@ -46,7 +46,6 @@ class build_py(_build_py): install_requires = [ "python-dateutil<2.8.1,>=2.1", # Consolidates azure-blob-storage and boto3 "ply>=3.9", # See https://github.com/pyinstaller/pyinstaller/issues/1945 - "configparser>=3.5.0", "colorama>=0.3.9", "configobj>=5.0.6", "gitpython>=2.1.8", @@ -75,6 +74,10 @@ install_requires = [ "win-unicode-console>=0.5; sys_platform == 'win32'", "pywin32>=225; sys_platform == 'win32'", "networkx>=2.1,<2.4", + "speedcopy>=2.0.1", + "pyfastcopy>=1.0.3", + "flatten-dict>=0.2.0", + "texttable>=0.5.2", ] @@ -84,10 +87,7 @@ gs = ["google-cloud-storage==1.19.0"] # google-api-python-client is internal dependency of pydrive. After merge of # https://github.com/gsuitedevs/PyDrive/pull/180 into pydrive's master, # usage of google-api-python-client can be removed from DVC. -gdrive = [ - "pydrive @ git+https://github.com/gsuitedevs/PyDrive@master#egg=pydrive", - "google-api-python-client>=1.2", -] +gdrive = ["pydrive2>=1.4.0", "google-api-python-client>=1.2"] s3 = ["boto3>=1.9.201"] azure = ["azure-storage-blob==2.1.0"] oss = ["oss2==2.6.1"] diff --git a/tests/conftest.py b/tests/conftest.py index 7ae4ed571..6e01739aa 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,12 +2,8 @@ import os import mockssh import pytest -from git import Repo -from git.exc import GitCommandNotFound from dvc.remote.ssh.connection import SSHConnection -from dvc.repo import Repo as DvcRepo -from .basic_env import TestDirFixture, TestDvcGitFixture, TestGitFixture from .dir_helpers import * # noqa @@ -31,49 +27,6 @@ def reset_loglevel(request, caplog): yield -# Wrap class like fixture as pytest-like one to avoid code duplication -@pytest.fixture -def repo_dir(): - old_fixture = TestDirFixture() - old_fixture.setUp() - try: - yield old_fixture - finally: - old_fixture.tearDown() - - -# NOTE: this duplicates code from GitFixture, -# would fix itself once class-based fixtures are removed -@pytest.fixture -def git(repo_dir): - # NOTE: handles EAGAIN error on BSD systems (osx in our case). - # Otherwise when running tests you might get this exception: - # - # GitCommandNotFound: Cmd('git') not found due to: - # OSError('[Errno 35] Resource temporarily unavailable') - retries = 5 - while True: - try: - git = Repo.init() - break - except GitCommandNotFound: - retries -= 1 - if not retries: - raise - - try: - git.index.add([repo_dir.CODE]) - git.index.commit("add code") - yield git - finally: - git.close() - - -@pytest.fixture -def dvc_repo(repo_dir): - yield DvcRepo.init(repo_dir._root_dir, no_scm=True) - - here = os.path.abspath(os.path.dirname(__file__)) user = "user" @@ -98,51 +51,9 @@ def ssh(ssh_server): yield SSHConnection(**ssh_server.test_creds) -@pytest.fixture -def erepo(repo_dir): - repo = TestDvcGitFixture() - repo.setUp() - try: - stage_foo = repo.dvc.add(repo.FOO)[0] - stage_bar = repo.dvc.add(repo.BAR)[0] - stage_data_dir = repo.dvc.add(repo.DATA_DIR)[0] - repo.dvc.scm.add([stage_foo.path, stage_bar.path, stage_data_dir.path]) - repo.dvc.scm.commit("init repo") - - repo.create("version", "master") - repo.dvc.add("version") - repo.dvc.scm.add([".gitignore", "version.dvc"]) - repo.dvc.scm.commit("master") - - repo.dvc.scm.checkout("branch", create_new=True) - os.unlink(os.path.join(repo.root_dir, "version")) - repo.create("version", "branch") - repo.dvc.add("version") - repo.dvc.scm.add([".gitignore", "version.dvc"]) - repo.dvc.scm.commit("branch") - - repo.dvc.scm.checkout("master") - - repo.dvc.scm.close() - repo.git.close() - - os.chdir(repo._saved_dir) - yield repo - finally: - repo.tearDown() - - @pytest.fixture(scope="session", autouse=True) def _close_pools(): from dvc.remote.pool import close_pools yield close_pools() - - -@pytest.fixture -def git_erepo(): - repo = TestGitFixture() - repo.setUp() - yield repo - repo.tearDown() diff --git a/tests/func/test_api.py b/tests/func/test_api.py index 6ba3aef47..b3f0c0aba 100644 --- a/tests/func/test_api.py +++ b/tests/func/test_api.py @@ -6,7 +6,7 @@ import ruamel.yaml import pytest from dvc import api -from dvc.api import SummonError +from dvc.api import SummonError, UrlNotDvcRepoError from dvc.compat import fspath from dvc.exceptions import FileMissingError from dvc.main import main @@ -51,6 +51,16 @@ def test_get_url_external(erepo_dir, remote_url): assert api.get_url("foo", repo=repo_url) == expected_url +def test_get_url_git_only_repo_throws_exception(tmp_dir, scm): + tmp_dir.scm_gen({"foo": "foo"}, commit="initial") + + with pytest.raises(UrlNotDvcRepoError) as exc_info: + api.get_url("foo", fspath(tmp_dir)) + + # On windows, `exc_info` has path escaped, eg: `C:\\\\Users\\\\travis` + assert fspath(tmp_dir) in str(exc_info).replace("\\\\", "\\") + + @pytest.mark.parametrize("remote_url", all_remote_params, indirect=True) def test_open(remote_url, tmp_dir, dvc): run_dvc("remote", "add", "-d", "upstream", remote_url) diff --git a/tests/func/test_data_cloud.py b/tests/func/test_data_cloud.py index 10fee7da3..748a7fe63 100644 --- a/tests/func/test_data_cloud.py +++ b/tests/func/test_data_cloud.py @@ -31,10 +31,11 @@ from tests.basic_env import TestDvc from tests.utils import spy from tests.remotes import ( - _should_test_gcp, - _should_test_hdfs, Azure, + GCP, GDrive, + HDFS, + Local, S3, SSHMocked, OSS, @@ -44,9 +45,6 @@ from tests.remotes import ( TEST_GDRIVE_CLIENT_ID, TEST_GDRIVE_CLIENT_SECRET, TEST_REMOTE, - get_gcp_url, - get_hdfs_url, - get_local_url, ) @@ -79,17 +77,19 @@ class TestDataCloudBase(TestDvc): def _get_cloud_class(self): return None - def _should_test(self): + @staticmethod + def should_test(): return False - def _get_url(self): - return "" + @staticmethod + def get_url(): + return NotImplementedError def _get_keyfile(self): return None def _ensure_should_run(self): - if not self._should_test(): + if not self.should_test(): raise SkipTest( "Test {} is disabled".format(self.__class__.__name__) ) @@ -97,7 +97,7 @@ class TestDataCloudBase(TestDvc): def _setup_cloud(self): self._ensure_should_run() - repo = self._get_url() + repo = self.get_url() keyfile = self._get_keyfile() config = copy.deepcopy(TEST_CONFIG) @@ -190,25 +190,16 @@ class TestDataCloudBase(TestDvc): self._test_cloud() -class TestRemoteS3(TestDataCloudBase): - def _should_test(self): - return S3.should_test() - - def _get_url(self): - return S3.get_url() - +class TestRemoteS3(S3, TestDataCloudBase): def _get_cloud_class(self): return RemoteS3 -class TestRemoteGDrive(TestDataCloudBase): - def _should_test(self): - return GDrive.should_test() - +class TestRemoteGDrive(GDrive, TestDataCloudBase): def _setup_cloud(self): self._ensure_should_run() - repo = self._get_url() + repo = self.get_url() config = copy.deepcopy(TEST_CONFIG) config[TEST_SECTION][Config.SECTION_REMOTE_URL] = repo @@ -223,21 +214,15 @@ class TestRemoteGDrive(TestDataCloudBase): self.assertIsInstance(self.cloud.get_remote(), self._get_cloud_class()) - def _get_url(self): - return GDrive.get_url() - def _get_cloud_class(self): return RemoteGDrive -class TestRemoteGS(TestDataCloudBase): - def _should_test(self): - return _should_test_gcp() - +class TestRemoteGS(GCP, TestDataCloudBase): def _setup_cloud(self): self._ensure_should_run() - repo = self._get_url() + repo = self.get_url() config = copy.deepcopy(TEST_CONFIG) config[TEST_SECTION][Config.SECTION_REMOTE_URL] = repo @@ -249,53 +234,27 @@ class TestRemoteGS(TestDataCloudBase): self.assertIsInstance(self.cloud.get_remote(), self._get_cloud_class()) - def _get_url(self): - return get_gcp_url() - def _get_cloud_class(self): return RemoteGS -class TestRemoteAZURE(TestDataCloudBase): - def _should_test(self): - return Azure.should_test() - - def _get_url(self): - return Azure.get_url() - +class TestRemoteAZURE(Azure, TestDataCloudBase): def _get_cloud_class(self): return RemoteAZURE -class TestRemoteOSS(TestDataCloudBase): - def _should_test(self): - return OSS.should_test() - - def _get_url(self): - return OSS.get_url() - +class TestRemoteOSS(OSS, TestDataCloudBase): def _get_cloud_class(self): return RemoteOSS -class TestRemoteLOCAL(TestDataCloudBase): - def _should_test(self): - return True - - def _get_url(self): - self.dname = get_local_url() - return self.dname - +class TestRemoteLOCAL(Local, TestDataCloudBase): def _get_cloud_class(self): return RemoteLOCAL - def test(self): - super().test() - self.assertTrue(os.path.isdir(self.dname)) - @pytest.mark.usefixtures("ssh_server") -class TestRemoteSSHMocked(TestDataCloudBase): +class TestRemoteSSHMocked(SSHMocked, TestDataCloudBase): @pytest.fixture(autouse=True) def setup_method_fixture(self, request, ssh_server): self.ssh_server = ssh_server @@ -304,7 +263,7 @@ class TestRemoteSSHMocked(TestDataCloudBase): def _setup_cloud(self): self._ensure_should_run() - repo = self._get_url() + repo = self.get_url() keyfile = self._get_keyfile() config = copy.deepcopy(TEST_CONFIG) @@ -316,27 +275,18 @@ class TestRemoteSSHMocked(TestDataCloudBase): self.assertIsInstance(self.cloud.get_remote(), self._get_cloud_class()) - def _get_url(self): + def get_url(self): user = self.ssh_server.test_creds["username"] - return SSHMocked.get_url(user, self.ssh_server.port) + return super().get_url(user, self.ssh_server.port) def _get_keyfile(self): return self.ssh_server.test_creds["key_filename"] - def _should_test(self): - return SSHMocked.should_test() - def _get_cloud_class(self): return RemoteSSH -class TestRemoteHDFS(TestDataCloudBase): - def _should_test(self): - return _should_test_hdfs() - - def _get_url(self): - return get_hdfs_url() - +class TestRemoteHDFS(HDFS, TestDataCloudBase): def _get_cloud_class(self): return RemoteHDFS @@ -346,6 +296,14 @@ class TestDataCloudCLIBase(TestDvc): ret = main(args) self.assertEqual(ret, 0) + @staticmethod + def should_test(): + return False + + @staticmethod + def get_url(): + raise NotImplementedError + def _test_cloud(self, remote=None): args = ["-v", "-j", "2"] if remote: @@ -406,59 +364,47 @@ class TestDataCloudCLIBase(TestDvc): self.assertTrue(os.path.isfile(self.FOO)) self.assertTrue(os.path.isdir(self.DATA_DIR)) - def _should_test(self): - return True - def _test(self): pass def test(self): - if not self._should_test(): + if not self.should_test(): raise SkipTest( "Test {} is disabled".format(self.__class__.__name__) ) self._test() -class TestRemoteLOCALCLI(TestDataCloudCLIBase): +class TestRemoteLOCALCLI(Local, TestDataCloudCLIBase): def _test(self): - url = get_local_url() + url = self.get_url() self.main(["remote", "add", TEST_REMOTE, url]) self._test_cloud(TEST_REMOTE) -class TestRemoteHDFSCLI(TestDataCloudCLIBase): - def _should_test(self): - return _should_test_hdfs() - +class TestRemoteHDFSCLI(HDFS, TestDataCloudCLIBase): def _test(self): - url = get_hdfs_url() + url = self.get_url() self.main(["remote", "add", TEST_REMOTE, url]) self._test_cloud(TEST_REMOTE) -class TestRemoteS3CLI(TestDataCloudCLIBase): - def _should_test(self): - return S3.should_test() - +class TestRemoteS3CLI(S3, TestDataCloudCLIBase): def _test(self): - url = S3.get_url() + url = self.get_url() self.main(["remote", "add", TEST_REMOTE, url]) self._test_cloud(TEST_REMOTE) -class TestRemoteGDriveCLI(TestDataCloudCLIBase): - def _should_test(self): - return GDrive.should_test() - +class TestRemoteGDriveCLI(GDrive, TestDataCloudCLIBase): def _test(self): - url = GDrive.get_url() + url = self.get_url() self.main(["remote", "add", TEST_REMOTE, url]) self.main( @@ -483,12 +429,9 @@ class TestRemoteGDriveCLI(TestDataCloudCLIBase): self._test_cloud(TEST_REMOTE) -class TestRemoteGSCLI(TestDataCloudCLIBase): - def _should_test(self): - return _should_test_gcp() - +class TestRemoteGSCLI(GCP, TestDataCloudCLIBase): def _test(self): - url = get_gcp_url() + url = self.get_url() self.main(["remote", "add", TEST_REMOTE, url]) self.main( @@ -504,24 +447,18 @@ class TestRemoteGSCLI(TestDataCloudCLIBase): self._test_cloud(TEST_REMOTE) -class TestRemoteAZURECLI(TestDataCloudCLIBase): - def _should_test(self): - return Azure.should_test() - +class TestRemoteAZURECLI(Azure, TestDataCloudCLIBase): def _test(self): - url = Azure.get_url() + url = self.get_url() self.main(["remote", "add", TEST_REMOTE, url]) self._test_cloud(TEST_REMOTE) -class TestRemoteOSSCLI(TestDataCloudCLIBase): - def _should_test(self): - return OSS.should_test() - +class TestRemoteOSSCLI(OSS, TestDataCloudCLIBase): def _test(self): - url = OSS.get_url() + url = self.get_url() self.main(["remote", "add", TEST_REMOTE, url]) @@ -547,7 +484,7 @@ class TestWarnOnOutdatedStage(TestDvc): self.assertEqual(ret, 0) def _test(self): - url = get_local_url() + url = Local.get_url() self.main(["remote", "add", "-d", TEST_REMOTE, url]) stage = self.dvc.run(outs=["bar"], cmd="echo bar > bar") @@ -573,18 +510,11 @@ class TestWarnOnOutdatedStage(TestDvc): self._test() -class TestRecursiveSyncOperations(TestDataCloudBase): +class TestRecursiveSyncOperations(Local, TestDataCloudBase): def main(self, args): ret = main(args) self.assertEqual(ret, 0) - def _get_url(self): - self.dname = get_local_url() - return self.dname - - def _should_test(self): - return True - def _get_cloud_class(self): return RemoteLOCAL @@ -661,7 +591,7 @@ class TestCheckSumRecalculation(TestDvc): with patch.object( RemoteLOCAL, "get_file_checksum", test_get_file_checksum ): - url = get_local_url() + url = Local.get_url() ret = main(["remote", "add", "-d", TEST_REMOTE, url]) self.assertEqual(ret, 0) ret = main(["config", "cache.type", "hardlink"]) diff --git a/tests/func/test_get.py b/tests/func/test_get.py index cdbbe4bbd..87d8ceea3 100644 --- a/tests/func/test_get.py +++ b/tests/func/test_get.py @@ -5,6 +5,7 @@ import pytest from dvc.cache import Cache from dvc.config import Config +from dvc.main import main from dvc.repo.get import GetDVCFileError, PathMissingError from dvc.repo import Repo from dvc.system import System @@ -174,8 +175,40 @@ def test_get_from_non_dvc_master(tmp_dir, erepo_dir, caplog): caplog.clear() dst = "file_imported" - with caplog.at_level(logging.INFO, logger="dvc"): + + # removing `git` import in conftest resulted in unexpected logs from + # that package, see https://github.com/iterative/dvc/issues/3167 + with caplog.at_level(logging.INFO, logger="git"), caplog.at_level( + logging.INFO, logger="dvc" + ): Repo.get(fspath(erepo_dir), "some_file", out=dst, rev="branch") assert caplog.text == "" assert (tmp_dir / dst).read_text() == "some_contents" + + +def test_get_url_positive(tmp_dir, erepo_dir, caplog): + with erepo_dir.chdir(): + erepo_dir.dvc_gen("foo", "foo") + + caplog.clear() + with caplog.at_level(logging.ERROR, logger="dvc"): + assert main(["get", fspath(erepo_dir), "foo", "--show-url"]) == 0 + assert caplog.text == "" + + +def test_get_url_not_existing(tmp_dir, erepo_dir, caplog): + with caplog.at_level(logging.ERROR, logger="dvc"): + assert ( + main(["get", fspath(erepo_dir), "not-existing-file", "--show-url"]) + == 1 + ) + assert "failed to show url" in caplog.text + + +def test_get_url_git_only_repo(tmp_dir, scm, caplog): + tmp_dir.scm_gen({"foo": "foo"}, commit="initial") + + with caplog.at_level(logging.ERROR): + assert main(["get", fspath(tmp_dir), "foo", "--show-url"]) == 1 + assert "failed to show url" in caplog.text diff --git a/tests/func/test_import.py b/tests/func/test_import.py index c3a2c7b00..380eaeb88 100644 --- a/tests/func/test_import.py +++ b/tests/func/test_import.py @@ -21,11 +21,15 @@ def test_import(tmp_dir, scm, dvc, erepo_dir): with erepo_dir.chdir(): erepo_dir.dvc_gen("foo", "foo content", commit="create foo") - dvc.imp(fspath(erepo_dir), "foo", "foo_imported") + stage = dvc.imp(fspath(erepo_dir), "foo", "foo_imported") assert os.path.isfile("foo_imported") assert (tmp_dir / "foo_imported").read_text() == "foo content" assert scm.repo.git.check_ignore("foo_imported") + assert stage.deps[0].def_repo == { + "url": fspath(erepo_dir), + "rev_lock": erepo_dir.scm.get_rev(), + } @pytest.mark.parametrize("src_is_dvc", [True, False]) @@ -39,13 +43,17 @@ def test_import_git_file(erepo_dir, tmp_dir, dvc, scm, src_is_dvc): erepo_dir.scm_gen({src: "hello"}, commit="add a regular file") - tmp_dir.dvc.imp(fspath(erepo_dir), src, dst) + stage = tmp_dir.dvc.imp(fspath(erepo_dir), src, dst) assert (tmp_dir / dst).is_file() assert filecmp.cmp( fspath(erepo_dir / src), fspath(tmp_dir / dst), shallow=False ) assert tmp_dir.scm.repo.git.check_ignore(fspath(tmp_dir / dst)) + assert stage.deps[0].def_repo == { + "url": fspath(erepo_dir), + "rev_lock": erepo_dir.scm.get_rev(), + } @pytest.mark.parametrize("src_is_dvc", [True, False]) @@ -59,22 +67,30 @@ def test_import_git_dir(erepo_dir, tmp_dir, dvc, scm, src_is_dvc): erepo_dir.scm_gen({src: {"file.txt": "hello"}}, commit="add a dir") - tmp_dir.dvc.imp(fspath(erepo_dir), src, dst) + stage = dvc.imp(fspath(erepo_dir), src, dst) assert (tmp_dir / dst).is_dir() trees_equal(fspath(erepo_dir / src), fspath(tmp_dir / dst)) assert tmp_dir.scm.repo.git.check_ignore(fspath(tmp_dir / dst)) + assert stage.deps[0].def_repo == { + "url": fspath(erepo_dir), + "rev_lock": erepo_dir.scm.get_rev(), + } def test_import_dir(tmp_dir, scm, dvc, erepo_dir): with erepo_dir.chdir(): erepo_dir.dvc_gen({"dir": {"foo": "foo content"}}, commit="create dir") - dvc.imp(fspath(erepo_dir), "dir", "dir_imported") + stage = dvc.imp(fspath(erepo_dir), "dir", "dir_imported") assert os.path.isdir("dir_imported") trees_equal(fspath(erepo_dir / "dir"), "dir_imported") assert scm.repo.git.check_ignore("dir_imported") + assert stage.deps[0].def_repo == { + "url": fspath(erepo_dir), + "rev_lock": erepo_dir.scm.get_rev(), + } def test_import_non_cached(erepo_dir, tmp_dir, dvc, scm): @@ -90,23 +106,34 @@ def test_import_non_cached(erepo_dir, tmp_dir, dvc, scm): erepo_dir.scm.add([fspath(erepo_dir / src)]) erepo_dir.scm.commit("add a non-cached output") - tmp_dir.dvc.imp(fspath(erepo_dir), src, dst) + stage = tmp_dir.dvc.imp(fspath(erepo_dir), src, dst) assert (tmp_dir / dst).is_file() assert filecmp.cmp( fspath(erepo_dir / src), fspath(tmp_dir / dst), shallow=False ) assert tmp_dir.scm.repo.git.check_ignore(dst) + assert stage.deps[0].def_repo == { + "url": fspath(erepo_dir), + "rev_lock": erepo_dir.scm.get_rev(), + } def test_import_rev(tmp_dir, scm, dvc, erepo_dir): + rev = None with erepo_dir.chdir(), erepo_dir.branch("branch", new=True): erepo_dir.dvc_gen("foo", "foo content", commit="create foo on branch") + rev = erepo_dir.scm.get_rev() - dvc.imp(fspath(erepo_dir), "foo", "foo_imported", rev="branch") + stage = dvc.imp(fspath(erepo_dir), "foo", "foo_imported", rev="branch") assert (tmp_dir / "foo_imported").read_text() == "foo content" assert scm.repo.git.check_ignore("foo_imported") + assert stage.deps[0].def_repo == { + "url": fspath(erepo_dir), + "rev": "branch", + "rev_lock": rev, + } def test_pull_imported_stage(tmp_dir, dvc, erepo_dir): diff --git a/tests/func/test_metrics.py b/tests/func/test_metrics.py index 8f69ec315..42043dbd8 100644 --- a/tests/func/test_metrics.py +++ b/tests/func/test_metrics.py @@ -4,6 +4,8 @@ import json import logging import os +import pytest + from dvc.exceptions import DvcException from dvc.exceptions import NoMetricsError from dvc.main import main @@ -113,9 +115,9 @@ class TestMetrics(TestMetricsBase): ["metric_json"], typ="json", xpath="branch", all_branches=True ) self.assertEqual(len(ret), 3) - self.assertSequenceEqual(ret["foo"]["metric_json"], ["foo"]) - self.assertSequenceEqual(ret["bar"]["metric_json"], ["bar"]) - self.assertSequenceEqual(ret["baz"]["metric_json"], ["baz"]) + self.assertSequenceEqual(ret["foo"]["metric_json"], {"branch": "foo"}) + self.assertSequenceEqual(ret["bar"]["metric_json"], {"branch": "bar"}) + self.assertSequenceEqual(ret["baz"]["metric_json"], {"branch": "baz"}) ret = self.dvc.metrics.show( ["metric_tsv"], typ="tsv", xpath="0,0", all_branches=True @@ -158,13 +160,13 @@ class TestMetrics(TestMetricsBase): self.assertEqual(len(ret), 1) self.assertSequenceEqual( ret["foo"]["metric_json_ext"], - [ - { + { + "metrics.[0]": { "dataset": "train", "deviation_mse": 0.173461, "value_mse": 0.421601, } - ], + }, ) self.assertRaises(KeyError, lambda: ret["bar"]) self.assertRaises(KeyError, lambda: ret["baz"]) @@ -723,9 +725,9 @@ class TestCachedMetrics(TestDvcGit): self.assertEqual( res, { - "master": {"metrics.json": ["master"]}, - "one": {"metrics.json": ["one"]}, - "two": {"metrics.json": ["two"]}, + "master": {"metrics.json": {"metrics": "master"}}, + "one": {"metrics.json": {"metrics": "one"}}, + "two": {"metrics.json": {"metrics": "two"}}, }, ) @@ -736,9 +738,9 @@ class TestCachedMetrics(TestDvcGit): self.assertEqual( res, { - "master": {"metrics.json": ["master"]}, - "one": {"metrics.json": ["one"]}, - "two": {"metrics.json": ["two"]}, + "master": {"metrics.json": {"metrics": "master"}}, + "one": {"metrics.json": {"metrics": "one"}}, + "two": {"metrics.json": {"metrics": "two"}}, }, ) @@ -802,6 +804,10 @@ class TestMetricsType(TestDvcGit): for branch in self.branches: if isinstance(ret[branch][file_name], list): self.assertSequenceEqual(ret[branch][file_name], [branch]) + elif isinstance(ret[branch][file_name], dict): + self.assertSequenceEqual( + ret[branch][file_name], {"branch": branch} + ) else: self.assertSequenceEqual(ret[branch][file_name], branch) @@ -834,7 +840,7 @@ def test_show_xpath_should_override_stage_xpath(tmp_dir, dvc): dvc.run(cmd="", overwrite=True, metrics=["metric"]) dvc.metrics.modify("metric", typ="json", xpath="m2") - assert dvc.metrics.show(xpath="m1") == {"": {"metric": [0.1]}} + assert dvc.metrics.show(xpath="m1") == {"": {"metric": {"m1": 0.1}}} def test_show_multiple_outputs(tmp_dir, dvc, caplog): @@ -871,3 +877,66 @@ def test_show_multiple_outputs(tmp_dir, dvc, caplog): "the following metrics do not exist, " "are not metric files or are malformed: 'not-found'" ) in caplog.text + + +def test_metrics_diff_raw(tmp_dir, scm, dvc): + def _gen(val): + tmp_dir.gen({"metrics": val}) + dvc.run(cmd="", metrics=["metrics"]) + dvc.scm.add(["metrics.dvc"]) + dvc.scm.commit(str(val)) + + _gen("raw 1") + _gen("raw 2") + _gen("raw 3") + + assert dvc.metrics.diff(a_ref="HEAD~2") == { + "metrics": {"": {"old": "raw 1", "new": "raw 3"}} + } + + +@pytest.mark.parametrize("xpath", [True, False]) +def test_metrics_diff_json(tmp_dir, scm, dvc, xpath): + def _gen(val): + metrics = {"a": {"b": {"c": val, "d": 1, "e": str(val)}}} + tmp_dir.gen({"m.json": json.dumps(metrics)}) + dvc.run(cmd="", metrics=["m.json"]) + dvc.metrics.modify("m.json", typ="json") + if xpath: + dvc.metrics.modify("m.json", xpath="a.b.c") + dvc.scm.add(["m.json.dvc"]) + dvc.scm.commit(str(val)) + + _gen(1) + _gen(2) + _gen(3) + + expected = {"m.json": {"a.b.c": {"old": 1, "new": 3, "diff": 2}}} + + if not xpath: + expected["m.json"]["a.b.e"] = {"old": "1", "new": "3"} + + assert expected == dvc.metrics.diff(a_ref="HEAD~2") + + +def test_metrics_diff_broken_json(tmp_dir, scm, dvc): + metrics = {"a": {"b": {"c": 1, "d": 1, "e": "3"}}} + tmp_dir.gen({"m.json": json.dumps(metrics)}) + dvc.run(cmd="", metrics_no_cache=["m.json"]) + dvc.scm.add(["m.json.dvc", "m.json"]) + dvc.scm.commit("add metrics") + + (tmp_dir / "m.json").write_text(json.dumps(metrics) + "ma\nlformed\n") + + assert dvc.metrics.diff() == { + "m.json": { + "a.b.c": {"old": 1, "new": "unable to parse"}, + "a.b.d": {"old": 1, "new": "unable to parse"}, + "a.b.e": {"old": "3", "new": "unable to parse"}, + } + } + + +def test_metrics_diff_no_metrics(tmp_dir, scm, dvc): + tmp_dir.scm_gen({"foo": "foo"}, commit="add foo") + assert dvc.metrics.diff(a_ref="HEAD~1") == {} diff --git a/tests/func/test_remote.py b/tests/func/test_remote.py index 777666bd9..c6d1cffe9 100644 --- a/tests/func/test_remote.py +++ b/tests/func/test_remote.py @@ -6,7 +6,7 @@ import configobj import pytest from mock import patch -from dvc.config import Config +from dvc.config import Config, ConfigError from dvc.exceptions import DownloadError, UploadError from dvc.main import main from dvc.path_info import PathInfo @@ -14,7 +14,7 @@ from dvc.remote import RemoteLOCAL, RemoteConfig from dvc.remote.base import RemoteBASE from dvc.compat import fspath from tests.basic_env import TestDvc -from tests.remotes import get_local_storagepath +from tests.remotes import Local class TestRemote(TestDvc): @@ -31,10 +31,10 @@ class TestRemote(TestDvc): self.assertEqual(main(["remote", "list"]), 0) - self.assertEqual(main(["remote", "remove", remotes[0]]), 0) self.assertEqual( main(["remote", "modify", remotes[0], "option", "value"]), 0 ) + self.assertEqual(main(["remote", "remove", remotes[0]]), 0) self.assertEqual(main(["remote", "list"]), 0) @@ -159,7 +159,7 @@ class TestRemoteShouldHandleUppercaseRemoteName(TestDvc): upper_case_remote_name = "UPPERCASEREMOTE" def test(self): - remote_url = get_local_storagepath() + remote_url = Local.get_storagepath() ret = main(["remote", "add", self.upper_case_remote_name, remote_url]) self.assertEqual(ret, 0) @@ -250,3 +250,10 @@ def test_raise_on_too_many_open_files(tmp_dir, dvc, tmp_path_factory, mocker): with pytest.raises(OSError) as e: dvc.push() assert e.errno == errno.EMFILE + + +def test_modify_missing_remote(dvc): + remote_config = RemoteConfig(dvc.config) + + with pytest.raises(ConfigError, match=r"unable to find remote section"): + remote_config.modify("myremote", "gdrive_client_id", "xxx") diff --git a/tests/func/test_repro.py b/tests/func/test_repro.py index 4a8435c5b..797e975e2 100644 --- a/tests/func/test_repro.py +++ b/tests/func/test_repro.py @@ -9,6 +9,7 @@ from pathlib import Path from subprocess import PIPE from subprocess import Popen from urllib.parse import urljoin +from unittest import SkipTest import boto3 import paramiko @@ -35,8 +36,9 @@ from dvc.utils.stage import dump_stage_file from dvc.utils.stage import load_stage_file from tests.basic_env import TestDvc from tests.remotes import ( - _should_test_gcp, - _should_test_hdfs, + GCP, + HDFS, + Local, S3, SSH, SSHMocked, @@ -806,7 +808,8 @@ class TestCmdReproChdir(TestDvc): class TestReproExternalBase(TestDvc): - def should_test(self): + @staticmethod + def should_test(): return False @property @@ -857,7 +860,9 @@ class TestReproExternalBase(TestDvc): @patch("dvc.prompt.confirm", return_value=True) def test(self, mock_prompt): if not self.should_test(): - return + raise SkipTest( + "Test {} is disabled".format(self.__class__.__name__) + ) cache = ( self.scheme @@ -953,10 +958,7 @@ class TestReproExternalBase(TestDvc): @pytest.mark.skipif(os.name == "nt", reason="temporarily disabled on windows") -class TestReproExternalS3(TestReproExternalBase): - def should_test(self): - return S3.should_test() - +class TestReproExternalS3(S3, TestReproExternalBase): @property def scheme(self): return "s3" @@ -973,10 +975,7 @@ class TestReproExternalS3(TestReproExternalBase): s3.put_object(Bucket=bucket, Key=key, Body=body) -class TestReproExternalGS(TestReproExternalBase): - def should_test(self): - return _should_test_gcp() - +class TestReproExternalGS(GCP, TestReproExternalBase): @property def scheme(self): return "gs" @@ -994,10 +993,7 @@ class TestReproExternalGS(TestReproExternalBase): bucket.blob(key).upload_from_string(body) -class TestReproExternalHDFS(TestReproExternalBase): - def should_test(self): - return _should_test_hdfs() - +class TestReproExternalHDFS(HDFS, TestReproExternalBase): @property def scheme(self): return "hdfs" @@ -1054,12 +1050,9 @@ class TestReproExternalHDFS(TestReproExternalBase): @flaky(max_runs=3, min_passes=1) -class TestReproExternalSSH(TestReproExternalBase): +class TestReproExternalSSH(SSH, TestReproExternalBase): _dir = None - def should_test(self): - return SSH.should_test() - @property def scheme(self): return "ssh" @@ -1067,7 +1060,7 @@ class TestReproExternalSSH(TestReproExternalBase): @property def bucket(self): if not self._dir: - self._dir = TestDvc.mkdtemp() + self._dir = self.mkdtemp() return "{}@127.0.0.1:{}".format(getpass.getuser(), self._dir) def cmd(self, i, o): @@ -1101,10 +1094,10 @@ class TestReproExternalSSH(TestReproExternalBase): fobj.write(body) -class TestReproExternalLOCAL(TestReproExternalBase): +class TestReproExternalLOCAL(Local, TestReproExternalBase): def setUp(self): super().setUp() - self.tmpdir = TestDvc.mkdtemp() + self.tmpdir = self.mkdtemp() ret = main(["config", "cache.type", "hardlink"]) self.assertEqual(ret, 0) self.dvc = DvcRepo(".") @@ -1113,9 +1106,6 @@ class TestReproExternalLOCAL(TestReproExternalBase): def cache_type(self): return "hardlink" - def should_test(self): - return True - @property def cache_scheme(self): return "local" diff --git a/tests/func/test_status.py b/tests/func/test_status.py index e4de03852..8b8d3d2ba 100644 --- a/tests/func/test_status.py +++ b/tests/func/test_status.py @@ -1,8 +1,12 @@ import os +import shutil from mock import patch +from dvc.repo import Repo from dvc.main import main +from dvc.compat import fspath +from dvc.external_repo import clean_repos from tests.basic_env import TestDvc @@ -23,3 +27,72 @@ class TestStatus(TestDvc): def test_implied_cloud(self, mock_status): main(["status", "--remote", "something"]) mock_status.assert_called() + + +def test_status_non_dvc_repo_import(tmp_dir, dvc, erepo_dir): + with erepo_dir.branch("branch", new=True), erepo_dir.chdir(): + erepo_dir.scm.repo.index.remove([".dvc"], r=True) + shutil.rmtree(".dvc") + erepo_dir.scm_gen("file", "first version") + erepo_dir.scm.add(["file"]) + erepo_dir.scm.commit("first version") + + dvc.imp(fspath(erepo_dir), "file", "file", rev="branch") + + status = dvc.status(["file.dvc"]) + + assert status == {} + + # Caching in external repos doesn't see upstream updates within single + # cli call, so we need to clean the caches to see the changes. + clean_repos() + + with erepo_dir.branch("branch", new=False), erepo_dir.chdir(): + erepo_dir.scm_gen("file", "second_version", commit="update file") + erepo_dir.scm.add(["file"]) + erepo_dir.scm.commit("first version") + + status, = dvc.status(["file.dvc"])["file.dvc"] + + assert status == { + "changed deps": { + "file ({})".format(fspath(erepo_dir)): "update available" + } + } + + +def test_status_before_and_after_dvc_init(tmp_dir, dvc, erepo_dir): + with erepo_dir.chdir(): + erepo_dir.scm.repo.index.remove([".dvc"], r=True) + shutil.rmtree(".dvc") + erepo_dir.scm_gen("file", "first version") + erepo_dir.scm.add(["file"]) + erepo_dir.scm.commit("first version") + old_rev = erepo_dir.scm.get_rev() + + dvc.imp(fspath(erepo_dir), "file", "file") + + assert dvc.status(["file.dvc"]) == {} + + with erepo_dir.chdir(): + Repo.init() + erepo_dir.scm.repo.index.remove(["file"]) + os.remove("file") + erepo_dir.dvc_gen("file", "second version") + erepo_dir.scm.add([".dvc", "file.dvc"]) + erepo_dir.scm.commit("version with dvc") + new_rev = erepo_dir.scm.get_rev() + + assert old_rev != new_rev + + # Caching in external repos doesn't see upstream updates within single + # cli call, so we need to clean the caches to see the changes. + clean_repos() + + status, = dvc.status(["file.dvc"])["file.dvc"] + + assert status == { + "changed deps": { + "file ({})".format(fspath(erepo_dir)): "update available" + } + } diff --git a/tests/func/test_update.py b/tests/func/test_update.py index 108e32b82..6f483fff0 100644 --- a/tests/func/test_update.py +++ b/tests/func/test_update.py @@ -1,31 +1,156 @@ +import pytest +import os +import shutil + +from dvc.repo import Repo +from dvc.stage import Stage from dvc.compat import fspath from dvc.external_repo import clean_repos -def test_update_import(tmp_dir, dvc, erepo_dir): +@pytest.mark.parametrize("cached", [True, False]) +def test_update_import(tmp_dir, dvc, erepo_dir, cached): + old_rev = None with erepo_dir.branch("branch", new=True), erepo_dir.chdir(): - erepo_dir.dvc_gen("version", "branch", "add version file") + gen = erepo_dir.dvc_gen if cached else erepo_dir.scm_gen + gen("version", "branch", "add version file") + old_rev = erepo_dir.scm.get_rev() stage = dvc.imp(fspath(erepo_dir), "version", "version", rev="branch") imported = tmp_dir / "version" assert imported.is_file() assert imported.read_text() == "branch" + assert stage.deps[0].def_repo == { + "url": fspath(erepo_dir), + "rev": "branch", + "rev_lock": old_rev, + } + new_rev = None with erepo_dir.branch("branch", new=False), erepo_dir.chdir(): - erepo_dir.dvc_gen("version", "updated", "update version content") + gen = erepo_dir.dvc_gen if cached else erepo_dir.scm_gen + gen("version", "updated", "update version content") + new_rev = erepo_dir.scm.get_rev() + + assert old_rev != new_rev # Caching in external repos doesn't see upstream updates within single # cli call, so we need to clean the caches to see the changes. clean_repos() - assert dvc.status([stage.path]) == {} + status, = dvc.status([stage.path])["version.dvc"] + changed_dep, = list(status["changed deps"].items()) + assert changed_dep[0].startswith("version ") + assert changed_dep[1] == "update available" + dvc.update(stage.path) + assert dvc.status([stage.path]) == {} assert imported.is_file() assert imported.read_text() == "updated" + stage = Stage.load(dvc, stage.path) + assert stage.deps[0].def_repo == { + "url": fspath(erepo_dir), + "rev": "branch", + "rev_lock": new_rev, + } + + +def test_update_import_after_remote_updates_to_dvc(tmp_dir, dvc, erepo_dir): + old_rev = None + with erepo_dir.branch("branch", new=True), erepo_dir.chdir(): + erepo_dir.scm_gen("version", "branch", commit="add version file") + old_rev = erepo_dir.scm.get_rev() + + stage = dvc.imp(fspath(erepo_dir), "version", "version", rev="branch") + + imported = tmp_dir / "version" + assert imported.is_file() + assert imported.read_text() == "branch" + assert stage.deps[0].def_repo == { + "url": fspath(erepo_dir), + "rev": "branch", + "rev_lock": old_rev, + } + + new_rev = None + with erepo_dir.branch("branch", new=False), erepo_dir.chdir(): + erepo_dir.scm.repo.index.remove("version") + erepo_dir.dvc_gen("version", "updated") + erepo_dir.scm.add(["version", "version.dvc"]) + erepo_dir.scm.commit("upgrade to DVC tracking") + new_rev = erepo_dir.scm.get_rev() + + assert old_rev != new_rev + + # Caching in external repos doesn't see upstream updates within single + # cli call, so we need to clean the caches to see the changes. + clean_repos() + + status, = dvc.status([stage.path])["version.dvc"] + changed_dep, = list(status["changed deps"].items()) + assert changed_dep[0].startswith("version ") + assert changed_dep[1] == "update available" + + dvc.update(stage.path) + + assert dvc.status([stage.path]) == {} + + assert imported.is_file() + assert imported.read_text() == "updated" + + stage = Stage.load(dvc, stage.path) + assert stage.deps[0].def_repo == { + "url": fspath(erepo_dir), + "rev": "branch", + "rev_lock": new_rev, + } + + +def test_update_before_and_after_dvc_init(tmp_dir, dvc, erepo_dir): + with erepo_dir.chdir(): + erepo_dir.scm.repo.index.remove([".dvc"], r=True) + shutil.rmtree(".dvc") + erepo_dir.scm_gen("file", "first version") + erepo_dir.scm.add(["file"]) + erepo_dir.scm.commit("first version") + old_rev = erepo_dir.scm.get_rev() + + stage = dvc.imp(fspath(erepo_dir), "file", "file") + + with erepo_dir.chdir(): + Repo.init() + erepo_dir.scm.repo.index.remove(["file"]) + os.remove("file") + erepo_dir.dvc_gen("file", "second version") + erepo_dir.scm.add([".dvc", "file.dvc"]) + erepo_dir.scm.commit("version with dvc") + new_rev = erepo_dir.scm.get_rev() + + assert old_rev != new_rev + + # Caching in external repos doesn't see upstream updates within single + # cli call, so we need to clean the caches to see the changes. + clean_repos() + + assert dvc.status([stage.path]) == { + "file.dvc": [ + { + "changed deps": { + "file ({})".format(fspath(erepo_dir)): "update available" + } + } + ] + } + + dvc.update(stage.path) + + assert (tmp_dir / "file").read_text() == "second version" + assert dvc.status([stage.path]) == {} + def test_update_import_url(tmp_dir, dvc, tmp_path_factory): import_src = tmp_path_factory.mktemp("import_url_source") diff --git a/tests/remotes.py b/tests/remotes.py index d4c61ebc6..2cf1c19b1 100644 --- a/tests/remotes.py +++ b/tests/remotes.py @@ -42,78 +42,19 @@ TEST_GDRIVE_CLIENT_ID = ( ) TEST_GDRIVE_CLIENT_SECRET = "2fy_HyzSwkxkGzEken7hThXb" - -def _should_test_gcp(): - do_test = env2bool("DVC_TEST_GCP", undefined=None) - if do_test is not None: - return do_test - - if not os.path.exists(TEST_GCP_CREDS_FILE): - return False - - try: - check_output( - [ - "gcloud", - "auth", - "activate-service-account", - "--key-file", - TEST_GCP_CREDS_FILE, - ] - ) - except (CalledProcessError, OSError): - return False - return True - - -def _should_test_hdfs(): - if platform.system() != "Linux": - return False - - try: - check_output( - ["hadoop", "version"], shell=True, executable=os.getenv("SHELL") - ) - except (CalledProcessError, IOError): - return False - - p = Popen( - "hadoop fs -ls hdfs://127.0.0.1/", - shell=True, - executable=os.getenv("SHELL"), - ) - p.communicate() - if p.returncode != 0: - return False - - return True - - -def get_local_storagepath(): - return TestDvc.mkdtemp() - - -def get_local_url(): - return get_local_storagepath() - - -def get_hdfs_url(): - return "hdfs://{}@127.0.0.1{}".format( - getpass.getuser(), get_local_storagepath() - ) - - -def get_gcp_storagepath(): - return TEST_GCP_REPO_BUCKET + "/" + str(uuid.uuid4()) - - -def get_gcp_url(): - return "gs://" + get_gcp_storagepath() +always_test = staticmethod(lambda: True) class Local: - should_test = lambda: True # noqa: E731 - get_url = get_local_url + should_test = always_test + + @staticmethod + def get_storagepath(): + return TestDvc.mkdtemp() + + @staticmethod + def get_url(): + return Local.get_storagepath() class S3: @@ -140,7 +81,7 @@ class S3: class S3Mocked(S3): - should_test = lambda: True # noqa: E731 + should_test = always_test @classmethod @contextmanager @@ -160,8 +101,36 @@ class S3Mocked(S3): class GCP: - should_test = _should_test_gcp - get_url = get_gcp_url + @staticmethod + def should_test(): + do_test = env2bool("DVC_TEST_GCP", undefined=None) + if do_test is not None: + return do_test + + if not os.path.exists(TEST_GCP_CREDS_FILE): + return False + + try: + check_output( + [ + "gcloud", + "auth", + "activate-service-account", + "--key-file", + TEST_GCP_CREDS_FILE, + ] + ) + except (CalledProcessError, OSError): + return False + return True + + @staticmethod + def get_storagepath(): + return TEST_GCP_REPO_BUCKET + "/" + str(uuid.uuid4()) + + @staticmethod + def get_url(): + return "gs://" + GCP.get_storagepath() @classmethod @contextmanager @@ -247,18 +216,18 @@ class SSH: @staticmethod def get_url(): return "ssh://{}@127.0.0.1:22{}".format( - getpass.getuser(), get_local_storagepath() + getpass.getuser(), Local.get_storagepath() ) class SSHMocked: - should_test = lambda: True # noqa: E731 + should_test = always_test @staticmethod def get_url(user, port): - path = get_local_storagepath() + path = Local.get_storagepath() if os.name == "nt": - # NOTE: On Windows get_local_storagepath() will return an + # NOTE: On Windows Local.get_storagepath() will return an # ntpath that looks something like `C:\some\path`, which is not # compatible with SFTP paths [1], so we need to convert it to # a proper posixpath. @@ -277,5 +246,33 @@ class SSHMocked: class HDFS: - should_test = _should_test_hdfs - get_url = get_hdfs_url + @staticmethod + def should_test(): + if platform.system() != "Linux": + return False + + try: + check_output( + ["hadoop", "version"], + shell=True, + executable=os.getenv("SHELL"), + ) + except (CalledProcessError, IOError): + return False + + p = Popen( + "hadoop fs -ls hdfs://127.0.0.1/", + shell=True, + executable=os.getenv("SHELL"), + ) + p.communicate() + if p.returncode != 0: + return False + + return True + + @staticmethod + def get_url(): + return "hdfs://{}@127.0.0.1{}".format( + getpass.getuser(), Local.get_storagepath() + ) diff --git a/tests/unit/command/test_get.py b/tests/unit/command/test_get.py index 2c464ecc9..6ac5e4bb5 100644 --- a/tests/unit/command/test_get.py +++ b/tests/unit/command/test_get.py @@ -14,3 +14,18 @@ def test_get(mocker): assert cmd.run() == 0 m.assert_called_once_with("repo_url", path="src", out="out", rev="version") + + +def test_get_url(mocker, caplog): + cli_args = parse_args( + ["get", "repo_url", "src", "--rev", "version", "--show-url"] + ) + assert cli_args.func == CmdGet + + cmd = cli_args.func(cli_args) + m = mocker.patch("dvc.api.get_url", return_value="resource_url") + + assert cmd.run() == 0 + assert "resource_url" in caplog.text + + m.assert_called_once_with("src", repo="repo_url", rev="version") diff --git a/tests/unit/command/test_metrics.py b/tests/unit/command/test_metrics.py new file mode 100644 index 000000000..fb2394bd2 --- /dev/null +++ b/tests/unit/command/test_metrics.py @@ -0,0 +1,67 @@ +from dvc.cli import parse_args +from dvc.command.metrics import CmdMetricsDiff, _show_diff + + +def test_metrics_diff(dvc, mocker): + cli_args = parse_args( + [ + "metrics", + "diff", + "HEAD~10", + "HEAD~1", + "-t", + "json", + "-x", + "x.path", + "-R", + "--show-json", + "--targets", + "target1", + "target2", + ] + ) + assert cli_args.func == CmdMetricsDiff + + cmd = cli_args.func(cli_args) + m = mocker.patch("dvc.repo.metrics.diff.diff", return_value={}) + + assert cmd.run() == 0 + + m.assert_called_once_with( + cmd.repo, + targets=["target1", "target2"], + a_ref="HEAD~10", + b_ref="HEAD~1", + typ="json", + xpath="x.path", + recursive=True, + ) + + +def test_metrics_show_json_diff(): + assert _show_diff( + {"metrics.json": {"a.b.c": {"old": 1, "new": 2, "diff": 3}}} + ) == ( + " Path Metric Value Change\n" + "metrics.json a.b.c 2 3 " + ) + + +def test_metrics_show_raw_diff(): + assert _show_diff({"metrics": {"": {"old": "1", "new": "2"}}}) == ( + " Path Metric Value Change \n" + "metrics 2 diff not supported" + ) + + +def test_metrics_diff_no_diff(): + assert _show_diff( + {"other.json": {"a.b.d": {"old": "old", "new": "new"}}} + ) == ( + " Path Metric Value Change \n" + "other.json a.b.d new diff not supported" + ) + + +def test_metrics_diff_no_changes(): + assert _show_diff({}) == "No changes." diff --git a/tests/unit/command/test_repro.py b/tests/unit/command/test_repro.py index 9dc66dc4f..dcdfdbe74 100644 --- a/tests/unit/command/test_repro.py +++ b/tests/unit/command/test_repro.py @@ -15,14 +15,14 @@ default_arguments = { } -def test_default_arguments(dvc_repo, mocker): +def test_default_arguments(dvc, mocker): cmd = CmdRepro(parse_args(["repro"])) mocker.patch.object(cmd.repo, "reproduce") cmd.run() cmd.repo.reproduce.assert_called_with("Dvcfile", **default_arguments) -def test_downstream(dvc_repo, mocker): +def test_downstream(dvc, mocker): cmd = CmdRepro(parse_args(["repro", "--downstream"])) mocker.patch.object(cmd.repo, "reproduce") cmd.run() diff --git a/tests/unit/command/test_run.py b/tests/unit/command/test_run.py index 6e1f3ba77..f11b8e7a1 100644 --- a/tests/unit/command/test_run.py +++ b/tests/unit/command/test_run.py @@ -2,7 +2,7 @@ from dvc.cli import parse_args from dvc.command.run import CmdRun -def test_run(mocker, dvc_repo): +def test_run(mocker, dvc): cli_args = parse_args( [ "run", @@ -64,7 +64,7 @@ def test_run(mocker, dvc_repo): ) -def test_run_args_from_cli(mocker, dvc_repo): +def test_run_args_from_cli(mocker, dvc): args = parse_args(["run", "echo", "foo"]) cmd = args.func(args) m = mocker.patch.object(cmd.repo, "run", autospec=True) @@ -90,7 +90,7 @@ def test_run_args_from_cli(mocker, dvc_repo): ) -def test_run_args_with_spaces(mocker, dvc_repo): +def test_run_args_with_spaces(mocker, dvc): args = parse_args(["run", "echo", "foo bar"]) cmd = args.func(args) m = mocker.patch.object(cmd.repo, "run", autospec=True) diff --git a/tests/unit/command/test_update.py b/tests/unit/command/test_update.py index c44def6fa..677b24a5a 100644 --- a/tests/unit/command/test_update.py +++ b/tests/unit/command/test_update.py @@ -2,7 +2,7 @@ from dvc.cli import parse_args from dvc.command.update import CmdUpdate -def test_update(dvc_repo, mocker): +def test_update(dvc, mocker): targets = ["target1", "target2", "target3"] cli_args = parse_args(["update"] + targets) assert cli_args.func == CmdUpdate diff --git a/tests/unit/output/test_local.py b/tests/unit/output/test_local.py index 1e64d32ac..50059e6ea 100644 --- a/tests/unit/output/test_local.py +++ b/tests/unit/output/test_local.py @@ -23,27 +23,27 @@ class TestOutputLOCAL(TestDvc): o.save() -def test_str_workdir_outside_repo(erepo): - stage = Stage(erepo.dvc) +def test_str_workdir_outside_repo(erepo_dir): + stage = Stage(erepo_dir.dvc) output = OutputLOCAL(stage, "path", cache=False) - assert relpath("path", erepo.dvc.root_dir) == str(output) + assert relpath("path", erepo_dir.dvc.root_dir) == str(output) -def test_str_workdir_inside_repo(dvc_repo): - stage = Stage(dvc_repo) +def test_str_workdir_inside_repo(dvc): + stage = Stage(dvc) output = OutputLOCAL(stage, "path", cache=False) assert "path" == str(output) - stage = Stage(dvc_repo, wdir="some_folder") + stage = Stage(dvc, wdir="some_folder") output = OutputLOCAL(stage, "path", cache=False) assert os.path.join("some_folder", "path") == str(output) -def test_str_on_absolute_path(dvc_repo): - stage = Stage(dvc_repo) +def test_str_on_absolute_path(dvc): + stage = Stage(dvc) path = os.path.abspath(os.path.join("path", "to", "file")) output = OutputLOCAL(stage, path, cache=False) diff --git a/tests/unit/remote/ssh/test_connection.py b/tests/unit/remote/ssh/test_connection.py index 3f79d9a49..adfada2aa 100644 --- a/tests/unit/remote/ssh/test_connection.py +++ b/tests/unit/remote/ssh/test_connection.py @@ -88,7 +88,8 @@ def test_walk(tmp_path, ssh): not in ["xfs", "apfs", "btrfs"], reason="Reflinks only work in specified file systems", ) -def test_reflink(repo_dir, ssh): +def test_reflink(tmp_dir, ssh): + tmp_dir.gen("foo", "foo content") ssh.reflink("foo", "link") assert filecmp.cmp("foo", "link") assert not System.is_symlink("link") @@ -99,7 +100,8 @@ def test_reflink(repo_dir, ssh): platform.system() == "Windows", reason="sftp symlink is not supported on Windows", ) -def test_symlink(repo_dir, ssh): +def test_symlink(tmp_dir, ssh): + tmp_dir.gen("foo", "foo content") ssh.symlink("foo", "link") assert System.is_symlink("link") @@ -108,7 +110,8 @@ def test_symlink(repo_dir, ssh): platform.system() == "Windows", reason="hardlink is temporarily not supported on Windows", ) -def test_hardlink(repo_dir, ssh): +def test_hardlink(tmp_dir, ssh): + tmp_dir.gen("foo", "foo content") ssh.hardlink("foo", "link") assert System.is_hardlink("link") @@ -117,12 +120,14 @@ def test_hardlink(repo_dir, ssh): platform.system() == "Windows", reason="copy is temporarily not supported on Windows", ) -def test_copy(repo_dir, ssh): +def test_copy(tmp_dir, ssh): + tmp_dir.gen("foo", "foo content") ssh.copy("foo", "link") assert filecmp.cmp("foo", "link") -def test_move(repo_dir, ssh): +def test_move(tmp_dir, ssh): + tmp_dir.gen("foo", "foo content") ssh.move("foo", "copy") assert os.path.exists("copy") assert not os.path.exists("foo") diff --git a/tests/unit/remote/ssh/test_ssh.py b/tests/unit/remote/ssh/test_ssh.py index 3c438bfb1..ce25665eb 100644 --- a/tests/unit/remote/ssh/test_ssh.py +++ b/tests/unit/remote/ssh/test_ssh.py @@ -186,7 +186,7 @@ def test_ssh_gss_auth(mock_file, mock_exists, config, expected_gss_auth): assert remote.gss_auth == expected_gss_auth -def test_hardlink_optimization(repo_dir, ssh_server): +def test_hardlink_optimization(tmp_dir, ssh_server): port = ssh_server.test_creds["port"] user = ssh_server.test_creds["username"] diff --git a/tests/unit/remote/test_gdrive.py b/tests/unit/remote/test_gdrive.py index bdc895107..09dc7e62b 100644 --- a/tests/unit/remote/test_gdrive.py +++ b/tests/unit/remote/test_gdrive.py @@ -1,6 +1,7 @@ import pytest import os +from dvc.config import Config from dvc.remote.gdrive import ( RemoteGDrive, GDriveAccessTokenRefreshError, @@ -14,6 +15,7 @@ USER_CREDS_MISSED_KEY_ERROR = "{}" class Repo(object): tmp_dir = "" + config = Config() class TestRemoteGDrive(object): diff --git a/tests/unit/remote/test_remote.py b/tests/unit/remote/test_remote.py new file mode 100644 index 000000000..b2fa73aff --- /dev/null +++ b/tests/unit/remote/test_remote.py @@ -0,0 +1,41 @@ +from dvc.remote import Remote + + +def set_config_opts(dvc, commands): + list(map(lambda args: dvc.config.set(*args), commands)) + + +def test_remote_with_checksum_jobs(dvc): + set_config_opts( + dvc, + [ + ('remote "with_checksum_jobs"', "url", "s3://bucket/name"), + ('remote "with_checksum_jobs"', "checksum_jobs", 100), + ("core", "checksum_jobs", 200), + ], + ) + + remote = Remote(dvc, name="with_checksum_jobs") + assert remote.checksum_jobs == 100 + + +def test_remote_without_checksum_jobs(dvc): + set_config_opts( + dvc, + [ + ('remote "without_checksum_jobs"', "url", "s3://bucket/name"), + ("core", "checksum_jobs", "200"), + ], + ) + + remote = Remote(dvc, name="without_checksum_jobs") + assert remote.checksum_jobs == 200 + + +def test_remote_without_checksum_jobs_default(dvc): + set_config_opts( + dvc, [('remote "without_checksum_jobs"', "url", "s3://bucket/name")] + ) + + remote = Remote(dvc, name="without_checksum_jobs") + assert remote.checksum_jobs == remote.CHECKSUM_JOBS diff --git a/tests/unit/test_stage.py b/tests/unit/test_stage.py index ce23fad52..298f3fd51 100644 --- a/tests/unit/test_stage.py +++ b/tests/unit/test_stage.py @@ -93,14 +93,14 @@ def test_stage_update(mocker): not isinstance(threading.current_thread(), threading._MainThread), reason="Not running in the main thread.", ) -def test_stage_run_ignore_sigint(dvc_repo, mocker): +def test_stage_run_ignore_sigint(dvc, mocker): proc = mocker.Mock() communicate = mocker.Mock() proc.configure_mock(returncode=0, communicate=communicate) popen = mocker.patch.object(subprocess, "Popen", return_value=proc) signal_mock = mocker.patch("signal.signal") - dvc_repo.run(cmd="path") + dvc.run(cmd="path") assert popen.called_once() assert communicate.called_once_with() @@ -108,9 +108,9 @@ def test_stage_run_ignore_sigint(dvc_repo, mocker): assert signal.getsignal(signal.SIGINT) == signal.default_int_handler -def test_always_changed(dvc_repo): - stage = Stage(dvc_repo, "path", always_changed=True) +def test_always_changed(dvc): + stage = Stage(dvc, "path", always_changed=True) stage.save() - with dvc_repo.lock: + with dvc.lock: assert stage.changed() assert stage.status()["path"] == ["always changed"] diff --git a/tests/unit/test_version.py b/tests/unit/test_version.py index 1d21c7205..5cd4f8ca5 100644 --- a/tests/unit/test_version.py +++ b/tests/unit/test_version.py @@ -12,10 +12,10 @@ def test_is_release(): assert ret is False m.side_effect = None - m.return_value = dvc.version._BASE_VERSION + m.return_value = dvc.version._BASE_VERSION.encode("ascii") ret = dvc.version._is_release(None, dvc.version._BASE_VERSION) assert ret - m.return_value = "630d1741c2d5dd89a3176bd15b63121b905d35c9" + m.return_value = b"630d1741c2d5dd89a3176bd15b63121b905d35c9" ret = dvc.version._is_release(None, dvc.version._BASE_VERSION) assert ret is False diff --git a/tests/unit/utils/test_fs.py b/tests/unit/utils/test_fs.py index fbde5fcdf..0c0e19a0b 100644 --- a/tests/unit/utils/test_fs.py +++ b/tests/unit/utils/test_fs.py @@ -6,6 +6,7 @@ import pytest from mock import patch import dvc +from dvc.compat import fspath from dvc.ignore import CleanTree from dvc.path_info import PathInfo from dvc.scm.tree import WorkingTree @@ -120,26 +121,33 @@ def test_relpath_windows_different_drives(): assert rel_info == path1 -def test_get_inode(repo_dir): - path = repo_dir.FOO - path_info = PathInfo(path) - assert get_inode(path) == get_inode(path_info) +def test_get_inode(tmp_dir): + tmp_dir.gen("foo", "foo content") + + assert get_inode("foo") == get_inode(PathInfo("foo")) -@pytest.mark.parametrize("path", [TestDir.DATA, TestDir.DATA_DIR]) -def test_path_object_and_str_are_valid_types_get_mtime_and_size( - path, repo_dir -): - tree = CleanTree(WorkingTree(repo_dir.root_dir)) - time, size = get_mtime_and_size(path, tree) - object_time, object_size = get_mtime_and_size(PathInfo(path), tree) +def test_path_object_and_str_are_valid_types_get_mtime_and_size(tmp_dir): + tmp_dir.gen( + {"dir": {"dir_file": "dir file content"}, "file": "file_content"} + ) + tree = CleanTree(WorkingTree(tmp_dir)) + + time, size = get_mtime_and_size("dir", tree) + object_time, object_size = get_mtime_and_size(PathInfo("dir"), tree) + assert time == object_time + assert size == object_size + + time, size = get_mtime_and_size("file", tree) + object_time, object_size = get_mtime_and_size(PathInfo("file"), tree) assert time == object_time assert size == object_size -def test_move(repo_dir): - src = repo_dir.FOO - src_info = PathInfo(repo_dir.BAR) +def test_move(tmp_dir): + tmp_dir.gen({"foo": "foo content", "bar": "bar content"}) + src = "foo" + src_info = PathInfo("bar") dest = os.path.join("some", "directory") dest_info = PathInfo(os.path.join("some", "path-like", "directory")) @@ -156,9 +164,10 @@ def test_move(repo_dir): assert len(os.listdir(dest_info.fspath)) == 1 -def test_remove(repo_dir): - path = repo_dir.FOO - path_info = PathInfo(repo_dir.BAR) +def test_remove(tmp_dir): + tmp_dir.gen({"foo": "foo content", "bar": "bar content"}) + path = "foo" + path_info = PathInfo("bar") remove(path) assert not os.path.isfile(path) @@ -208,11 +217,9 @@ def test_path_isin_with_absolute_path(): assert path_isin(child, parent) -def test_makedirs(repo_dir): - path = os.path.join(repo_dir.root_dir, "directory") - path_info = PathInfo( - os.path.join(repo_dir.root_dir, "another", "directory") - ) +def test_makedirs(tmp_dir): + path = os.path.join(fspath(tmp_dir), "directory") + path_info = PathInfo(os.path.join(fspath(tmp_dir), "another", "directory")) makedirs(path) assert os.path.isdir(path) @@ -221,11 +228,19 @@ def test_makedirs(repo_dir): assert os.path.isdir(path_info.fspath) -@pytest.mark.parametrize("path", [TestDir.DATA, TestDir.DATA_DIR]) -def test_copyfile(path, repo_dir): - src = repo_dir.FOO +@pytest.mark.parametrize("path", ["file", "dir"]) +def test_copyfile(path, tmp_dir): + tmp_dir.gen( + { + "foo": "foo content", + "bar": "bar content", + "file": "file content", + "dir": {}, + } + ) + src = "foo" dest = path - src_info = PathInfo(repo_dir.BAR) + src_info = PathInfo("bar") dest_info = PathInfo(path) copyfile(src, dest) diff --git a/tests/unit/utils/test_utils.py b/tests/unit/utils/test_utils.py index c4849ae30..62f31446b 100644 --- a/tests/unit/utils/test_utils.py +++ b/tests/unit/utils/test_utils.py @@ -4,7 +4,7 @@ import os import pytest from dvc.path_info import PathInfo -from dvc.utils import file_md5 +from dvc.utils import file_md5, resolve_output from dvc.utils import fix_env from dvc.utils import relpath from dvc.utils import to_chunks @@ -78,10 +78,10 @@ def test_fix_env_pyenv(path, orig): assert fix_env(env)["PATH"] == orig -def test_file_md5(repo_dir): - fname = repo_dir.FOO - fname_object = PathInfo(fname) - assert file_md5(fname) == file_md5(fname_object) +def test_file_md5(tmp_dir): + tmp_dir.gen("foo", "foo content") + + assert file_md5("foo") == file_md5(PathInfo("foo")) def test_tmp_fname(): @@ -104,3 +104,28 @@ def test_relpath(): path_info = PathInfo(path) assert relpath(path) == relpath(path_info) + + +@pytest.mark.parametrize( + "inp,out,is_dir,expected", + [ + ["target", None, False, "target"], + ["target", "dir", True, os.path.join("dir", "target")], + ["target", "file_target", False, "file_target"], + [ + "target", + os.path.join("dir", "subdir"), + True, + os.path.join("dir", "subdir", "target"), + ], + ["dir/", None, False, "dir"], + ["dir", None, False, "dir"], + ["dir", "other_dir", False, "other_dir"], + ["dir", "other_dir", True, os.path.join("other_dir", "dir")], + ], +) +def test_resolve_output(inp, out, is_dir, expected, mocker): + with mocker.patch("os.path.isdir", return_value=is_dir): + result = resolve_output(inp, out) + + assert result == expected