Merge pull request #3104 from efiop/ssh_dir_out

tests: remove redundant native ssh tests

Author: efiop

dvc/remote/ssh/connection.py

@@ -156,14 +156,12 @@ def _remove_file(self, path):
     def _remove_dir(self, path):
         for root, dirs, files in self.walk(path, topdown=False):
             for fname in files:
-                path = posixpath.join(root, fname)
                 with suppress(FileNotFoundError):
-                    self._remove_file(path)
+                    self._remove_file(posixpath.join(root, fname))
 
             for dname in dirs:
-                path = posixpath.join(root, dname)
                 with suppress(FileNotFoundError):
-                    self.sftp.rmdir(dname)
+                    self.sftp.rmdir(posixpath.join(root, dname))
 
         with suppress(FileNotFoundError):
             self.sftp.rmdir(path)

scripts/build-requirements.txt

@@ -0,0 +1,2 @@
+PyInstaller==3.5
+psutil

scripts/build_posix.sh

@@ -95,10 +95,8 @@ install_dependencies()
 	pip install --upgrade pip
 
 	print_info "Installing requirements..."
-	pip install .[all] psutil
-
-	print_info "Installing pyinstaller..."
-	pip install pyinstaller
+	pip install .[all]
+	pip install -r scripts/build-requirements.txt
 }
 
 build_dvc()

scripts/build_windows.cmd

@@ -23,9 +23,8 @@ if %errorlevel% neq 0 (echo Error: Couldn't find Inno Setup compiler. && goto :e
 echo ====== Installing requirements... ======
 echo PKG = "exe" > dvc\utils\build.py
 call pip install .[all] || goto :error
-call pip install psutil || goto :error
+call pip install -r scripts\build-requirements.txt || goto :error
 call dvc pull || goto :error
-call pip install pyinstaller || goto :error
 
 echo ====== Building dvc binary... ======
 call pyinstaller --additional-hooks-dir scripts\hooks dvc/__main__.py --name dvc --specpath build

scripts/hooks/hook-Crypto.py

@@ -99,7 +99,6 @@ def run(self):
 
 # Extra dependecies to run tests
 tests_requirements = [
-    "PyInstaller==3.5",
     "wheel>=0.31.1",
     "pydot>=1.2.4",
     # Test requirements:

scripts/hooks/hook-aliyunsdkcore.py

@@ -33,7 +33,6 @@
 from tests.remotes import (
     _should_test_gcp,
     _should_test_hdfs,
-    _should_test_ssh,
     Azure,
     GDrive,
     S3,
@@ -47,7 +46,6 @@
     get_gcp_url,
     get_hdfs_url,
     get_local_url,
-    get_ssh_url,
     get_ssh_url_mocked,
 )
 
@@ -296,17 +294,6 @@ def test(self):
         self.assertTrue(os.path.isdir(self.dname))
 
 
-class TestRemoteSSH(TestDataCloudBase):
-    def _should_test(self):
-        return _should_test_ssh()
-
-    def _get_url(self):
-        return get_ssh_url()
-
-    def _get_cloud_class(self):
-        return RemoteSSH
-
-
 @pytest.mark.usefixtures("ssh_server")
 class TestRemoteSSHMocked(TestDataCloudBase):
     @pytest.fixture(autouse=True)
@@ -442,18 +429,6 @@ def _test(self):
         self._test_cloud(TEST_REMOTE)
 
 
-class TestRemoteSSHCLI(TestDataCloudCLIBase):
-    def _should_test(self):
-        return _should_test_ssh()
-
-    def _test(self):
-        url = get_ssh_url()
-
-        self.main(["remote", "add", TEST_REMOTE, url])
-
-        self._test_cloud(TEST_REMOTE)
-
-
 class TestRemoteHDFSCLI(TestDataCloudCLIBase):
     def _should_test(self):
         return _should_test_hdfs()

setup.py

@@ -38,7 +38,7 @@
     _should_test_gcp,
     _should_test_hdfs,
     _should_test_ssh,
-    get_ssh_url,
+    get_ssh_url_mocked,
     S3,
     TEST_AWS_REPO_BUCKET,
     TEST_GCP_REPO_BUCKET,
@@ -1611,30 +1611,46 @@ def test(self):
         assert evaluation[2].relpath == "E.dvc"
 
 
-def test_ssh_dir_out(tmp_dir, dvc):
-    if not _should_test_ssh():
-        pytest.skip()
-
+@pytest.mark.skipif(
+    os.name == "nt",
+    reason="external output scenario is not supported on Windows",
+)
+def test_ssh_dir_out(tmp_dir, dvc, ssh_server):
     tmp_dir.gen({"foo": "foo content"})
 
     # Set up remote and cache
-    remote_url = get_ssh_url()
+    user = ssh_server.test_creds["username"]
+    port = ssh_server.port
+    keyfile = ssh_server.test_creds["key_filename"]
+
+    remote_url = get_ssh_url_mocked(user, port)
     assert main(["remote", "add", "upstream", remote_url]) == 0
+    assert main(["remote", "modify", "upstream", "keyfile", keyfile]) == 0
 
-    cache_url = get_ssh_url()
+    cache_url = get_ssh_url_mocked(user, port)
     assert main(["remote", "add", "sshcache", cache_url]) == 0
     assert main(["config", "cache.ssh", "sshcache"]) == 0
+    assert main(["remote", "modify", "sshcache", "keyfile", keyfile]) == 0
 
     # Recreating to reread configs
     repo = DvcRepo(dvc.root_dir)
 
+    # To avoid "WARNING: UNPROTECTED PRIVATE KEY FILE" from ssh
+    os.chmod(keyfile, 0o600)
+
+    (tmp_dir / "script.py").write_text(
+        "import sys, pathlib\n"
+        "path = pathlib.Path(sys.argv[1])\n"
+        "dir_out = path / 'dir-out'\n"
+        "dir_out.mkdir()\n"
+        "(dir_out / '1.txt').write_text('1')\n"
+        "(dir_out / '2.txt').write_text('2')\n"
+    )
+
     url_info = URLInfo(remote_url)
-    mkdir_cmd = "mkdir dir-out;cd dir-out;echo 1 > 1.txt; echo 2 > 2.txt"
     repo.run(
-        cmd="ssh {netloc} 'cd {path};{cmd}'".format(
-            netloc=url_info.netloc, path=url_info.path, cmd=mkdir_cmd
-        ),
-        outs=[(url_info / "dir-out").url],
+        cmd="python {} {}".format(tmp_dir / "script.py", url_info.path),
+        outs=["remote://upstream/dir-out"],
         deps=["foo"],  # add a fake dep to not consider this a callback
     )
 

tests/func/test_data_cloud.py

@@ -35,6 +35,16 @@ def test_makedirs(tmp_path, ssh):
     assert os.path.isdir(path)
 
 
+def test_remove_dir(tmp_path, ssh):
+    dpath = tmp_path / "dir"
+    dpath.mkdir()
+    (dpath / "file").write_text("file")
+    (dpath / "subdir").mkdir()
+    (dpath / "subdir" / "subfile").write_text("subfile")
+    ssh.remove(dpath.absolute().as_posix())
+    assert not dpath.exists()
+
+
 def test_walk(tmp_path, ssh):
     root_path = tmp_path
     dir_path = root_path / "dir"