From 789faa4f115f0d47f7bc9baa60e9057aeba42be8 Mon Sep 17 00:00:00 2001 From: Saugat Pachhai Date: Tue, 18 May 2021 17:40:11 +0545 Subject: [PATCH] ui: migrate checkout to ui.write (#6028) --- dvc/command/checkout.py | 17 ++++------------- tests/func/test_checkout.py | 27 +++++++++++---------------- tests/func/test_data_cloud.py | 15 +++++++++------ tests/unit/command/test_checkout.py | 16 +++++++--------- 4 files changed, 31 insertions(+), 44 deletions(-) diff --git a/dvc/command/checkout.py b/dvc/command/checkout.py index 858358df8..a0dfe0477 100644 --- a/dvc/command/checkout.py +++ b/dvc/command/checkout.py @@ -1,5 +1,4 @@ import argparse -import logging import operator import colorama @@ -7,8 +6,7 @@ import colorama from dvc.command import completion from dvc.command.base import CmdBase, append_doc_link from dvc.exceptions import CheckoutError - -logger = logging.getLogger(__name__) +from dvc.ui import ui def log_changes(stats): @@ -24,16 +22,9 @@ def log_changes(stats): if not entries: continue + nc = colorama.Fore.RESET for entry in entries: - logger.info( - "{color}{state}{nc}{spacing}{entry}".format( - color=color, - state=state[0].upper(), - nc=colorama.Fore.RESET, - spacing="\t", - entry=entry, - ) - ) + ui.write(f"{color}{state[0].upper()}{nc}", entry, sep="\t") class CmdCheckout(CmdBase): @@ -58,7 +49,7 @@ class CmdCheckout(CmdBase): msg = get_summary( sorted(stats.items(), key=operator.itemgetter(0)) ) - logger.info(msg or default_message) + ui.write(msg or default_message) else: log_changes(stats) diff --git a/tests/func/test_checkout.py b/tests/func/test_checkout.py index 2939dd45b..00f7e3256 100644 --- a/tests/func/test_checkout.py +++ b/tests/func/test_checkout.py @@ -26,6 +26,7 @@ from dvc.utils import relpath from dvc.utils.fs import remove, walk_files from dvc.utils.serialize import dump_yaml, load_yaml from tests.basic_env import TestDvc, TestDvcGit +from tests.func.parsing.test_errors import escape_ansi from tests.func.test_repro import TestRepro logger = logging.getLogger("dvc") @@ -676,7 +677,7 @@ def test_stats_on_removed_file_from_tracked_dir(tmp_dir, dvc, scm): def test_stats_on_show_changes_does_not_show_summary( - tmp_dir, dvc, scm, caplog + tmp_dir, dvc, scm, capsys ): tmp_dir.dvc_gen( {"dir": {"subdir": {"file": "file"}}, "other": "other"}, @@ -684,29 +685,23 @@ def test_stats_on_show_changes_does_not_show_summary( ) scm.checkout("HEAD~") - with caplog.at_level(logging.INFO, logger="dvc"): - caplog.clear() - assert main(["checkout"]) == 0 - for out in ["D\tdir" + os.sep, "D\tother"]: - assert out in caplog.text - assert "modified" not in caplog.text - assert "deleted" not in caplog.text - assert "added" not in caplog.text + assert main(["checkout"]) == 0 + + out, _ = capsys.readouterr() + assert escape_ansi(out).splitlines() == [f"D\tdir{os.sep}", "D\tother"] -def test_stats_does_not_show_changes_by_default(tmp_dir, dvc, scm, caplog): +def test_stats_does_not_show_changes_by_default(tmp_dir, dvc, scm, capsys): tmp_dir.dvc_gen( {"dir": {"subdir": {"file": "file"}}, "other": "other"}, commit="initial", ) scm.checkout("HEAD~") - with caplog.at_level(logging.INFO, logger="dvc"): - caplog.clear() - assert main(["checkout", "--summary"]) == 0 - assert "2 files deleted" in caplog.text - assert "dir" not in caplog.text - assert "other" not in caplog.text + assert main(["checkout", "--summary"]) == 0 + + out, _ = capsys.readouterr() + assert "2 files deleted" == out.rstrip() @pytest.mark.parametrize("link", ["hardlink", "symlink", "copy"]) diff --git a/tests/func/test_data_cloud.py b/tests/func/test_data_cloud.py index 958c75ac6..b8f53cb94 100644 --- a/tests/func/test_data_cloud.py +++ b/tests/func/test_data_cloud.py @@ -18,6 +18,7 @@ from dvc.remote.base import ( from dvc.stage.exceptions import StageNotFound from dvc.utils.fs import move, remove from dvc.utils.serialize import dump_yaml, load_yaml +from tests.func.parsing.test_errors import escape_ansi all_clouds = [ pytest.lazy_fixture(cloud) @@ -484,16 +485,18 @@ def test_fetch_stats(tmp_dir, dvc, fs, msg, caplog, local_remote): assert msg in caplog.text -def test_pull_stats(tmp_dir, dvc, caplog, local_remote): +def test_pull_stats(tmp_dir, dvc, capsys, caplog, local_remote): tmp_dir.dvc_gen({"foo": "foo", "bar": "bar"}) dvc.push() clean(["foo", "bar"], dvc) (tmp_dir / "bar").write_text("foobar") - caplog.clear() - with caplog.at_level(level=logging.INFO, logger="dvc"): - main(["pull", "--force"]) - assert "M\tbar" in caplog.text - assert "A\tfoo" in caplog.text + + assert main(["pull", "--force"]) == 0 + + out, _ = capsys.readouterr() + out = escape_ansi(out) + assert "M\tbar" in out + assert "A\tfoo" in out assert "2 files fetched" in caplog.text assert "1 file added" in caplog.text assert "1 file modified" in caplog.text diff --git a/tests/unit/command/test_checkout.py b/tests/unit/command/test_checkout.py index e2315a290..6e76410e2 100644 --- a/tests/unit/command/test_checkout.py +++ b/tests/unit/command/test_checkout.py @@ -1,7 +1,6 @@ -import logging - from dvc.cli import parse_args from dvc.command.checkout import CmdCheckout, log_changes +from tests.func.parsing.test_errors import escape_ansi def test_checkout(tmp_dir, dvc, mocker): @@ -23,7 +22,7 @@ def test_checkout(tmp_dir, dvc, mocker): ) -def test_log_changes(caplog): +def test_log_changes(capsys): stats = { "added": ["file1", "dir1/"], "deleted": ["dir2/"], @@ -33,12 +32,11 @@ def test_log_changes(caplog): from itertools import zip_longest def _assert_output(stats, expected_outs): - with caplog.at_level(logging.INFO, logger="dvc"): - caplog.clear() - log_changes(stats) - actual_output = caplog.text.splitlines() - for out, line in zip_longest(expected_outs, actual_output): - assert out in line + log_changes(stats) + out, _ = capsys.readouterr() + actual_output = escape_ansi(out).splitlines() + for out, line in zip_longest(expected_outs, actual_output): + assert out in line _assert_output(stats, ["M\tfile2", "A\tfile1", "A\tdir1/", "D\tdir2/"])