From 918ef646dcd9d59fd0e975bbc06cb0ee5753cc85 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Sat, 22 Jun 2019 00:14:52 +0300 Subject: [PATCH] appveyor: test py2 and py3 Related to #2166 Signed-off-by: Ruslan Kuprieiev --- .appveyor.yml | 35 +++++-- .gitignore | 2 + dvc/remote/local/__init__.py | 6 +- dvc/remote/ssh/__init__.py | 2 +- dvc/stage.py | 12 ++- dvc/utils/__init__.py | 11 ++- setup.py | 2 +- tests/.basic_env.py.un~ | Bin 0 -> 11837 bytes tests/basic_env.py | 116 +++++++---------------- tests/conftest.py | 27 +++--- tests/func/test_add.py | 10 +- tests/func/test_checkout.py | 7 +- tests/func/test_data_cloud.py | 2 +- tests/func/test_diff.py | 6 +- tests/func/test_gc.py | 8 +- tests/func/test_install.py | 10 +- tests/func/test_metrics.py | 16 ++-- tests/func/test_move.py | 10 +- tests/func/test_pipeline.py | 89 ++++++++---------- tests/func/test_repo.py | 6 +- tests/func/test_repro.py | 7 -- tests/func/test_run.py | 95 +++++++------------ tests/unit/command/test_run.py | 165 +++++++++++++++++++++------------ tests/unit/scm/test_git.py | 4 +- 24 files changed, 317 insertions(+), 331 deletions(-) create mode 100644 tests/.basic_env.py.un~ diff --git a/.appveyor.yml b/.appveyor.yml index d03b8d529..3cbb7e0a6 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -21,26 +21,39 @@ environment: matrix: - - PYTHON: "C:\\Python36" - PYTHON_VERSION: "3.6.x" + - PYTHON: "C:\\Python27" + PYTHON_VERSION: "2.7.x" PYTHON_ARCH: "32" RUBY: C:\Ruby25\bin + - PYTHON: "C:\\Python37" + PYTHON_VERSION: "3.7.x" + PYTHON_ARCH: "32" + RUBY: C:\Ruby25\bin + +matrix: + allow_failures: + - PYTHON: "C:\\Python37" + install: + - cinst wget + - cinst awscli + - cinst gcloudsdk + - cinst gsutil + - cinst openssl.light --version 1.1.1 + - refreshenv + - "SET PATH=%PYTHON%;%PYTHON%\\Scripts;%RUBY%;%PATH%" - git clone -q https://github.com/iterative/oss-emulator.git - gem install thor builder - ps: cd oss-emulator - ps: $OSSProcess = Start-Process -FilePath ruby -ArgumentList "bin\emulator -r store -p 50004" -PassThru -NoNewWindow - ps: cd .. - - npm install -g azurite + # NOTE: azurite 3 on py3 suffers from bug [1], so we are using an older + # tried-and-true version instead. + # + # [1] https://github.com/Azure/Azurite/issues/215 + - npm install -g azurite@2.7.0 # NOTE azurite 3 is affecte - ps: $AzuriteProcess = Start-Process azurite-blob -PassThru - - cinst wget - - cinst awscli - - cinst gcloudsdk - - refreshenv - - cinst gsutil - - cinst openssl.light --version 1.1.1 - - "SET PATH=%PYTHON%;%PYTHON%\\Scripts;%RUBY%;%PATH%" - python -m pip install -U pip setuptools wheel - pip install -e .[all] - python setup.py install @@ -77,3 +90,5 @@ deploy: secure: 25c1pNoXWbtTwUkKLcThwcl2A2xG8oXHsPuD73HVbQ7SRy9X6HzkCc2sXzhpfRtF on: APPVEYOR_REPO_TAG: true + PYTHON_VERSION: "3.7.x" + PYTHON_ARCH: "32" diff --git a/.gitignore b/.gitignore index e4688cdf1..514e9ba8b 100644 --- a/.gitignore +++ b/.gitignore @@ -34,3 +34,5 @@ env.sh tests/remotes_env scripts/ci/gcp-creds.json .gcp-creds.json + +*~ diff --git a/dvc/remote/local/__init__.py b/dvc/remote/local/__init__.py index b35d326a2..7bfd52067 100644 --- a/dvc/remote/local/__init__.py +++ b/dvc/remote/local/__init__.py @@ -8,7 +8,7 @@ from dvc.utils.compat import str, makedirs, fspath_py35 import os import stat -import uuid +from shortuuid import uuid import shutil import logging @@ -244,7 +244,7 @@ class RemoteLOCAL(RemoteBASE): # moving in two stages to make the whole operation atomic in # case inp and outp are in different filesystems and actual # physical copying of data is happening - tmp = "{}.{}".format(outp, str(uuid.uuid4())) + tmp = "{}.{}".format(outp, str(uuid())) move(inp, tmp) move(tmp, outp) @@ -458,7 +458,7 @@ class RemoteLOCAL(RemoteBASE): def _unprotect_file(path): if System.is_symlink(path) or System.is_hardlink(path): logger.debug("Unprotecting '{}'".format(path)) - tmp = os.path.join(os.path.dirname(path), "." + str(uuid.uuid4())) + tmp = os.path.join(os.path.dirname(path), "." + str(uuid())) # The operations order is important here - if some application # would access the file during the process of copyfile then it diff --git a/dvc/remote/ssh/__init__.py b/dvc/remote/ssh/__init__.py index 4c5cdaeb1..04a04a811 100644 --- a/dvc/remote/ssh/__init__.py +++ b/dvc/remote/ssh/__init__.py @@ -28,7 +28,7 @@ class RemoteSSH(RemoteBASE): scheme = Schemes.SSH REQUIRES = {"paramiko": paramiko} - JOBS = 8 + JOBS = 4 PARAM_CHECKSUM = "md5" DEFAULT_PORT = 22 TIMEOUT = 1800 diff --git a/dvc/stage.py b/dvc/stage.py index 1f217b84b..a9f5409bd 100644 --- a/dvc/stage.py +++ b/dvc/stage.py @@ -8,6 +8,7 @@ import os import subprocess import logging import signal +import threading from dvc.utils import relpath from dvc.utils.compat import pathlib @@ -771,7 +772,12 @@ class Stage(object): executable = os.getenv("SHELL") if os.name != "nt" else None self._warn_if_fish(executable) - old_handler = signal.signal(signal.SIGINT, signal.SIG_IGN) + main_thread = isinstance( + threading.current_thread(), threading._MainThread + ) + if main_thread: + old_handler = signal.signal(signal.SIGINT, signal.SIG_IGN) + p = None try: @@ -781,10 +787,12 @@ class Stage(object): shell=True, env=fix_env(os.environ), executable=executable, + close_fds=True, ) p.communicate() finally: - signal.signal(signal.SIGINT, old_handler) + if main_thread: + signal.signal(signal.SIGINT, old_handler) if (p is None) or (p.returncode != 0): raise StageCmdFailedError(self) diff --git a/dvc/utils/__init__.py b/dvc/utils/__init__.py index 0c513b88e..589f09008 100644 --- a/dvc/utils/__init__.py +++ b/dvc/utils/__init__.py @@ -164,12 +164,13 @@ def move(src, dst): def _chmod(func, p, excinfo): + perm = os.lstat(p).st_mode + perm |= stat.S_IWRITE + try: - perm = os.stat(p).st_mode - perm |= stat.S_IWRITE os.chmod(p, perm) except OSError as exc: - # NOTE: broken symlink case. + # broken symlink if exc.errno != errno.ENOENT: raise @@ -240,9 +241,9 @@ def convert_to_unicode(data): def tmp_fname(fname): """ Temporary name for a partial download """ - from uuid import uuid4 + from shortuuid import uuid - return fspath(fname) + "." + str(uuid4()) + ".tmp" + return fspath(fname) + "." + str(uuid()) + ".tmp" def current_timestamp(): diff --git a/setup.py b/setup.py index 47a80e1f9..3d2d7a54c 100644 --- a/setup.py +++ b/setup.py @@ -69,7 +69,7 @@ install_requires = [ # Extra dependencies for remote integrations gs = ["google-cloud-storage==1.13.0"] s3 = ["boto3==1.9.115"] -azure = ["azure-storage-blob==1.3.0"] +azure = ["azure-storage-blob==2.0.1"] oss = ["oss2==2.6.1"] ssh = ["paramiko>=2.4.1"] all_remotes = gs + s3 + azure + ssh + oss diff --git a/tests/.basic_env.py.un~ b/tests/.basic_env.py.un~ new file mode 100644 index 0000000000000000000000000000000000000000..bc48da4dd76f85edeb0dbe7714a4e988b8bd2a5a GIT binary patch literal 11837 zcmeI&%WD%s90%}CZH>?PtXc}P)K?27KI^-e(u)cr_&{4~t#(6G8&@`oSj0mQ{s|t% zyLj{9zv0zCK<&wcT2F%hzOysoXPf*+kl$?vrZbx~yR*MfX0tQ1x2m<*Hy@3spQ5h# zWBTIl-SJEPeaofZ{qKs4za|SS>Dc?vm&aZ%|9*8PiVEAJD7qN4^f*f@)u*F*F&x*M z6>R3ka6DVhM(@wmlWD3X4;pEKj2+TU3p>t z;;o02cVX){C9^ARgrpFGsnX0k!xHnvLp0Ijc3YBpIwH z3zhP8mZlAS1BOPHj3+*{j!K_2-FGO49cl33`L(CcCCg>2Ad0nx4cG!L)U6fl9eo;4V-0h0wpx9Y<@v=+1T z?7V<<&qc+_Od}qWXBp{*~A(FGWx$f1tu2>t&*7T8K&->2|Z+Q z8z!)9W{m*Z9m?if8JJjPRH`)N^3^c)$oUa&DHv*YOOFt&CHxV6KDrvT~@YtpYVA>*fSSzfdPtn zeV2e8fL%1yOCRt#30PcfvA*!!&Q1f?!IuR@x9amfEBO8kFF1FwQvh|~WdYHx`gqR> zywXCc?Yz)5%PICOARTmdN!i|gx~BzQVHMos0@EyF+am%x;Ii=RRtfiqbn`qS;C6n( z!P>&IH6KWII0su@P_}oU?J2?542;@6ykcqXTx08KU=FmcaA;S2v?m2xjEDqTNbX_n z$4qgMWdYHx`eaW?vLY-XY2LY&fE-|3;n1%5V2=y1qAMKnOqxqD4z4a5>ZQ;1nBXd1 zhrx))L0tlIV0F<@FMX`7C2lxa&(2W literal 0 HcmV?d00001 diff --git a/tests/basic_env.py b/tests/basic_env.py index 3d463771a..5de08bc0f 100644 --- a/tests/basic_env.py +++ b/tests/basic_env.py @@ -3,7 +3,6 @@ from __future__ import unicode_literals import os -import shutil import shortuuid import tempfile import logging @@ -14,9 +13,9 @@ from git.exc import GitCommandNotFound from unittest import TestCase import pytest -from dvc.command.remote import CmdRemoteAdd from dvc.repo import Repo as DvcRepo from dvc.utils.compat import open, str +from dvc.utils import remove logger = logging.getLogger("dvc") @@ -50,11 +49,8 @@ class TestDirFixture(object): UNICODE = "тест" UNICODE_CONTENTS = "проверка" - def __init__(self, root_dir=None): - if root_dir: - os.mkdir(root_dir) - else: - root_dir = self.mkdtemp() + def __init__(self): + root_dir = self.mkdtemp() self._root_dir = os.path.realpath(root_dir) @property @@ -104,7 +100,7 @@ class TestDirFixture(object): def tearDown(self): self._popd() try: - shutil.rmtree(self._root_dir) + remove(self._root_dir) except OSError as exc: # We ignore this under Windows with a warning because it happened # to be really hard to trace all not properly closed files. @@ -131,25 +127,24 @@ class TestGitFixture(TestDirFixture): # GitCommandNotFound: Cmd('git') not found due to: # OSError('[Errno 35] Resource temporarily unavailable') retries = self.N_RETRIES - while retries: + while True: try: self.git = Repo.init() + break except GitCommandNotFound: retries -= 1 - continue - break + if not retries: + raise self.git.index.add([self.CODE]) self.git.index.commit("add code") def tearDown(self): self.git.close() + super(TestGitFixture, self).tearDown() class TestGitSubmoduleFixture(TestGitFixture): - def __init__(self, root_dir=None): - super(TestGitSubmoduleFixture, self).__init__(root_dir) - def setUp(self): super(TestGitSubmoduleFixture, self).setUp() subrepo = Repo.init() @@ -158,96 +153,57 @@ class TestGitSubmoduleFixture(TestGitFixture): self._pushd(subrepo_path) -class TestDvcFixture(TestGitFixture): - def __init__(self, root_dir=None): - super(TestDvcFixture, self).__init__(root_dir) - +class TestDvcFixture(TestDirFixture): def setUp(self): super(TestDvcFixture, self).setUp() - self.dvc = DvcRepo.init(self._root_dir) + self.dvc = DvcRepo.init(self.root_dir, no_scm=True) + + +class TestDvcGitFixture(TestGitFixture): + def setUp(self): + super(TestDvcGitFixture, self).setUp() + self.dvc = DvcRepo.init(self.root_dir) self.dvc.scm.commit("init dvc") def tearDown(self): self.dvc.scm.git.close() - - -class TestDvcGitInitializedFixture(TestDvcFixture): - def __init__(self, root_dir=None): - super(TestDvcGitInitializedFixture, self).__init__(root_dir) - - def setUp(self): - super(TestDvcGitInitializedFixture, self).setUp() - self.git.init() - - -class TestDvcDataFileFixture(TestDvcGitInitializedFixture): - DATA_DVC_FILE = "data.dvc" - DATA_DIR_DVC_FILE = "data_sub_dir.dvc" - REMOTE = "myremote2" - - def __init__(self, root_dir=None, cache_dir=None): - super(TestDvcDataFileFixture, self).__init__(root_dir) - self.cache_dir = cache_dir - - def setUp(self): - super(TestDvcDataFileFixture, self).setUp() - - self.dvc.add(self.DATA) - self.dvc.add(self.DATA_SUB_DIR) - - if self.cache_dir: - shutil.copytree(self.dvc.cache.local.cache_dir, self.cache_dir) - - class MockConfig: - level = 1 - force = False - default = True - name = self.REMOTE - url = self.cache_dir - - cmd = CmdRemoteAdd(MockConfig()) - cmd.run() - - self.git.index.add( - [ - os.path.join(self.DATA_DIR, self.DATA_DVC_FILE), - os.path.join(self.DATA_DIR, self.DATA_DIR_DVC_FILE), - os.path.join(self.DATA_DIR, ".gitignore"), - ".dvc/config", - self.FOO, - self.BAR, - ] - ) - self.git.index.commit("Hello world commit") - - # Return to the dir we started to not confuse parent fixture - os.chdir(self._saved_dir) + super(TestDvcGitFixture, self).tearDown() # NOTE: Inheritance order in the classes below is important. class TestDir(TestDirFixture, TestCase): - def __init__(self, methodName, root_dir=None): - TestDirFixture.__init__(self, root_dir) + def __init__(self, methodName): + TestDirFixture.__init__(self) TestCase.__init__(self, methodName) class TestGit(TestGitFixture, TestCase): - def __init__(self, methodName, root_dir=None): - TestGitFixture.__init__(self, root_dir) + def __init__(self, methodName): + TestGitFixture.__init__(self) TestCase.__init__(self, methodName) class TestGitSubmodule(TestGitSubmoduleFixture, TestCase): - def __init__(self, methodName, root_dir=None): - TestGitSubmoduleFixture.__init__(self, root_dir) + def __init__(self, methodName): + TestGitSubmoduleFixture.__init__(self) TestCase.__init__(self, methodName) class TestDvc(TestDvcFixture, TestCase): - def __init__(self, methodName, root_dir=None): - TestDvcFixture.__init__(self, root_dir) + def __init__(self, methodName): + TestDvcFixture.__init__(self) + TestCase.__init__(self, methodName) + + @pytest.fixture(autouse=True) + def inject_fixtures(self, caplog): + self._caplog = caplog + + +class TestDvcGit(TestDvcGitFixture, TestCase): + def __init__(self, methodName): + TestDvcGitFixture.__init__(self) TestCase.__init__(self, methodName) @pytest.fixture(autouse=True) diff --git a/tests/conftest.py b/tests/conftest.py index e349d7639..c87b4e2d5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,7 +9,7 @@ from dvc.remote.config import RemoteConfig from dvc.utils.compat import cast_bytes_py2 from dvc.remote.ssh.connection import SSHConnection from dvc.repo import Repo as DvcRepo -from .basic_env import TestDirFixture, TestDvcFixture +from .basic_env import TestDirFixture, TestDvcGitFixture # Prevent updater and analytics from running their processes @@ -51,13 +51,14 @@ def git(repo_dir): # GitCommandNotFound: Cmd('git') not found due to: # OSError('[Errno 35] Resource temporarily unavailable') retries = 5 - while retries: + while True: try: git = Repo.init() + break except GitCommandNotFound: retries -= 1 - continue - break + if not retries: + raise try: git.index.add([repo_dir.CODE]) @@ -68,15 +69,8 @@ def git(repo_dir): @pytest.fixture -def dvc_repo(repo_dir, git): - dvc = None - try: - dvc = DvcRepo.init(repo_dir._root_dir) - dvc.scm.commit("init dvc") - yield dvc - finally: - if dvc: - dvc.scm.git.close() +def dvc_repo(repo_dir): + yield DvcRepo.init(repo_dir._root_dir, no_scm=True) here = os.path.abspath(os.path.dirname(__file__)) @@ -139,7 +133,7 @@ def temporary_windows_drive(repo_dir): @pytest.fixture def erepo(repo_dir): - repo = TestDvcFixture() + repo = TestDvcGitFixture() repo.setUp() try: stage_foo = repo.dvc.add(repo.FOO)[0] @@ -167,7 +161,10 @@ def erepo(repo_dir): repo.dvc.scm.checkout("master") - repo._popd() + repo.dvc.scm.git.close() + repo.git.close() + + os.chdir(repo._saved_dir) yield repo finally: repo.tearDown() diff --git a/tests/func/test_add.py b/tests/func/test_add.py index 07e243040..38c8f49d8 100644 --- a/tests/func/test_add.py +++ b/tests/func/test_add.py @@ -24,7 +24,7 @@ from dvc.exceptions import DvcException, RecursiveAddingWhileUsingFilename from dvc.output.base import OutputAlreadyTrackedError from dvc.repo import Repo as DvcRepo -from tests.basic_env import TestDvc +from tests.basic_env import TestDvc, TestDvcGit from tests.utils import spy, get_gitignore_content @@ -129,7 +129,7 @@ class TestAddDirectoryWithForwardSlash(TestDvc): self.assertEqual(os.path.abspath("directory.dvc"), stage.path) -class TestAddTrackedFile(TestDvc): +class TestAddTrackedFile(TestDvcGit): def test(self): fname = "tracked_file" self.create(fname, "tracked file contents") @@ -266,7 +266,7 @@ class TestShouldUpdateStateEntryForFileAfterAdd(TestDvc): self.assertEqual(ret, 0) self.assertEqual(file_md5_counter.mock.call_count, 1) - ret = main(["run", "-d", self.FOO, "cat {}".format(self.FOO)]) + ret = main(["run", "-d", self.FOO, "echo foo"]) self.assertEqual(ret, 0) self.assertEqual(file_md5_counter.mock.call_count, 1) @@ -297,7 +297,7 @@ class TestShouldUpdateStateEntryForDirectoryAfterAdd(TestDvc): self.assertEqual(file_md5_counter.mock.call_count, 3) ret = main( - ["run", "-d", self.DATA_DIR, "ls {}".format(self.DATA_DIR)] + ["run", "-d", self.DATA_DIR, "dir {}".format(self.DATA_DIR)] ) self.assertEqual(ret, 0) self.assertEqual(file_md5_counter.mock.call_count, 3) @@ -468,7 +468,7 @@ class TestAddFilename(TestDvc): self.assertFalse(os.path.exists("foo.dvc")) -class TestShouldCleanUpAfterFailedAdd(TestDvc): +class TestShouldCleanUpAfterFailedAdd(TestDvcGit): def test(self): ret = main(["add", self.FOO]) self.assertEqual(0, ret) diff --git a/tests/func/test_checkout.py b/tests/func/test_checkout.py index 437927bc4..02671f44a 100644 --- a/tests/func/test_checkout.py +++ b/tests/func/test_checkout.py @@ -13,7 +13,8 @@ from dvc.repo import Repo as DvcRepo from dvc.system import System from dvc.utils import walk_files, relpath from dvc.utils.stage import load_stage_file, dump_stage_file -from tests.basic_env import TestDvc +from dvc.utils.compat import is_py2 +from tests.basic_env import TestDvc, TestDvcGit from tests.func.test_repro import TestRepro from dvc.stage import Stage, StageFileBadNameError, StageFileDoesNotExistError from dvc.remote.local import RemoteLOCAL @@ -113,7 +114,7 @@ class TestCmdCheckout(TestCheckout): self._test_checkout() -class CheckoutBase(TestDvc): +class CheckoutBase(TestDvcGit): GIT_IGNORE = ".gitignore" def commit_data_file(self, fname, content="random text"): @@ -538,7 +539,7 @@ class TestCheckoutMovedCacheDirWithSymlinks(TestDvc): ret = main(["add", self.DATA_DIR]) self.assertEqual(ret, 0) - if os.name == "nt": + if os.name == "nt" and is_py2: from jaraco.windows.filesystem import readlink else: readlink = os.readlink diff --git a/tests/func/test_data_cloud.py b/tests/func/test_data_cloud.py index 644a534a9..ef6558939 100644 --- a/tests/func/test_data_cloud.py +++ b/tests/func/test_data_cloud.py @@ -183,7 +183,7 @@ def get_ssh_url_mocked(user, port): # # [1]https://tools.ietf.org/html/draft-ietf-secsh-filexfer-13#section-6 drive, path = os.path.splitdrive(path) - assert drive == "c:" + assert drive.lower() == "c:" path = path.replace("\\", "/") url = "ssh://{}@127.0.0.1:{}{}".format(user, port, path) return url diff --git a/tests/func/test_diff.py b/tests/func/test_diff.py index 6638bfac2..f4b148cfa 100644 --- a/tests/func/test_diff.py +++ b/tests/func/test_diff.py @@ -10,7 +10,7 @@ import dvc.repo.diff as diff from dvc.command.diff import CmdDiff -from tests.basic_env import TestDvc +from tests.basic_env import TestDvcGit def _get_checksum(repo, file_name): @@ -20,7 +20,7 @@ def _get_checksum(repo, file_name): return out.checksum -class TestDiff(TestDvc): +class TestDiff(TestDvcGit): def setUp(self): super(TestDiff, self).setUp() @@ -88,7 +88,7 @@ class TestDiffCmdMessage(TestDiff): self.assertEqual(test_msg, msg) -class TestDiffDir(TestDvc): +class TestDiffDir(TestDvcGit): def setUp(self): super(TestDiffDir, self).setUp() diff --git a/tests/func/test_gc.py b/tests/func/test_gc.py index 7d07c8a6b..deece6adb 100644 --- a/tests/func/test_gc.py +++ b/tests/func/test_gc.py @@ -6,11 +6,11 @@ from git import Repo from dvc.main import main from dvc.repo import Repo as DvcRepo -from tests.basic_env import TestDvc +from tests.basic_env import TestDvcGit from tests.basic_env import TestDir -class TestGC(TestDvc): +class TestGC(TestDvcGit): def setUp(self): super(TestGC, self).setUp() @@ -44,7 +44,7 @@ class TestGC(TestDvc): self.assertTrue(os.path.exists(c)) -class TestGCBranchesTags(TestDvc): +class TestGCBranchesTags(TestDvcGit): def _check_cache(self, num): total = 0 for root, dirs, files in os.walk(os.path.join(".dvc", "cache")): @@ -104,7 +104,7 @@ class TestGCBranchesTags(TestDvc): self._check_cache(1) -class TestGCMultipleDvcRepos(TestDvc): +class TestGCMultipleDvcRepos(TestDvcGit): def _check_cache(self, num): total = 0 for root, dirs, files in os.walk(os.path.join(".dvc", "cache")): diff --git a/tests/func/test_install.py b/tests/func/test_install.py index f845ba38e..5e5efee29 100644 --- a/tests/func/test_install.py +++ b/tests/func/test_install.py @@ -15,7 +15,7 @@ class TestInstall(object): def _hook(self, name): return os.path.join(".git", "hooks", name) - def test_should_create_hooks(self, dvc_repo): + def test_should_create_hooks(self, git, dvc_repo): assert main(["install"]) == 0 hooks_with_commands = [ @@ -30,7 +30,7 @@ class TestInstall(object): with open(self._hook(fname), "r") as fobj: assert command in fobj.read() - def test_should_append_hooks_if_file_already_exists(self, dvc_repo): + def test_should_append_hooks_if_file_already_exists(self, git, dvc_repo): with open(self._hook("post-checkout"), "w") as fobj: fobj.write("#!/bin/sh\n" "echo hello\n") @@ -41,7 +41,7 @@ class TestInstall(object): with open(self._hook("post-checkout"), "r") as fobj: assert fobj.read() == expected_script - def test_should_be_idempotent(self, dvc_repo): + def test_should_be_idempotent(self, git, dvc_repo): assert main(["install"]) == 0 assert main(["install"]) == 0 @@ -50,7 +50,7 @@ class TestInstall(object): with open(self._hook("post-checkout"), "r") as fobj: assert fobj.read() == expected_script - def test_should_post_checkout_hook_checkout(self, repo_dir, dvc_repo): + def test_should_post_checkout_hook_checkout(self, repo_dir, git, dvc_repo): assert main(["install"]) == 0 stage_file = repo_dir.FOO + Stage.STAGE_FILE_SUFFIX @@ -64,7 +64,7 @@ class TestInstall(object): assert os.path.isfile(repo_dir.FOO) - def test_should_pre_push_hook_push(self, repo_dir, dvc_repo): + def test_should_pre_push_hook_push(self, repo_dir, git, dvc_repo): assert main(["install"]) == 0 temp = repo_dir.mkdtemp() diff --git a/tests/func/test_metrics.py b/tests/func/test_metrics.py index ab137fd99..3e2286c88 100644 --- a/tests/func/test_metrics.py +++ b/tests/func/test_metrics.py @@ -11,10 +11,10 @@ from dvc.exceptions import DvcException, BadMetricError, NoMetricsError from dvc.repo.metrics.show import NO_METRICS_FILE_AT_REFERENCE_WARNING from dvc.stage import Stage from dvc.utils import relpath -from tests.basic_env import TestDvc +from tests.basic_env import TestDvcGit -class TestMetricsBase(TestDvc): +class TestMetricsBase(TestDvcGit): def setUp(self): super(TestMetricsBase, self).setUp() self.dvc.scm.commit("init") @@ -366,7 +366,7 @@ class TestMetrics(TestMetricsBase): ) -class TestMetricsRecursive(TestDvc): +class TestMetricsRecursive(TestDvcGit): def setUp(self): super(TestMetricsRecursive, self).setUp() self.dvc.scm.commit("init") @@ -430,7 +430,7 @@ class TestMetricsRecursive(TestDvc): ) -class TestMetricsReproCLI(TestDvc): +class TestMetricsReproCLI(TestDvcGit): def test(self): stage = self.dvc.run( metrics_no_cache=["metrics"], @@ -627,7 +627,7 @@ class TestMetricsCLI(TestMetricsBase): assert "\tmetric.unknown: unknown" in self._caplog.text -class TestNoMetrics(TestDvc): +class TestNoMetrics(TestDvcGit): def test(self): with self.assertRaises(NoMetricsError): self.dvc.metrics.show() @@ -637,7 +637,7 @@ class TestNoMetrics(TestDvc): self.assertNotEqual(ret, 0) -class TestCachedMetrics(TestDvc): +class TestCachedMetrics(TestDvcGit): def _do_add(self, branch): self.dvc.scm.checkout(branch) self.dvc.checkout(force=True) @@ -731,7 +731,7 @@ class TestCachedMetrics(TestDvc): self._test_metrics(self._do_run) -class TestMetricsType(TestDvc): +class TestMetricsType(TestDvcGit): branches = ["foo", "bar", "baz"] files = [ "metric", @@ -810,7 +810,7 @@ class TestShouldDisplayMetricsEvenIfMetricIsMissing(object): self._commit_metric(dvc, "master") - def test(self, dvc_repo, caplog): + def test(self, git, dvc_repo, caplog): self.setUp(dvc_repo) dvc_repo.scm.checkout(self.BRANCH_MISSING_METRIC) diff --git a/tests/func/test_move.py b/tests/func/test_move.py index ccf2d3937..1d2973f99 100644 --- a/tests/func/test_move.py +++ b/tests/func/test_move.py @@ -5,7 +5,7 @@ from dvc.exceptions import DvcException, MoveNotDataSourceError from dvc.stage import Stage from dvc.utils.stage import load_stage_file -from tests.basic_env import TestDvc +from tests.basic_env import TestDvc, TestDvcGit from tests.func.test_repro import TestRepro from tests.utils import cd @@ -122,19 +122,17 @@ class TestMoveFileToDirectoryWithoutSpecifiedTargetName(TestDvc): self.assertEqual(self.FOO, new_stage["outs"][0]["path"]) -class TestMoveDirectoryShouldNotOverwriteExisting(TestDvc): +class TestMoveDirectoryShouldNotOverwriteExisting(TestDvcGit): def test(self): dir_name = "dir" orig_listdir = set(os.listdir(self.DATA_DIR)) - ret = main(["add", self.DATA_DIR]) - self.assertEqual(ret, 0) + self.dvc.add(self.DATA_DIR) os.mkdir(dir_name) new_dir_name = os.path.join(dir_name, self.DATA_DIR) - ret = main(["move", self.DATA_DIR, dir_name]) - self.assertEqual(ret, 0) + self.dvc.move(self.DATA_DIR, dir_name) data_dir_stage = self.DATA_DIR + Stage.STAGE_FILE_SUFFIX self.assertFalse(os.path.exists(self.DATA_DIR)) diff --git a/tests/func/test_pipeline.py b/tests/func/test_pipeline.py index 889499457..bee789879 100644 --- a/tests/func/test_pipeline.py +++ b/tests/func/test_pipeline.py @@ -25,10 +25,6 @@ class TestPipelineShowSingle(TestDvc): ret = main(["pipeline", "show", self.stage, "--outs"]) self.assertEqual(ret, 0) - def test_ascii(self): - ret = main(["pipeline", "show", "--ascii", self.stage]) - self.assertEqual(ret, 0) - def test_dot(self): ret = main(["pipeline", "show", "--dot", self.stage]) self.assertEqual(ret, 0) @@ -37,10 +33,6 @@ class TestPipelineShowSingle(TestDvc): ret = main(["pipeline", "show", "--tree", self.stage]) self.assertEqual(ret, 0) - def test_ascii_commands(self): - ret = main(["pipeline", "show", "--ascii", self.stage, "--commands"]) - self.assertEqual(ret, 0) - def test_ascii_outs(self): ret = main(["pipeline", "show", "--ascii", self.stage, "--outs"]) self.assertEqual(ret, 0) @@ -62,10 +54,17 @@ class TestPipelineShowSingle(TestDvc): self.assertNotEqual(ret, 0) -class TestPipelineShow(TestRepro): - def setUp(self): - super(TestPipelineShow, self).setUp() +def test_single_ascii(repo_dir, dvc_repo): + dvc_repo.add(repo_dir.FOO) + assert main(["pipeline", "show", "--ascii", "foo.dvc"]) == 0 + +def test_single_ascii_commands(repo_dir, dvc_repo): + dvc_repo.add(repo_dir.FOO) + assert main(["pipeline", "show", "--ascii", "foo.dvc", "--commands"]) == 0 + + +class TestPipelineShow(TestRepro): def test(self): ret = main(["pipeline", "show", self.file1_stage]) self.assertEqual(ret, 0) @@ -74,10 +73,6 @@ class TestPipelineShow(TestRepro): ret = main(["pipeline", "show", self.file1_stage, "--commands"]) self.assertEqual(ret, 0) - def test_outs(self): - ret = main(["pipeline", "show", self.file1_stage, "--outs"]) - self.assertEqual(ret, 0) - def test_ascii(self): ret = main(["pipeline", "show", "--ascii", self.file1_stage]) self.assertEqual(ret, 0) @@ -102,36 +97,40 @@ class TestPipelineShow(TestRepro): ) self.assertEqual(ret, 0) - def test_dot_outs(self): - ret = main(["pipeline", "show", "--dot", self.file1_stage, "--outs"]) + +def test_print_locked_stages(repo_dir, dvc_repo, caplog): + dvc_repo.add("foo") + dvc_repo.add("bar") + dvc_repo.lock_stage("foo.dvc") + + caplog.clear() + with caplog.at_level(logging.INFO, logger="dvc"): + assert main(["pipeline", "show", "foo.dvc", "--locked"]) == 0 + + assert "foo.dvc" in caplog.text + assert "bar.dvc" not in caplog.text + + +def test_dot_outs(repo_dir, dvc_repo): + dvc_repo.add(repo_dir.FOO) + dvc_repo.run( + outs=["file"], + deps=[repo_dir.FOO, repo_dir.CODE], + cmd="python {} {} {}".format(repo_dir.CODE, repo_dir.FOO, "file"), + ) + assert main(["pipeline", "show", "--dot", "file.dvc", "--outs"]) == 0 + + +class TestPipelineShowOuts(TestRepro): + def setUp(self): + super(TestPipelineShowOuts, self).setUp() + + def test_outs(self): + ret = main(["pipeline", "show", self.file1_stage, "--outs"]) self.assertEqual(ret, 0) - def test_not_dvc_file(self): - ret = main(["pipeline", "show", self.file1]) - self.assertNotEqual(ret, 0) - - def test_non_existing(self): - ret = main(["pipeline", "show", "non-existing"]) - self.assertNotEqual(ret, 0) - - def test_print_locked_stages(self): - self.dvc.add("foo") - self.dvc.add("bar") - self.dvc.lock_stage("foo.dvc") - - self._caplog.clear() - with self._caplog.at_level(logging.INFO, logger="dvc"): - ret = main(["pipeline", "show", "foo.dvc", "--locked"]) - self.assertEqual(ret, 0) - - assert "foo.dvc" in self._caplog.text - assert "bar.dvc" not in self._caplog.text - class TestPipelineShowDeep(TestReproChangedDeepData): - def setUp(self): - super(TestPipelineShowDeep, self).setUp() - def test(self): ret = main(["pipeline", "show", self.file1_stage]) self.assertEqual(ret, 0) @@ -172,14 +171,6 @@ class TestPipelineShowDeep(TestReproChangedDeepData): ret = main(["pipeline", "show", "--dot", self.file1_stage, "--outs"]) self.assertEqual(ret, 0) - def test_not_dvc_file(self): - ret = main(["pipeline", "show", self.file1]) - self.assertNotEqual(ret, 0) - - def test_non_existing(self): - ret = main(["pipeline", "show", "non-existing"]) - self.assertNotEqual(ret, 0) - class TestPipelineListEmpty(TestDvc): def test(self): diff --git a/tests/func/test_repo.py b/tests/func/test_repo.py index 89880c492..abd8ed380 100644 --- a/tests/func/test_repo.py +++ b/tests/func/test_repo.py @@ -1,12 +1,12 @@ from dvc.main import main from dvc.stage import Stage -from tests.basic_env import TestDvc +from tests.basic_env import TestDvcGit from dvc.scm.git import GitTree from dvc.scm.tree import WorkingTree -class TestCollect(TestDvc): +class TestCollect(TestDvcGit): def setUp(self): super(TestCollect, self).setUp() self.dvc.add(self.FOO) @@ -46,7 +46,7 @@ class TestCollect(TestDvc): self.assertEqual([str(i) for i in result[0].deps], ["bar"]) -class TestIgnore(TestDvc): +class TestIgnore(TestDvcGit): def _stage_name(self, file): return file + Stage.STAGE_FILE_SUFFIX diff --git a/tests/func/test_repro.py b/tests/func/test_repro.py index 5f174a78d..d01dfd656 100644 --- a/tests/func/test_repro.py +++ b/tests/func/test_repro.py @@ -1242,13 +1242,6 @@ class TestReproShell(TestDvc): self.assertEqual(os.getenv("SHELL"), fd.read().strip()) -class TestReproNoSCM(TestRepro): - def test(self): - shutil.rmtree(self.dvc.scm.dir) - ret = main(["repro", self.file1_stage]) - self.assertEqual(ret, 0) - - class TestReproAllPipelines(TestDvc): def test(self): self.dvc.run( diff --git a/tests/func/test_run.py b/tests/func/test_run.py index 3d707cdda..774999f9b 100644 --- a/tests/func/test_run.py +++ b/tests/func/test_run.py @@ -7,6 +7,8 @@ import shutil import filecmp import subprocess import signal +import threading +import pytest from dvc.main import main from dvc.output import OutputBase @@ -280,91 +282,64 @@ class TestRunBadName(TestDvc): ) -class TestCmdRun(TestDvc): - def test_run(self): - ret = main( +@pytest.mark.skipif( + not isinstance(threading.current_thread(), threading._MainThread), + reason="Not running in the main thread.", +) +@mock.patch.object(subprocess.Popen, "wait", new=KeyboardInterrupt) +def test_keyboard_interrupt(repo_dir, dvc_repo): + assert ( + main( [ "run", "-d", - self.FOO, + repo_dir.FOO, "-d", - self.CODE, + repo_dir.CODE, "-o", "out", "-f", "out.dvc", "python", - self.CODE, - self.FOO, + repo_dir.CODE, + repo_dir.FOO, "out", ] ) + == 1 + ) - stage = Stage.load(self.dvc, fname="out.dvc") - self.assertEqual(ret, 0) - self.assertTrue(os.path.isfile("out")) - self.assertTrue(os.path.isfile("out.dvc")) - self.assertTrue(filecmp.cmp(self.FOO, "out", shallow=False)) - self.assertEqual(stage.cmd, "python code.py foo out") - - def test_run_args_from_cli(self): - ret = main(["run", "echo", "foo"]) - stage = Stage.load(self.dvc, fname="Dvcfile") - self.assertEqual(ret, 0) - self.assertEqual(stage.cmd, "echo foo") - - def test_run_bad_command(self): - ret = main(["run", "non-existing-command"]) - self.assertNotEqual(ret, 0) - - def test_run_args_with_spaces(self): - ret = main(["run", "echo", "foo bar"]) - stage = Stage.load(self.dvc, fname="Dvcfile") - self.assertEqual(ret, 0) - self.assertEqual(stage.cmd, 'echo "foo bar"') - - @mock.patch.object(subprocess.Popen, "wait", new=KeyboardInterrupt) - def test_keyboard_interrupt(self): - ret = main( +@pytest.mark.skipif( + not isinstance(threading.current_thread(), threading._MainThread), + reason="Not running in the main thread.", +) +def test_keyboard_interrupt_after_second_signal_call( + mocker, repo_dir, dvc_repo +): + mocker.patch.object( + signal, "signal", side_effect=[None, KeyboardInterrupt] + ) + assert ( + main( [ "run", "-d", - self.FOO, + repo_dir.FOO, "-d", - self.CODE, + repo_dir.CODE, "-o", "out", "-f", "out.dvc", "python", - self.CODE, - self.FOO, + repo_dir.CODE, + repo_dir.FOO, "out", ] ) - self.assertEqual(ret, 1) - - @mock.patch.object(signal, "signal", side_effect=[None, KeyboardInterrupt]) - def test_keyboard_interrupt_after_second_signal_call(self, _): - ret = main( - [ - "run", - "-d", - self.FOO, - "-d", - self.CODE, - "-o", - "out", - "-f", - "out.dvc", - "python", - self.CODE, - self.FOO, - "out", - ] - ) - self.assertEqual(ret, 252) + == 252 + ) class TestRunRemoveOuts(TestDvc): @@ -977,7 +952,7 @@ class TestShouldNotCheckoutUponCorruptedLocalHardlinkCache(TestDvc): self.dvc = DvcRepo(".") def test(self): - cmd = "cp {} {}".format(self.FOO, self.BAR) + cmd = "python {} {} {}".format(self.CODE, self.FOO, self.BAR) stage = self.dvc.run(deps=[self.FOO], outs=[self.BAR], cmd=cmd) with open(self.BAR, "w") as fd: diff --git a/tests/unit/command/test_run.py b/tests/unit/command/test_run.py index 0dee369bd..4fde85a72 100644 --- a/tests/unit/command/test_run.py +++ b/tests/unit/command/test_run.py @@ -1,63 +1,112 @@ -import mock - -from unittest import TestCase - -from dvc.main import main +from dvc.cli import parse_args +from dvc.command.run import CmdRun -class TestCli(TestCase): - @mock.patch("dvc.main.Analytics", autospec=True) - @mock.patch("dvc.cli.run.CmdRun", autospec=True) - def test_file_name(self, cmd_mock, _): - cmd_mock.return_value.run_cmd.return_value = 0 - ret = main(["run", "-f", "test/out.dvc", "echo"]) - self.assertEqual(ret, 0) - cmd_mock.assert_called_once() - args, kwargs = cmd_mock.call_args_list[0] - self.assertEqual(len(args), 1) - self.assertEqual(len(kwargs), 0) - self.assertEqual(args[0].file, "test/out.dvc") - self.assertSequenceEqual(args[0].command, ["echo"]) - cmd_mock.return_value.run_cmd.assert_called_once() +def test_run(mocker, dvc_repo): + cli_args = parse_args( + [ + "run", + "--deps", + "deps", + "--outs", + "outs", + "--outs-no-cache", + "outs-no-cache", + "--metrics", + "metrics", + "--metrics-no-cache", + "metrics-no-cache", + "--file", + "file", + "--cwd", + "cwd", + "--wdir", + "wdir", + "--no-exec", + "--yes", + "--overwrite-dvcfile", + "--ignore-build-cache", + "--remove-outs", + "--no-commit", + "--outs-persist", + "outs-persist", + "--outs-persist-no-cache", + "outs-persist-no-cache", + "command", + ] + ) + assert cli_args.func == CmdRun - @mock.patch("dvc.main.Analytics", autospec=True) - @mock.patch("dvc.cli.run.CmdRun", autospec=True) - def test_file_name_default(self, cmd_mock, _): - cmd_mock.return_value.run_cmd.return_value = 0 - ret = main(["run", "-d", "foo", "echo"]) - self.assertEqual(ret, 0) - cmd_mock.assert_called_once() - args, kwargs = cmd_mock.call_args_list[0] - self.assertEqual(len(args), 1) - self.assertEqual(len(kwargs), 0) - self.assertIsNone(args[0].file) - self.assertSequenceEqual(args[0].command, ["echo"]) - cmd_mock.return_value.run_cmd.assert_called_once() + cmd = cli_args.func(cli_args) + m = mocker.patch.object(cmd.repo, "run", autospec=True) - @mock.patch("dvc.main.Analytics", autospec=True) - @mock.patch("dvc.cli.run.CmdRun", autospec=True) - def test_wdir(self, cmd_mock, _): - cmd_mock.return_value.run_cmd.return_value = 0 - ret = main(["run", "--wdir", "test/", "echo"]) - self.assertEqual(ret, 0) - cmd_mock.assert_called_once() - args, kwargs = cmd_mock.call_args_list[0] - self.assertEqual(len(args), 1) - self.assertEqual(len(kwargs), 0) - self.assertEqual(args[0].wdir, "test/") - self.assertSequenceEqual(args[0].command, ["echo"]) - cmd_mock.return_value.run_cmd.assert_called_once() + assert cmd.run() == 0 - @mock.patch("dvc.main.Analytics", autospec=True) - @mock.patch("dvc.cli.run.CmdRun", autospec=True) - def test_wdir_default(self, cmd_mock, _): - cmd_mock.return_value.run_cmd.return_value = 0 - ret = main(["run", "echo"]) - self.assertEqual(ret, 0) - cmd_mock.assert_called_once() - args, kwargs = cmd_mock.call_args_list[0] - self.assertEqual(len(args), 1) - self.assertEqual(len(kwargs), 0) - self.assertEqual(args[0].wdir, None) - self.assertSequenceEqual(args[0].command, ["echo"]) - cmd_mock.return_value.run_cmd.assert_called_once() + m.assert_called_once_with( + deps=["deps"], + outs=["outs"], + outs_no_cache=["outs-no-cache"], + metrics=["metrics"], + metrics_no_cache=["metrics-no-cache"], + outs_persist=["outs-persist"], + outs_persist_no_cache=["outs-persist-no-cache"], + fname="file", + cwd="cwd", + wdir="wdir", + no_exec=True, + overwrite=True, + ignore_build_cache=True, + remove_outs=True, + no_commit=True, + cmd="command", + ) + + +def test_run_args_from_cli(mocker, dvc_repo): + args = parse_args(["run", "echo", "foo"]) + cmd = args.func(args) + m = mocker.patch.object(cmd.repo, "run", autospec=True) + assert cmd.run() == 0 + m.assert_called_once_with( + deps=[], + outs=[], + outs_no_cache=[], + metrics=[], + metrics_no_cache=[], + outs_persist=[], + outs_persist_no_cache=[], + fname=None, + cwd=None, + wdir=None, + no_exec=False, + overwrite=False, + ignore_build_cache=False, + remove_outs=False, + no_commit=False, + cmd="echo foo", + ) + + +def test_run_args_with_spaces(mocker, dvc_repo): + args = parse_args(["run", "echo", "foo bar"]) + cmd = args.func(args) + m = mocker.patch.object(cmd.repo, "run", autospec=True) + assert cmd.run() == 0 + m.assert_called_once_with( + deps=[], + outs=[], + outs_no_cache=[], + metrics=[], + metrics_no_cache=[], + outs_persist=[], + outs_persist_no_cache=[], + fname=None, + cwd=None, + wdir=None, + no_exec=False, + overwrite=False, + ignore_build_cache=False, + remove_outs=False, + no_commit=False, + cmd='echo "foo bar"', + ) diff --git a/tests/unit/scm/test_git.py b/tests/unit/scm/test_git.py index b9ad27615..2874da16b 100644 --- a/tests/unit/scm/test_git.py +++ b/tests/unit/scm/test_git.py @@ -1,9 +1,9 @@ import os -from tests.basic_env import TestDvc +from tests.basic_env import TestDvcGit -class TestGit(TestDvc): +class TestGit(TestDvcGit): def test_belongs_to_scm_true_on_gitignore(self): path = os.path.join("path", "to", ".gitignore") self.assertTrue(self.dvc.scm.belongs_to_scm(path))