diff --git a/.travis.yml b/.travis.yml index 66be424aa..4bddf32e6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -75,6 +75,26 @@ jobs: language: python python: 3.7 script: ./scripts/build_package.sh + # build snap in parallel with other pkgs + - name: Snapcraft snap + addons: + snaps: + - name: snapcraft + channel: stable + confinement: classic + - name: lxd + channel: stable + env: + - SNAPCRAFT_IMAGE_INFO: '{"build_url": "$TRAVIS_BUILD_URL"}' + before_install: + - sudo /snap/bin/lxd.migrate -yes + - sudo /snap/bin/lxd waitready + - sudo /snap/bin/lxd init --auto + install: + script: + - sudo snapcraft --use-lxd + after_failure: + - sudo journalctl -u snapd # deploy jobs - name: "Pypi pkgs" if: tag is present @@ -98,6 +118,7 @@ jobs: tags: true repo: iterative/dvc - name: Snapcraft snap + if: type != pull_request stage: deploy addons: snaps: 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/dependency/repo.py b/dvc/dependency/repo.py index 57244bc4f..3e1878f9c 100644 --- a/dvc/dependency/repo.py +++ b/dvc/dependency/repo.py @@ -11,6 +11,7 @@ from dvc.exceptions import NotDvcRepoError from dvc.exceptions import OutputNotFoundError from dvc.exceptions import PathMissingError from dvc.utils.fs import fs_copy +from dvc.scm import SCM class DependencyREPO(DependencyLOCAL): @@ -105,6 +106,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/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/version.py b/dvc/version.py index 7e0719fc0..a24e36253 100644 --- a/dvc/version.py +++ b/dvc/version.py @@ -7,7 +7,7 @@ import os import subprocess -_BASE_VERSION = "0.81.1" +_BASE_VERSION = "0.81.3" def _generate_version(base_version): diff --git a/setup.py b/setup.py index a3fe8421f..011ca93e0 100644 --- a/setup.py +++ b/setup.py @@ -77,6 +77,8 @@ install_requires = [ "networkx>=2.1,<2.4", "speedcopy>=2.0.1", "pyfastcopy>=1.0.3", + "flatten-dict>=0.2.0", + "texttable>=0.5.2", ] @@ -86,10 +88,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 = ["pydrive>=1.3.1", "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/func/test_data_cloud.py b/tests/func/test_data_cloud.py index 7b2e349c8..748a7fe63 100644 --- a/tests/func/test_data_cloud.py +++ b/tests/func/test_data_cloud.py @@ -35,6 +35,7 @@ from tests.remotes import ( GCP, GDrive, HDFS, + Local, S3, SSHMocked, OSS, @@ -44,7 +45,6 @@ from tests.remotes import ( TEST_GDRIVE_CLIENT_ID, TEST_GDRIVE_CLIENT_SECRET, TEST_REMOTE, - get_local_url, ) @@ -77,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__) ) @@ -95,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) @@ -188,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 @@ -221,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 GCP.should_test() - +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 @@ -247,53 +234,27 @@ class TestRemoteGS(TestDataCloudBase): self.assertIsInstance(self.cloud.get_remote(), self._get_cloud_class()) - def _get_url(self): - return GCP.get_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 @@ -302,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) @@ -314,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 HDFS.should_test() - - def _get_url(self): - return HDFS.get_url() - +class TestRemoteHDFS(HDFS, TestDataCloudBase): def _get_cloud_class(self): return RemoteHDFS @@ -344,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: @@ -404,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 HDFS.should_test() - +class TestRemoteHDFSCLI(HDFS, TestDataCloudCLIBase): def _test(self): - url = HDFS.get_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( @@ -481,12 +429,9 @@ class TestRemoteGDriveCLI(TestDataCloudCLIBase): self._test_cloud(TEST_REMOTE) -class TestRemoteGSCLI(TestDataCloudCLIBase): - def _should_test(self): - return GCP.should_test() - +class TestRemoteGSCLI(GCP, TestDataCloudCLIBase): def _test(self): - url = GCP.get_url() + url = self.get_url() self.main(["remote", "add", TEST_REMOTE, url]) self.main( @@ -502,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]) @@ -545,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") @@ -571,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 @@ -659,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_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 09674eff3..c6d1cffe9 100644 --- a/tests/func/test_remote.py +++ b/tests/func/test_remote.py @@ -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): @@ -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) diff --git a/tests/func/test_repro.py b/tests/func/test_repro.py index 3e3be2349..797e975e2 100644 --- a/tests/func/test_repro.py +++ b/tests/func/test_repro.py @@ -38,6 +38,7 @@ from tests.basic_env import TestDvc from tests.remotes import ( GCP, HDFS, + Local, S3, SSH, SSHMocked, @@ -807,7 +808,8 @@ class TestCmdReproChdir(TestDvc): class TestReproExternalBase(TestDvc): - def should_test(self): + @staticmethod + def should_test(): return False @property @@ -956,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" @@ -976,10 +975,7 @@ class TestReproExternalS3(TestReproExternalBase): s3.put_object(Bucket=bucket, Key=key, Body=body) -class TestReproExternalGS(TestReproExternalBase): - def should_test(self): - return GCP.should_test() - +class TestReproExternalGS(GCP, TestReproExternalBase): @property def scheme(self): return "gs" @@ -997,10 +993,7 @@ class TestReproExternalGS(TestReproExternalBase): bucket.blob(key).upload_from_string(body) -class TestReproExternalHDFS(TestReproExternalBase): - def should_test(self): - return HDFS.should_test() - +class TestReproExternalHDFS(HDFS, TestReproExternalBase): @property def scheme(self): return "hdfs" @@ -1057,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" @@ -1070,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): @@ -1104,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(".") @@ -1116,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_update.py b/tests/func/test_update.py index 108e32b82..5afbac048 100644 --- a/tests/func/test_update.py +++ b/tests/func/test_update.py @@ -1,19 +1,36 @@ +import pytest + +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. @@ -26,6 +43,13 @@ def test_update_import(tmp_dir, dvc, erepo_dir): 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_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 4623c7606..2cf1c19b1 100644 --- a/tests/remotes.py +++ b/tests/remotes.py @@ -42,18 +42,19 @@ TEST_GDRIVE_CLIENT_ID = ( ) TEST_GDRIVE_CLIENT_SECRET = "2fy_HyzSwkxkGzEken7hThXb" - -def get_local_storagepath(): - return TestDvc.mkdtemp() - - -def get_local_url(): - return get_local_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: @@ -80,7 +81,7 @@ class S3: class S3Mocked(S3): - should_test = lambda: True # noqa: E731 + should_test = always_test @classmethod @contextmanager @@ -215,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. @@ -273,5 +274,5 @@ class HDFS: @staticmethod def get_url(): return "hdfs://{}@127.0.0.1{}".format( - getpass.getuser(), get_local_storagepath() + getpass.getuser(), Local.get_storagepath() ) 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/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/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..e53c8df21 100644 --- a/tests/unit/utils/test_utils.py +++ b/tests/unit/utils/test_utils.py @@ -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():