tests: erepo_dir: extract remote setup to separate method (#3748)

tests/dir_helpers.py

@@ -59,10 +59,10 @@ __all__ = [
     "tmp_dir",
     "scm",
     "dvc",
-    "local_remote",
     "run_copy",
     "erepo_dir",
     "git_dir",
+    "setup_remote",
 ]
 
 
@@ -256,17 +256,6 @@ def _git_init(path):
     git.close()
 
 
-@pytest.fixture
-def local_remote(request, tmp_dir, dvc, make_tmp_dir):
-    path = make_tmp_dir("local-remote")
-    with dvc.config.edit() as conf:
-        conf["remote"]["upstream"] = {"url": fspath(path)}
-        conf["core"]["remote"] = "upstream"
-    if "scm" in request.fixturenames:
-        tmp_dir.scm_add([dvc.config.files["repo"]], commit="add remote")
-    return path
-
-
 @pytest.fixture
 def run_copy(tmp_dir, dvc):
     tmp_dir.gen(
@@ -287,17 +276,7 @@ def run_copy(tmp_dir, dvc):
 
 @pytest.fixture
 def erepo_dir(make_tmp_dir):
-    path = make_tmp_dir("erepo", scm=True, dvc=True)
-
-    # Chdir for git and dvc to work locally
-    with path.chdir():
-        with path.dvc.config.edit() as conf:
-            cache_dir = path.dvc.cache.local.cache_dir
-            conf["remote"]["upstream"] = {"url": cache_dir}
-            conf["core"]["remote"] = "upstream"
-        path.scm_add([path.dvc.config.files["repo"]], commit="add remote")
-
-    return path
+    return make_tmp_dir("erepo", scm=True, dvc=True)
 
 
 @pytest.fixture
@@ -305,3 +284,20 @@ def git_dir(make_tmp_dir):
     path = make_tmp_dir("git-erepo", scm=True)
     path.scm.commit("init repo")
     return path
+
+
+@pytest.fixture
+def setup_remote(make_tmp_dir):
+    def create(repo, url=None, name="upstream", default=True):
+        if not url:
+            url = fspath(make_tmp_dir("local_remote"))
+        with repo.config.edit() as conf:
+            conf["remote"][name] = {"url": url}
+            if default:
+                conf["core"]["remote"] = name
+
+        repo.scm.add(repo.config.files["repo"])
+        repo.scm.commit("add '{}' remote".format(name))
+        return url
+
+    return create

tests/func/test_api.py

@@ -36,8 +36,8 @@ def test_get_url(tmp_dir, dvc, remote_url):
 
 
 @pytest.mark.parametrize("remote_url", remote_params, indirect=True)
-def test_get_url_external(erepo_dir, remote_url):
-    _set_remote_url_and_commit(erepo_dir.dvc, remote_url)
+def test_get_url_external(erepo_dir, remote_url, setup_remote):
+    setup_remote(erepo_dir.dvc, url=remote_url)
     with erepo_dir.chdir():
         erepo_dir.dvc_gen("foo", "foo", commit="add foo")
 
@@ -71,8 +71,8 @@ def test_open(remote_url, tmp_dir, dvc):
 
 
 @pytest.mark.parametrize("remote_url", all_remote_params, indirect=True)
-def test_open_external(remote_url, erepo_dir):
-    _set_remote_url_and_commit(erepo_dir.dvc, remote_url)
+def test_open_external(remote_url, erepo_dir, setup_remote):
+    setup_remote(erepo_dir.dvc, url=remote_url)
 
     with erepo_dir.chdir():
         erepo_dir.dvc_gen("version", "master", commit="add version")
@@ -106,13 +106,6 @@ def test_missing(remote_url, tmp_dir, dvc):
         api.read("foo")
 
 
-def _set_remote_url_and_commit(repo, remote_url):
-    with repo.config.edit() as conf:
-        conf["remote"]["upstream"]["url"] = remote_url
-    repo.scm.add([repo.config.files["repo"]])
-    repo.scm.commit("modify remote")
-
-
 def test_open_scm_controlled(tmp_dir, erepo_dir):
     erepo_dir.scm_gen({"scm_controlled": "file content"}, commit="create file")
 

tests/func/test_data_cloud.py

@@ -8,7 +8,7 @@ from unittest import SkipTest
 import pytest
 
 from dvc.cache import NamedCache
-from dvc.compat import fspath, fspath_py35
+from dvc.compat import fspath
 from dvc.data_cloud import DataCloud
 from dvc.external_repo import clean_repos
 from dvc.main import main
@@ -688,14 +688,13 @@ class TestShouldWarnOnNoChecksumInLocalAndRemoteCache(TestDvc):
         assert self.message_bar_part in self._caplog.text
 
 
-def test_verify_checksums(tmp_dir, scm, dvc, mocker, tmp_path_factory):
+def test_verify_checksums(
+    tmp_dir, scm, dvc, mocker, tmp_path_factory, setup_remote
+):
+
+    setup_remote(dvc, name="upstream")
     tmp_dir.dvc_gen({"file": "file1 content"}, commit="add file")
     tmp_dir.dvc_gen({"dir": {"subfile": "file2 content"}}, commit="add dir")
-
-    dvc.config["remote"]["local_remote"] = {
-        "url": fspath(tmp_path_factory.mktemp("local_remote"))
-    }
-    dvc.config["core"]["remote"] = "local_remote"
     dvc.push()
 
     # remove artifacts and cache to trigger fetching
@@ -711,7 +710,7 @@ def test_verify_checksums(tmp_dir, scm, dvc, mocker, tmp_path_factory):
     # Removing cache will invalidate existing state entries
     remove(dvc.cache.local.cache_dir)
 
-    dvc.config["remote"]["local_remote"]["verify"] = True
+    dvc.config["remote"]["upstream"]["verify"] = True
 
     dvc.pull()
     assert checksum_spy.call_count == 3
@@ -784,13 +783,15 @@ def recurse_list_dir(d):
     ]
 
 
-def test_dvc_pull_pipeline_stages(tmp_dir, dvc, local_remote, run_copy):
+def test_dvc_pull_pipeline_stages(tmp_dir, dvc, run_copy, setup_remote):
+    setup_remote(dvc)
     (stage0,) = tmp_dir.dvc_gen("foo", "foo")
     stage1 = run_copy("foo", "bar", single_stage=True)
     stage2 = run_copy("bar", "foobar", name="copy-bar-foobar")
+    dvc.push()
+
     outs = ["foo", "bar", "foobar"]
 
-    dvc.push()
     clean(outs, dvc)
     dvc.pull()
     assert all((tmp_dir / file).exists() for file in outs)
@@ -814,7 +815,8 @@ def test_dvc_pull_pipeline_stages(tmp_dir, dvc, local_remote, run_copy):
     assert set(stats["added"]) == set(outs)
 
 
-def test_pipeline_file_target_ops(tmp_dir, dvc, local_remote, run_copy):
+def test_pipeline_file_target_ops(tmp_dir, dvc, run_copy, setup_remote):
+    remote_path = setup_remote(dvc)
     tmp_dir.dvc_gen("foo", "foo")
     run_copy("foo", "bar", single_stage=True)
 
@@ -829,8 +831,11 @@ def test_pipeline_file_target_ops(tmp_dir, dvc, local_remote, run_copy):
     remove(dvc.stage_cache.cache_dir)
 
     dvc.push()
+
+    outs = ["foo", "bar", "lorem", "ipsum", "baz", "lorem2"]
+
     # each one's a copy of other, hence 3
-    assert len(recurse_list_dir(fspath_py35(local_remote))) == 3
+    assert len(recurse_list_dir(remote_path)) == 3
 
     clean(outs, dvc)
     assert set(dvc.pull(["dvc.yaml"])["added"]) == {"lorem2", "baz"}
@@ -839,13 +844,15 @@ def test_pipeline_file_target_ops(tmp_dir, dvc, local_remote, run_copy):
     assert set(dvc.pull()["added"]) == set(outs)
 
     # clean everything in remote and push
-    clean(local_remote.iterdir())
-    dvc.push(["dvc.yaml:copy-ipsum-baz"])
-    assert len(recurse_list_dir(fspath_py35(local_remote))) == 1
+    from tests.dir_helpers import TmpDir
 
-    clean(local_remote.iterdir())
+    clean(TmpDir(remote_path).iterdir())
+    dvc.push(["dvc.yaml:copy-ipsum-baz"])
+    assert len(recurse_list_dir(remote_path)) == 1
+
+    clean(TmpDir(remote_path).iterdir())
     dvc.push(["dvc.yaml"])
-    assert len(recurse_list_dir(fspath_py35(local_remote))) == 2
+    assert len(recurse_list_dir(remote_path)) == 2
 
     with pytest.raises(StageNotFound):
         dvc.push(["dvc.yaml:StageThatDoesNotExist"])
@@ -862,8 +869,10 @@ def test_pipeline_file_target_ops(tmp_dir, dvc, local_remote, run_copy):
         ({}, "Everything is up to date"),
     ],
 )
-def test_push_stats(tmp_dir, dvc, fs, msg, local_remote, caplog):
+def test_push_stats(tmp_dir, dvc, fs, msg, caplog, setup_remote):
+    setup_remote(dvc)
     tmp_dir.dvc_gen(fs)
+
     caplog.clear()
     with caplog.at_level(level=logging.INFO, logger="dvc"):
         main(["push"])
@@ -878,7 +887,8 @@ def test_push_stats(tmp_dir, dvc, fs, msg, local_remote, caplog):
         ({}, "Everything is up to date."),
     ],
 )
-def test_fetch_stats(tmp_dir, dvc, fs, msg, local_remote, caplog):
+def test_fetch_stats(tmp_dir, dvc, fs, msg, caplog, setup_remote):
+    setup_remote(dvc)
     tmp_dir.dvc_gen(fs)
     dvc.push()
     clean(list(fs.keys()), dvc)
@@ -888,7 +898,8 @@ def test_fetch_stats(tmp_dir, dvc, fs, msg, local_remote, caplog):
     assert msg in caplog.text
 
 
-def test_pull_stats(tmp_dir, dvc, local_remote, caplog):
+def test_pull_stats(tmp_dir, dvc, caplog, setup_remote):
+    setup_remote(dvc)
     tmp_dir.dvc_gen({"foo": "foo", "bar": "bar"})
     dvc.push()
     clean(["foo", "bar"], dvc)

tests/func/test_external_repo.py

@@ -43,9 +43,11 @@ def test_source_change(erepo_dir):
     assert old_rev != new_rev
 
 
-def test_cache_reused(erepo_dir, mocker):
+def test_cache_reused(erepo_dir, mocker, setup_remote):
+    setup_remote(erepo_dir.dvc)
     with erepo_dir.chdir():
         erepo_dir.dvc_gen("file", "text", commit="add file")
+    erepo_dir.dvc.push()
 
     download_spy = mocker.spy(LocalRemote, "download")
 
@@ -63,6 +65,8 @@ def test_cache_reused(erepo_dir, mocker):
 
 
 def test_known_sha(erepo_dir):
+    erepo_dir.scm.commit("init")
+
     url = "file://{}".format(erepo_dir)
     with external_repo(url) as repo:
         rev = repo.scm.get_rev()
@@ -92,7 +96,7 @@ def test_pull_subdir_file(tmp_dir, erepo_dir):
     assert dest.read_text() == "contents"
 
 
-def test_relative_remote(erepo_dir, tmp_dir):
+def test_relative_remote(erepo_dir, tmp_dir, setup_remote):
     # these steps reproduce the script on this issue:
     # https://github.com/iterative/dvc/issues/2756
     with erepo_dir.chdir():
@@ -100,13 +104,8 @@ def test_relative_remote(erepo_dir, tmp_dir):
 
     upstream_dir = tmp_dir
     upstream_url = relpath(upstream_dir, erepo_dir)
-    with erepo_dir.dvc.config.edit() as conf:
-        conf["remote"]["upstream"] = {"url": upstream_url}
-        conf["core"]["remote"] = "upstream"
+    setup_remote(erepo_dir.dvc, url=upstream_url, name="upstream")
 
-    erepo_dir.scm_add(
-        erepo_dir.dvc.config.files["repo"], commit="Update dvc config"
-    )
     erepo_dir.dvc.push()
 
     (erepo_dir / "file").unlink()

tests/func/test_gc.py

@@ -238,18 +238,18 @@ def test_gc_without_workspace_raises_error(tmp_dir, dvc):
         dvc.gc(force=True, workspace=False)
 
 
-def test_gc_cloud_with_or_without_specifier(tmp_dir, erepo_dir):
+def test_gc_cloud_with_or_without_specifier(tmp_dir, erepo_dir, setup_remote):
     dvc = erepo_dir.dvc
-    with erepo_dir.chdir():
-        from dvc.exceptions import InvalidArgumentError
+    setup_remote(dvc)
+    from dvc.exceptions import InvalidArgumentError
 
-        with pytest.raises(InvalidArgumentError):
-            dvc.gc(force=True, cloud=True)
+    with pytest.raises(InvalidArgumentError):
+        dvc.gc(force=True, cloud=True)
 
-        dvc.gc(cloud=True, all_tags=True)
-        dvc.gc(cloud=True, all_commits=True)
-        dvc.gc(cloud=True, all_branches=True)
-        dvc.gc(cloud=True, all_commits=False, all_branches=True, all_tags=True)
+    dvc.gc(cloud=True, all_tags=True)
+    dvc.gc(cloud=True, all_commits=True)
+    dvc.gc(cloud=True, all_branches=True)
+    dvc.gc(cloud=True, all_commits=False, all_branches=True, all_tags=True)
 
 
 def test_gc_without_workspace_on_tags_branches_commits(tmp_dir, dvc):
@@ -295,13 +295,8 @@ def test_gc_with_possible_args_positive(tmp_dir, dvc):
         assert main(["gc", "-vf", flag]) == 0
 
 
-def test_gc_cloud_positive(tmp_dir, dvc, tmp_path_factory):
-    with dvc.config.edit() as conf:
-        storage = fspath(tmp_path_factory.mktemp("test_remote_base"))
-        conf["remote"]["local_remote"] = {"url": storage}
-        conf["core"]["remote"] = "local_remote"
-
-    dvc.push()
+def test_gc_cloud_positive(tmp_dir, dvc, tmp_path_factory, setup_remote):
+    setup_remote(dvc)
 
     for flag in ["-cw", "-ca", "-cT", "-caT", "-cwT"]:
         assert main(["gc", "-vf", flag]) == 0

tests/func/test_get.py

@@ -197,9 +197,11 @@ def test_get_file_from_dir(tmp_dir, erepo_dir):
     assert (tmp_dir / "X").read_text() == "foo"
 
 
-def test_get_url_positive(tmp_dir, erepo_dir, caplog):
+def test_get_url_positive(tmp_dir, erepo_dir, caplog, setup_remote):
+    setup_remote(erepo_dir.dvc)
     with erepo_dir.chdir():
         erepo_dir.dvc_gen("foo", "foo")
+    erepo_dir.dvc.push()
 
     caplog.clear()
     with caplog.at_level(logging.ERROR, logger="dvc"):
@@ -225,15 +227,17 @@ def test_get_url_git_only_repo(tmp_dir, scm, caplog):
 
 
 def test_get_pipeline_tracked_outs(
-    tmp_dir, dvc, scm, git_dir, local_remote, run_copy
+    tmp_dir, dvc, scm, git_dir, run_copy, setup_remote
 ):
     from dvc.dvcfile import PIPELINE_FILE, PIPELINE_LOCK
 
+    setup_remote(dvc)
     tmp_dir.gen("foo", "foo")
     run_copy("foo", "bar", name="copy-foo-bar")
+    dvc.push()
+
     dvc.scm.add([PIPELINE_FILE, PIPELINE_LOCK])
     dvc.scm.commit("add pipeline stage")
-    dvc.push()
 
     with git_dir.chdir():
         Repo.get("file:///{}".format(fspath(tmp_dir)), "bar", out="baz")

tests/func/test_import.py

@@ -309,17 +309,20 @@ def test_pull_no_rev_lock(erepo_dir, tmp_dir, dvc):
     assert (tmp_dir / "foo_imported").read_text() == "contents"
 
 
-def test_import_from_bare_git_repo(tmp_dir, make_tmp_dir, erepo_dir):
+def test_import_from_bare_git_repo(
+    tmp_dir, make_tmp_dir, erepo_dir, setup_remote
+):
     import git
 
     git.Repo.init(fspath(tmp_dir), bare=True)
 
+    setup_remote(erepo_dir.dvc)
     with erepo_dir.chdir():
         erepo_dir.dvc_gen({"foo": "foo"}, commit="initial")
-        erepo_dir.dvc.push()
+    erepo_dir.dvc.push()
 
-        erepo_dir.scm.repo.create_remote("origin", fspath(tmp_dir))
-        erepo_dir.scm.repo.remote("origin").push("master")
+    erepo_dir.scm.repo.create_remote("origin", fspath(tmp_dir))
+    erepo_dir.scm.repo.remote("origin").push("master")
 
     dvc_repo = make_tmp_dir("dvc-repo", scm=True, dvc=True)
     with dvc_repo.chdir():
@@ -327,15 +330,17 @@ def test_import_from_bare_git_repo(tmp_dir, make_tmp_dir, erepo_dir):
 
 
 def test_import_pipeline_tracked_outs(
-    tmp_dir, dvc, scm, erepo_dir, local_remote, run_copy
+    tmp_dir, dvc, scm, erepo_dir, run_copy, setup_remote
 ):
     from dvc.dvcfile import PIPELINE_FILE, PIPELINE_LOCK
 
+    setup_remote(dvc)
     tmp_dir.gen("foo", "foo")
     run_copy("foo", "bar", name="copy-foo-bar")
+    dvc.push()
+
     dvc.scm.add([PIPELINE_FILE, PIPELINE_LOCK])
     dvc.scm.commit("add pipeline stage")
-    dvc.push()
 
     with erepo_dir.chdir():
         erepo_dir.dvc.imp(

tests/func/test_remote.py

@@ -173,10 +173,8 @@ def test_dir_checksum_should_be_key_order_agnostic(tmp_dir, dvc):
     assert checksum1 == checksum2
 
 
-def test_partial_push_n_pull(tmp_dir, dvc, tmp_path_factory):
-    url = fspath(tmp_path_factory.mktemp("upstream"))
-    dvc.config["remote"]["upstream"] = {"url": url}
-    dvc.config["core"]["remote"] = "upstream"
+def test_partial_push_n_pull(tmp_dir, dvc, tmp_path_factory, setup_remote):
+    setup_remote(dvc, name="upstream")
 
     foo = tmp_dir.dvc_gen({"foo": "foo content"})[0].outs[0]
     bar = tmp_dir.dvc_gen({"bar": "bar content"})[0].outs[0]
@@ -212,11 +210,10 @@ def test_partial_push_n_pull(tmp_dir, dvc, tmp_path_factory):
         assert download_error_info.value.amount == 3
 
 
-def test_raise_on_too_many_open_files(tmp_dir, dvc, tmp_path_factory, mocker):
-    storage = fspath(tmp_path_factory.mktemp("test_remote_base"))
-    dvc.config["remote"]["local_remote"] = {"url": storage}
-    dvc.config["core"]["remote"] = "local_remote"
-
+def test_raise_on_too_many_open_files(
+    tmp_dir, dvc, tmp_path_factory, mocker, setup_remote
+):
+    setup_remote(dvc)
     tmp_dir.dvc_gen({"file": "file content"})
 
     mocker.patch.object(
@@ -245,11 +242,8 @@ def test_external_dir_resource_on_no_cache(tmp_dir, dvc, tmp_path_factory):
         dvc.run(deps=[fspath(external_dir)], single_stage=True)
 
 
-def test_push_order(tmp_dir, dvc, tmp_path_factory, mocker):
-    url = fspath(tmp_path_factory.mktemp("upstream"))
-    dvc.config["remote"]["upstream"] = {"url": url}
-    dvc.config["core"]["remote"] = "upstream"
-
+def test_push_order(tmp_dir, dvc, tmp_path_factory, mocker, setup_remote):
+    setup_remote(dvc)
     tmp_dir.dvc_gen({"foo": {"bar": "bar content"}})
     tmp_dir.dvc_gen({"baz": "baz content"})