-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
external repo: checkout revision before initializing dvc repo #2852
Changes from all commits
06d6c2c
b788a99
b9d5933
5c0c423
5e5c826
97e4b02
f1d59dd
ca85217
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
from __future__ import unicode_literals | ||
|
||
import filecmp | ||
import logging | ||
import os | ||
|
||
import pytest | ||
|
@@ -11,6 +12,7 @@ | |
from dvc.repo import Repo | ||
from dvc.system import System | ||
from dvc.utils import makedirs | ||
from dvc.utils.compat import fspath | ||
from tests.utils import trees_equal | ||
|
||
|
||
|
@@ -87,3 +89,19 @@ def test_get_to_dir(dname, erepo): | |
|
||
assert os.path.isdir(dname) | ||
assert filecmp.cmp(erepo.FOO, dst, shallow=False) | ||
|
||
|
||
def test_get_from_non_dvc_master(erepo, tmp_path, monkeypatch, caplog): | ||
monkeypatch.chdir(fspath(tmp_path)) | ||
erepo.dvc.scm.repo.index.remove([".dvc"], r=True) | ||
efiop marked this conversation as resolved.
Show resolved
Hide resolved
|
||
erepo.dvc.scm.commit("remove .dvc") | ||
|
||
caplog.clear() | ||
imported_file = "foo_imported" | ||
with caplog.at_level(logging.INFO, logger="dvc"): | ||
Repo.get(erepo._root_dir, erepo.FOO, out=imported_file, rev="branch") | ||
|
||
assert caplog.text == "" | ||
Comment on lines
+101
to
+104
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, why do we test that there is no info output to logger? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #2858 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You'll probably get progress bars in capsys, not easy to test, especially automatically, but your call, I don't have a strong opinion here π There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. pbars go to stderr by default, so shouldn't be an issue. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder why |
||
assert filecmp.cmp( | ||
os.path.join(erepo._root_dir, erepo.FOO), imported_file, shallow=False | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can skip creating
Repo
and useConfig
directly here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that it can be done with just config, though it would require a change in
test_get_from_non_dvc_repo
. I'll create an issue for that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#2880
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should not require changing any tests. This should be a self-contained change.