appveyor: test py2 and py3

Related to #2166

Signed-off-by: Ruslan Kuprieiev <[email protected]>

.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 [email protected] # 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"

.gitignore

@@ -34,3 +34,5 @@ env.sh
 tests/remotes_env
 scripts/ci/gcp-creds.json
 .gcp-creds.json
+
+*~

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

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

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)

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():

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

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)

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()

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)

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

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

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()
 

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")):

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()

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)

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))

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):

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
 

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(

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:

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"',
+    )

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))